Vamsi-klu opened a new pull request, #19400:
URL: https://github.com/apache/pinot/pull/19400
Fixes #12264
Please tag `bug`. No `release-notes` (existing keys only, no new config).
## What's the problem
`generateTmpSegmentFileName` appended `UUID.randomUUID()`. A Pinot-level
HOLD then retry of `uploadSegment` minted a new deep-store object each attempt.
Leftover temps sat until async cleanup.
## What's the solution
Destination is `{segment}.tmp.upload`. `isTmpFile` accepts last token
`upload` or a UUID so mixed-version leftovers still clean up. Keep
temp-then-rename. Do not upload to the final deep-store name. Do not cancel the
timed-out Future (last-writer-wins on overlap is accepted).
## Why this way
Jackie wanted to keep temp-then-rename. @jadami10 asked to drop the UUID and
keep a deterministic temp. Unique attempt URIs plus atomic move plus
Future.cancel would be a larger change than this issue.
## How it is implemented
- `SegmentCompletionUtils.generateTmpSegmentFileName` now returns
`{segment}.tmp.upload`.
- `isTmpFile` is true for `.tmp.upload` and for the old `.tmp.<uuid>` form.
- `PinotFSSegmentUploader` still uploads to the temp URI, then the existing
commit path renames.
## Impact
Retries overwrite one object. Old servers still emit UUID temps. New
`isTmpFile` accepts both.
Existing keys, unchanged:
- `controller.realtime.segment.tmpFileAsyncDeletionEnabled` (default false)
- `controller.realtime.segment.tmpFileRetentionInSeconds` (default 3600)
No new keys.
## Test plan
- [x] `SegmentCompletionUtilsTest`: same dest twice, `isTmpFile` for
`.tmp.upload` and leftover UUID
- [x] `PinotFSSegmentUploaderTest.testRetryUsesSameDestUri`
- [x] `PinotLLCRealtimeSegmentManagerTest`: plants `.tmp.upload` and a
leftover UUID
- [x] `./mvnw spotless:apply checkstyle:check license:format license:check
-pl pinot-core,pinot-controller`
```
./mvnw -pl pinot-core
-Dtest=SegmentCompletionUtilsTest,PinotFSSegmentUploaderTest
-Dsurefire.failIfNoSpecifiedTests=false test
./mvnw -pl pinot-controller -Dtest=PinotLLCRealtimeSegmentManagerTest
-Dsurefire.failIfNoSpecifiedTests=false test
```
cc @jadami10 @Jackie-Jiang @mcvsubbu @sajjad-moradi @xiangfu0
##### Was generative AI tooling used to co-author this PR?
- [x] Yes
Made with [Cursor](https://cursor.com)
--
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]
