KKcorps commented on code in PR #19627:
URL: https://github.com/apache/pinot/pull/19627#discussion_r4078688547
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/ControllerAdminApiApplication.java:
##########
@@ -233,4 +242,30 @@ public void dispose(ExecutorService executorService) {
// 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.
+ ///
+ /// Resolved lazily: the admin application is constructed before
[ControllerFilePathProvider] is initialized, but
+ /// Jersey does not build the multipart reader until the first multipart
request arrives.
+ @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());
Review Comment:
**Minor comment: Recreate the spill directory before each multipart request**
Jersey calls this resolver when constructing its multipart reader and caches
the path in `MIMEConfig`. If an OS cleanup or operator later deletes
`multipartTemp` or its parent, subsequent uploads that spill to disk fail with
`NoSuchFileException` before reaching the resource. `getMultiPartTempDir()` is
never called again to recreate the directory, and the catch here cannot handle
those later parsing failures.
This regresses the deleted-temp-directory recovery added in #14503. The
existing `testCreateSegmentFileFromMultipart` covers deleting the parent
directory, but supplies a mocked multipart and therefore bypasses this parsing
stage.
Please ensure the directory exists for each multipart request **before
entity parsing**, for example in a request filter, and add a real multipart
regression test that spills successfully, deletes the directory or parent, and
succeeds again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
