This is an automated email from the ASF dual-hosted git repository.
KKcorps pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 0306987954e Keep orphaned multipart temp files out of java.io.tmpdir
(#19627)
0306987954e is described below
commit 0306987954e81501813b74146988bfd8f64b954f
Author: Shounak kulkarni <[email protected]>
AuthorDate: Wed Sep 23 17:01:58 2026 +0530
Keep orphaned multipart temp files out of java.io.tmpdir (#19627)
---
.../api/ControllerAdminApiApplication.java | 58 ++++++
.../api/resources/ControllerFilePathProvider.java | 12 ++
.../PinotSegmentUploadDownloadRestletResource.java | 19 +-
.../api/ControllerAdminApiApplicationTest.java | 216 +++++++++++++++++++++
.../api/ControllerFilePathProviderTest.java | 48 +++++
...otSegmentUploadDownloadRestletResourceTest.java | 39 ++++
6 files changed, 391 insertions(+), 1 deletion(-)
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/ControllerAdminApiApplication.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/ControllerAdminApiApplication.java
index a2825e64490..aad9e5e67ac 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/ControllerAdminApiApplication.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/ControllerAdminApiApplication.java
@@ -18,6 +18,7 @@
*/
package org.apache.pinot.controller.api;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import io.swagger.jaxrs.listing.SwaggerSerializers;
import java.io.IOException;
@@ -29,8 +30,11 @@ import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.http.HttpServletResponse;
import javax.ws.rs.container.ContainerRequestContext;
+import javax.ws.rs.container.ContainerRequestFilter;
import javax.ws.rs.container.ContainerResponseContext;
import javax.ws.rs.container.ContainerResponseFilter;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.ext.ContextResolver;
import javax.ws.rs.ext.Provider;
import org.apache.pinot.common.audit.AuditLogFilter;
import org.apache.pinot.common.metrics.ControllerGauge;
@@ -39,6 +43,7 @@ import
org.apache.pinot.common.swagger.SwaggerApiListingResource;
import org.apache.pinot.common.swagger.SwaggerSetupUtils;
import org.apache.pinot.controller.ControllerConf;
import org.apache.pinot.controller.api.access.AuthenticationFilter;
+import org.apache.pinot.controller.api.resources.ControllerFilePathProvider;
import org.apache.pinot.core.api.ServiceAutoDiscoveryFeature;
import org.apache.pinot.core.transport.ListenerConfig;
import org.apache.pinot.core.util.ListenerConfigUtil;
@@ -55,12 +60,17 @@ import org.glassfish.grizzly.threadpool.ThreadPoolProbe;
import org.glassfish.hk2.utilities.binding.AbstractBinder;
import org.glassfish.jersey.jackson.JacksonFeature;
import org.glassfish.jersey.media.multipart.MultiPartFeature;
+import org.glassfish.jersey.media.multipart.MultiPartProperties;
import org.glassfish.jersey.server.ManagedAsyncExecutor;
import org.glassfish.jersey.server.ResourceConfig;
import org.glassfish.jersey.spi.ExecutorServiceProvider;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class ControllerAdminApiApplication extends ResourceConfig {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(ControllerAdminApiApplication.class);
+
public static final String PINOT_CONFIGURATION = "pinotConfiguration";
public static final String START_TIME = "controllerStartTime";
@@ -86,6 +96,8 @@ public class ControllerAdminApiApplication extends
ResourceConfig {
}
register(JacksonFeature.class);
register(MultiPartFeature.class);
+ register(new MultiPartTempDirResolver());
+ register(new MultiPartTempDirGuard());
register(SwaggerApiListingResource.class);
register(SwaggerSerializers.class);
register(new CorsFilter());
@@ -233,4 +245,50 @@ public class ControllerAdminApiApplication extends
ResourceConfig {
// managed in ControllerAdminApiApplication.stop()
}
}
+
+ /// Points Jersey's multipart parser at the controller's own temporary
directory instead of `java.io.tmpdir`.
+ ///
+ /// Jersey buffers any part larger than its threshold to disk, but only
registers the parsed `MultiPart` with the
+ /// request's `CloseableService` after parsing succeeds. A request that
fails to parse — a truncated upload, a
+ /// client disconnect, a malformed `Content-Disposition` — therefore leaves
its spilled parts behind, and for
+ /// segment uploads those are the size of the segment. Directing them at the
controller's temp tree means the
+ /// startup clean in [ControllerFilePathProvider] reclaims them rather than
leaving them on the host forever.
+ @VisibleForTesting
+ static class MultiPartTempDirResolver implements
ContextResolver<MultiPartProperties> {
+ @Override
+ public MultiPartProperties getContext(Class<?> type) {
+ MultiPartProperties properties = new MultiPartProperties();
+ try {
+ return
properties.tempDir(ControllerFilePathProvider.getInstance().getMultiPartTempDir().getAbsolutePath());
+ } catch (Exception e) {
+ // Falling back is still better than failing every upload, but it is
not a per-request fallback: this runs
+ // once at startup, so the controller is stuck with java.io.tmpdir
until it restarts.
+ LOGGER.error("Failed to resolve the multipart temporary directory.
Multipart uploads will spill into the JVM "
+ + "default temporary directory for the lifetime of this
controller, where orphaned parts are never "
+ + "reclaimed", e);
+ return properties;
+ }
+ }
+ }
+
+ /// Re-creates the multipart temporary directory if it has gone missing
since [MultiPartTempDirResolver] resolved it.
+ /// Only multipart requests pay for this, and only the cost of a `stat` when
the directory is present.
+ @Provider
+ @VisibleForTesting
+ static class MultiPartTempDirGuard implements ContainerRequestFilter {
+ @Override
+ public void filter(ContainerRequestContext requestContext) {
+ MediaType mediaType = requestContext.getMediaType();
+ if (mediaType == null ||
!mediaType.getType().equalsIgnoreCase("multipart")) {
+ return;
+ }
+ try {
+ ControllerFilePathProvider.getInstance().getMultiPartTempDir();
+ } catch (Exception e) {
+ // Leave the request alone: if the directory really is unusable the
parse fails with its own error, and this
+ // guard must not be the thing that rejects an otherwise valid upload.
+ LOGGER.warn("Failed to ensure the multipart temporary directory
exists", e);
+ }
+ }
+ }
}
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ControllerFilePathProvider.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ControllerFilePathProvider.java
index 14e40180b1a..15f0c7a18e9 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ControllerFilePathProvider.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ControllerFilePathProvider.java
@@ -37,6 +37,7 @@ public class ControllerFilePathProvider {
private static final String FILE_UPLOAD_TEMP_DIR = "fileUploadTemp";
private static final String UNTARRED_FILE_TEMP_DIR = "untarredFileTemp";
private static final String FILE_DOWNLOAD_TEMP_DIR = "fileDownloadTemp";
+ private static final String MULTIPART_TEMP_DIR = "multipartTemp";
private static ControllerFilePathProvider _instance;
@@ -56,6 +57,7 @@ public class ControllerFilePathProvider {
private final File _fileUploadTempDir;
private final File _untarredFileTempDir;
private final File _fileDownloadTempDir;
+ private final File _multiPartTempDir;
private final String _vip;
private ControllerFilePathProvider(ControllerConf controllerConf)
@@ -107,6 +109,11 @@ public class ControllerFilePathProvider {
LOGGER.info("File download temporary directory: {}",
_fileDownloadTempDir);
initDir(_fileDownloadTempDir);
+ // Backing store for the multipart parts that Jersey buffers to disk.
+ _multiPartTempDir = new File(localTempDir, MULTIPART_TEMP_DIR);
+ LOGGER.info("Multipart temporary directory: {}", _multiPartTempDir);
+ initDir(_multiPartTempDir);
+
_vip = controllerConf.generateVipUrl();
} catch (Exception e) {
throw new InvalidControllerConfigException("Caught exception while
initializing file upload path provider", e);
@@ -144,4 +151,9 @@ public class ControllerFilePathProvider {
org.apache.pinot.common.utils.FileUtils.ensureDirectoryExists(_fileDownloadTempDir.toPath());
return _fileDownloadTempDir;
}
+
+ public File getMultiPartTempDir() {
+
org.apache.pinot.common.utils.FileUtils.ensureDirectoryExists(_multiPartTempDir.toPath());
+ return _multiPartTempDir;
+ }
}
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
index d9deb410e9d..af7b6d930c1 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
@@ -463,6 +463,7 @@ public class PinotSegmentUploadDownloadRestletResource {
FileUtils.deleteQuietly(tempEncryptedFile);
FileUtils.deleteQuietly(tempDecryptedFile);
FileUtils.deleteQuietly(tempSegmentDir);
+ cleanupMultiPart(multiPart);
}
}
@@ -555,6 +556,7 @@ public class PinotSegmentUploadDownloadRestletResource {
} finally {
FileUtils.deleteQuietly(tempTarFile);
FileUtils.deleteQuietly(tempSegmentDir);
+ cleanupMultiPart(multiPart);
}
}
@@ -724,7 +726,7 @@ public class PinotSegmentUploadDownloadRestletResource {
}
} finally {
cleanupTempFiles(tempFiles);
- multiPart.cleanup();
+ cleanupMultiPart(multiPart);
}
return new SuccessResponse(String.format("Successfully uploaded segments:
%s of table: %s in %s ms",
@@ -737,6 +739,21 @@ public class PinotSegmentUploadDownloadRestletResource {
}
}
+ /// Releases the temporary files Jersey spilled the multipart body into.
Safe to call more than once, and safe on the
+ /// paths where the request carried no multipart body at all.
+ @VisibleForTesting
+ static void cleanupMultiPart(@Nullable FormDataMultiPart multiPart) {
+ if (multiPart == null) {
+ return;
+ }
+ try {
+ multiPart.cleanup();
+ } catch (Exception e) {
+ // Never let cleanup mask the outcome of the request it belongs to.
+ LOGGER.warn("Caught exception while cleaning up the multipart request",
e);
+ }
+ }
+
@VisibleForTesting
static String resolveDestinationTableName(@Nullable String requestTableName,
@Nullable String headerTableName,
@Nullable String metadataTableName, TableType tableType, HttpHeaders
headers,
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/ControllerAdminApiApplicationTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/ControllerAdminApiApplicationTest.java
new file mode 100644
index 00000000000..d74b21c1c82
--- /dev/null
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/ControllerAdminApiApplicationTest.java
@@ -0,0 +1,216 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.api;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.net.URI;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.List;
+import javax.ws.rs.Consumes;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.controller.ControllerConf;
+import org.apache.pinot.controller.api.resources.ControllerFilePathProvider;
+import org.apache.pinot.spi.env.PinotConfiguration;
+import org.apache.pinot.spi.filesystem.PinotFSFactory;
+import org.glassfish.jersey.internal.MapPropertiesDelegate;
+import org.glassfish.jersey.media.multipart.FormDataMultiPart;
+import org.glassfish.jersey.media.multipart.MultiPartFeature;
+import org.glassfish.jersey.media.multipart.MultiPartProperties;
+import org.glassfish.jersey.server.ApplicationHandler;
+import org.glassfish.jersey.server.ContainerRequest;
+import org.glassfish.jersey.server.ContainerResponse;
+import org.glassfish.jersey.server.ResourceConfig;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
+
+
+public class ControllerAdminApiApplicationTest {
+ private static final File DATA_DIR = new File(FileUtils.getTempDirectory(),
"ControllerAdminApiApplicationTest");
+ private static final File LOCAL_TEMP_DIR = new File(DATA_DIR, "localTemp");
+ private static final String BOUNDARY = "PinotMultiPartTempDirTestBoundary";
+
+ /// Comfortably past Jersey's default buffer threshold
(`ReaderWriter.BUFFER_SIZE`, 8 KB), so mimepull is forced to
+ /// spill the part to disk rather than keeping it in memory. A segment tar
is of course far larger still.
+ private static final int PART_SIZE_BYTES = 256 * 1024;
+
+ @BeforeMethod
+ public void setUp()
+ throws Exception {
+ FileUtils.deleteQuietly(DATA_DIR);
+ PinotFSFactory.init(new PinotConfiguration());
+ ControllerFilePathProvider.init(newControllerConf());
+ MultiPartProbeResource.reset();
+ }
+
+ @AfterMethod
+ public void tearDown() {
+ FileUtils.deleteQuietly(DATA_DIR);
+ }
+
+ private static ControllerConf newControllerConf() {
+ ControllerConf controllerConf = new ControllerConf();
+ controllerConf.setControllerHost("localhost");
+ controllerConf.setControllerPort("12345");
+ controllerConf.setDataDir(DATA_DIR.getPath());
+ controllerConf.setLocalTempDir(LOCAL_TEMP_DIR.getPath());
+ return controllerConf;
+ }
+
+ /// Jersey spills large multipart parts to disk and abandons them when a
request fails to parse, so they must land
+ /// somewhere the controller clears on restart rather than in java.io.tmpdir.
+ @Test
+ public void testMultiPartTempDirResolvesToControllerTempDir() {
+ MultiPartProperties properties =
+ new
ControllerAdminApiApplication.MultiPartTempDirResolver().getContext(getClass());
+
+ assertNotNull(properties);
+ assertEquals(properties.getTempDir(),
+
ControllerFilePathProvider.getInstance().getMultiPartTempDir().getAbsolutePath());
+ assertEquals(properties.getTempDir(), new File(LOCAL_TEMP_DIR,
"multipartTemp").getAbsolutePath());
+ }
+
+ /// The load-bearing test: drives a real multipart request through a real
`ApplicationHandler` with
+ /// `MultiPartFeature` registered, and asserts the part was actually spilled
into the controller's directory.
+ ///
+ /// Asserting that `Providers` merely hands back the resolver would not
prove much — Jersey's multipart reader is
+ /// what has to find it, and it does so once, in its own constructor. This
exercises registration, the `Providers`
+ /// lookup, the resulting `MIMEConfig`, and mimepull's `createTempFile` in
one go.
+ @Test
+ public void testJerseySpillsMultiPartBodiesIntoControllerTempDir()
+ throws Exception {
+ File multiPartTempDir =
ControllerFilePathProvider.getInstance().getMultiPartTempDir();
+
+ ContainerResponse response = postMultiPart(newHandler());
+
+ assertEquals(response.getStatus(), 200);
+
assertTrue(MultiPartProbeResource.getObservedSpillFiles().stream().anyMatch(name
-> name.startsWith("MIME")),
+ "Jersey did not spill the multipart body into " + multiPartTempDir +
", it saw: "
+ + MultiPartProbeResource.getObservedSpillFiles());
+ }
+
+ /// The resolver runs once at startup and mimepull holds that path for the
life of the controller, so a directory
+ /// that disappears underneath a running controller would otherwise fail
every upload until a restart.
+ @Test
+ public void testMultiPartUploadSurvivesTempDirDeletion()
+ throws Exception {
+ ApplicationHandler handler = newHandler();
+ File multiPartTempDir =
ControllerFilePathProvider.getInstance().getMultiPartTempDir();
+
+ // Stand in for a tmp sweeper, or an operator clearing
controller.local.temp.dir, removing it mid-flight
+ FileUtils.deleteDirectory(multiPartTempDir);
+ assertFalse(multiPartTempDir.exists());
+
+ ContainerResponse response = postMultiPart(handler);
+
+ assertEquals(response.getStatus(), 200, "Upload failed after the multipart
temporary directory was deleted");
+
assertTrue(MultiPartProbeResource.getObservedSpillFiles().stream().anyMatch(name
-> name.startsWith("MIME")),
+ "The multipart temporary directory was not re-created, spill files
seen: "
+ + MultiPartProbeResource.getObservedSpillFiles());
+ }
+
+ /// Guards the registration itself: without it the tests above still pass
while Jersey quietly keeps spilling parts
+ /// into java.io.tmpdir.
+ @Test
+ public void testAdminApplicationRegistersTheMultiPartProviders() {
+ ControllerAdminApiApplication application = new
ControllerAdminApiApplication(newControllerConf());
+
+ assertTrue(
+ application.getInstances().stream()
+ .anyMatch(instance -> instance instanceof
ControllerAdminApiApplication.MultiPartTempDirResolver),
+ "The admin application must register a MultiPartProperties resolver,
otherwise Jersey buffers multipart "
+ + "uploads into java.io.tmpdir where orphaned parts are never
reclaimed");
+ assertTrue(
+ application.getInstances().stream()
+ .anyMatch(instance -> instance instanceof
ControllerAdminApiApplication.MultiPartTempDirGuard),
+ "The admin application must register the multipart temporary directory
guard, otherwise a directory removed "
+ + "at runtime fails every upload until the controller restarts");
+ }
+
+ private static ApplicationHandler newHandler() {
+ ResourceConfig resourceConfig = new ResourceConfig();
+ resourceConfig.register(MultiPartFeature.class);
+ resourceConfig.register(new
ControllerAdminApiApplication.MultiPartTempDirResolver());
+ resourceConfig.register(new
ControllerAdminApiApplication.MultiPartTempDirGuard());
+ resourceConfig.register(MultiPartProbeResource.class);
+ return new ApplicationHandler(resourceConfig);
+ }
+
+ private static ContainerResponse postMultiPart(ApplicationHandler handler)
+ throws Exception {
+ ContainerRequest request =
+ new ContainerRequest(URI.create("http://localhost/";),
URI.create("http://localhost/probe";), "POST", null,
+ new MapPropertiesDelegate(), handler.getConfiguration());
+ request.getHeaders().add(HttpHeaders.CONTENT_TYPE,
MediaType.MULTIPART_FORM_DATA + "; boundary=" + BOUNDARY);
+ request.setEntityStream(new ByteArrayInputStream(multiPartBody()));
+ return handler.apply(request).get();
+ }
+
+ private static byte[] multiPartBody()
+ throws Exception {
+ byte[] payload = new byte[PART_SIZE_BYTES];
+ Arrays.fill(payload, (byte) 'x');
+
+ ByteArrayOutputStream body = new ByteArrayOutputStream();
+ body.write(("--" + BOUNDARY + "\r\n"
+ + "Content-Disposition: form-data; name=\"segment\";
filename=\"segment.tar.gz\"\r\n"
+ + "Content-Type:
application/octet-stream\r\n\r\n").getBytes(StandardCharsets.UTF_8));
+ body.write(payload);
+ body.write(("\r\n--" + BOUNDARY +
"--\r\n").getBytes(StandardCharsets.UTF_8));
+ return body.toByteArray();
+ }
+
+ /// Reports what is sitting in the multipart temporary directory while the
request is still in flight, which is the
+ /// only window in which the spilled part is observable — `CloseableService`
deletes it once the request ends.
+ @Path("/")
+ public static class MultiPartProbeResource {
+ private static volatile List<String> _observedSpillFiles = List.of();
+
+ static void reset() {
+ _observedSpillFiles = List.of();
+ }
+
+ static List<String> getObservedSpillFiles() {
+ return _observedSpillFiles;
+ }
+
+ @POST
+ @Path("probe")
+ @Consumes(MediaType.MULTIPART_FORM_DATA)
+ @Produces(MediaType.TEXT_PLAIN)
+ public String probe(FormDataMultiPart multiPart) {
+ String[] children =
ControllerFilePathProvider.getInstance().getMultiPartTempDir().list();
+ _observedSpillFiles = children == null ? List.of() : List.of(children);
+ return String.valueOf(multiPart.getBodyParts().size());
+ }
+ }
+}
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/ControllerFilePathProviderTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/ControllerFilePathProviderTest.java
index 3ec6c85ef81..1fcf3e158f0 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/ControllerFilePathProviderTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/ControllerFilePathProviderTest.java
@@ -20,6 +20,7 @@ package org.apache.pinot.controller.api;
import java.io.File;
import java.net.URI;
+import java.nio.charset.StandardCharsets;
import org.apache.commons.io.FileUtils;
import org.apache.pinot.controller.ControllerConf;
import org.apache.pinot.controller.api.resources.ControllerFilePathProvider;
@@ -28,6 +29,7 @@ import org.apache.pinot.spi.filesystem.PinotFSFactory;
import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertTrue;
@@ -68,6 +70,10 @@ public class ControllerFilePathProviderTest {
assertEquals(fileDownloadTempDir, new File(LOCAL_TEMP_DIR,
"fileDownloadTemp"));
checkDirExistAndEmpty(fileDownloadTempDir);
+ File multiPartTempDir = provider.getMultiPartTempDir();
+ assertEquals(multiPartTempDir, new File(LOCAL_TEMP_DIR, "multipartTemp"));
+ checkDirExistAndEmpty(multiPartTempDir);
+
assertEquals(provider.getVip(), "http://localhost:12345";);
FileUtils.forceDelete(DATA_DIR);
@@ -102,6 +108,10 @@ public class ControllerFilePathProviderTest {
assertEquals(fileDownloadTempDir, new File(DATA_DIR,
"localhost_12345/fileDownloadTemp"));
checkDirExistAndEmpty(fileDownloadTempDir);
+ File multiPartTempDir = provider.getMultiPartTempDir();
+ assertEquals(multiPartTempDir, new File(DATA_DIR,
"localhost_12345/multipartTemp"));
+ checkDirExistAndEmpty(multiPartTempDir);
+
assertEquals(provider.getVip(), "http://localhost:12345";);
FileUtils.forceDelete(DATA_DIR);
@@ -133,9 +143,14 @@ public class ControllerFilePathProviderTest {
assertEquals(fileDownloadTempDir, new File(LOCAL_TEMP_DIR,
"fileDownloadTemp"));
checkDirExistAndEmpty(fileDownloadTempDir);
+ File multiPartTempDir = provider.getMultiPartTempDir();
+ assertEquals(multiPartTempDir, new File(LOCAL_TEMP_DIR, "multipartTemp"));
+ checkDirExistAndEmpty(multiPartTempDir);
+
FileUtils.deleteQuietly(fileUploadTempDir);
FileUtils.deleteQuietly(untarredFileTempDir);
FileUtils.deleteQuietly(fileDownloadTempDir);
+ FileUtils.deleteQuietly(multiPartTempDir);
fileUploadTempDir = provider.getFileUploadTempDir();
assertEquals(fileUploadTempDir, new File(LOCAL_TEMP_DIR,
"fileUploadTemp"));
@@ -148,6 +163,39 @@ public class ControllerFilePathProviderTest {
fileDownloadTempDir = provider.getFileDownloadTempDir();
assertEquals(fileDownloadTempDir, new File(LOCAL_TEMP_DIR,
"fileDownloadTemp"));
checkDirExistAndEmpty(fileDownloadTempDir);
+
+ multiPartTempDir = provider.getMultiPartTempDir();
+ assertEquals(multiPartTempDir, new File(LOCAL_TEMP_DIR, "multipartTemp"));
+ checkDirExistAndEmpty(multiPartTempDir);
+ }
+
+ /// The multipart directory exists so that parts Jersey orphans on a failed
parse are reclaimed on restart rather
+ /// than accumulating in java.io.tmpdir, so the startup clean is the
behavior that matters.
+ @Test
+ public void testStaleMultiPartFilesClearedOnInit()
+ throws Exception {
+ FileUtils.deleteQuietly(DATA_DIR);
+ PinotFSFactory.init(new PinotConfiguration());
+
+ ControllerConf controllerConf = new ControllerConf();
+ controllerConf.setControllerHost(HOST);
+ controllerConf.setControllerPort(PORT);
+ controllerConf.setDataDir(DATA_DIR.getPath());
+ controllerConf.setLocalTempDir(LOCAL_TEMP_DIR.getPath());
+ ControllerFilePathProvider.init(controllerConf);
+
+ // Stand in for a part Jersey spilled to disk and then abandoned when the
request failed to parse
+ File orphan = new
File(ControllerFilePathProvider.getInstance().getMultiPartTempDir(),
"MIME1234567890");
+ FileUtils.writeStringToFile(orphan, "orphaned part",
StandardCharsets.UTF_8);
+ assertTrue(orphan.exists());
+
+ // Restart
+ ControllerFilePathProvider.init(controllerConf);
+
+ assertFalse(orphan.exists());
+
checkDirExistAndEmpty(ControllerFilePathProvider.getInstance().getMultiPartTempDir());
+
+ FileUtils.forceDelete(DATA_DIR);
}
private void checkDirExistAndEmpty(File dir) {
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResourceTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResourceTest.java
index e3f6fb0f4b6..ef90451103c 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResourceTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResourceTest.java
@@ -57,7 +57,10 @@ import org.testng.annotations.BeforeClass;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.testng.Assert.assertEquals;
@@ -290,6 +293,42 @@ public class PinotSegmentUploadDownloadRestletResourceTest
{
.collect(Collectors.toSet());
}
+ @Test
+ public void testCleanupMultiPartIsNullSafeAndSwallowsFailures() {
+ // The URI upload path carries no multipart body at all
+ PinotSegmentUploadDownloadRestletResource.cleanupMultiPart(null);
+
+ // Cleanup runs in a finally block, so a failure there must never replace
the exception that got us there
+ FormDataMultiPart throwing = mock(FormDataMultiPart.class);
+ doThrow(new RuntimeException("cleanup blew up")).when(throwing).cleanup();
+ PinotSegmentUploadDownloadRestletResource.cleanupMultiPart(throwing);
+ verify(throwing).cleanup();
+ }
+
+ @Test
+ public void testCleanupMultiPartReleasesSpilledParts()
+ throws IOException {
+ // Stand in for the file Jersey spills a large part into;
BodyPartEntity#cleanup deletes exactly this
+ File spilled = new File(_tempDir, "MIME1234567890");
+ FileUtils.touch(spilled);
+
+ FormDataBodyPart bodyPart = mock(FormDataBodyPart.class);
+ doAnswer(invocation -> {
+ FileUtils.deleteQuietly(spilled);
+ return null;
+ }).when(bodyPart).cleanup();
+
+ FormDataMultiPart multiPart = new FormDataMultiPart();
+ multiPart.getBodyParts().add(bodyPart);
+
+ PinotSegmentUploadDownloadRestletResource.cleanupMultiPart(multiPart);
+ Assert.assertFalse(spilled.exists());
+
+ // The request-scoped CloseableService closes the same multipart again at
the end of the request
+ PinotSegmentUploadDownloadRestletResource.cleanupMultiPart(multiPart);
+ verify(bodyPart, times(2)).cleanup();
+ }
+
@Test
public void testGetSegmentSizeFromFile()
throws IOException {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
