deepthi912 opened a new pull request, #19455:
URL: https://github.com/apache/pinot/pull/19455
## Summary
- Fixes a temp-file leak on the controller: Jersey/mimepull `MIME*.tmp`
files under `java.io.tmpdir` accumulate on failure paths of three segment
upload handlers (`uploadSegment`, `uploadReingestedSegment`, and the pre-try
section of `uploadSegments`) in `PinotSegmentUploadDownloadRestletResource`.
- Root cause: the outer `finally` blocks of these handlers deleted only
controller-owned tempfiles (`tempEncryptedFile`, `tempDecryptedFile`,
`tempSegmentDir`) but did not call `multiPart.cleanup()`. Cleanup was only
invoked inside `createSegmentFileFromMultipart`, which is not reached on:
- Guard-clause throws (missing `DOWNLOAD_URI`, wrong upload type,
`!copySegmentToFinalLocation` with empty source URI)
- `case URI` when a client sends an unexpected multipart body
- `default` case (unrecognized upload type)
- Pre-try validation throws in `uploadSegments` (missing tableName,
tableType, tableConfig, wrong upload type)
- Real-world symptom: controllers accumulate `MIME<random>.tmp` files (~260
MB each for TAR-mode segment uploads) until the `java.io.tmpdir` volume fills
up. Subsequent uploads then return `500 IOException: No space left on device`,
which stalls realtime segment commits.
### What this fix does
- Adds `if (multiPart != null) multiPart.cleanup();` to the outer `finally`
of `uploadSegment` and `uploadReingestedSegment`.
- Wraps `uploadSegments` in an outer `try/finally` so pre-try validation
throws are also covered; removes the duplicate `multiPart.cleanup()` from the
inner `finally` to avoid double invocation.
- Removes `multiPart.cleanup()` from `createSegmentFileFromMultipart` and
adds a comment: the outer handler now owns the multipart lifecycle.
`MIMEPart.close()` is idempotent in Jersey 2.45, but relying on double-cleanup
couples the code to that internal detail; single-owner is cleaner.
### What this fix does NOT catch
Leak paths that no per-handler `finally` can cover:
- JVM/pod termination mid-upload (no `finally` runs).
- `IOException` from mimepull itself while spooling the body to disk
(handler is never invoked).
- Client disconnect during upload.
These need a separate startup-time reaper for stale `MIME*.tmp` — out of
scope for this PR.
## Test plan
- [ ] Unit tests covering the three fixed failure paths (`uploadSegment`
guard-clause throws, `uploadReingestedSegment` guard-clause throws,
`uploadSegments` pre-try throws). Follow the pattern from #19377
(`testCreateSegmentsMetadataInfoMapRegistersTempFilesForCleanup`) using
`listTempEntries(...)` before/after snapshots.
- [ ] Local repro of `MIME*.tmp` accumulation with the fix applied: force a
`DOWNLOAD_URI` missing 400 in a loop, verify no residue in `java.io.tmpdir`.
- [ ] `./mvnw checkstyle:check -pl pinot-controller` passes.
- [ ] `./mvnw test -pl pinot-controller
-Dtest=PinotSegmentUploadDownloadRestletResourceTest` passes.
--
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]
