Vamsi-klu commented on PR #19400:
URL: https://github.com/apache/pinot/pull/19400#issuecomment-5490208953
@Jackie-Jiang Thanks for the review. I agree the UUID is there so two
writers do not override each other.
You suggested uploading straight to the final deep-store key. I am not doing
that in this PR. That path still needs leftover-failed-object handling on the
final key, and the #10815 cleanup plus split-commit `move` are built around tmp
names. I would rather keep temp-then-rename.
What I am doing instead is `{segment}.tmp.{instanceId}`. Same-server
HOLD/retry overwrites one object. Two replicas stay on different keys.
`isTmpFile` still accepts leftover UUID temps. That bounds the S3 flood to one
object per server per segment without two replicas sharing a key.
I am also dropping the later generation-safe rewrite (uploadId,
single-flight, controller locks). That was more than you asked for. The
published tree still has that rewrite. I will replace it with the instance-id
temps.
Race still left open: a timed-out writer and a retry from the same server
are last-writer-wins on that server's key. I am not cancelling the first
Future. A controller move of a half-replaced blob is still possible. If you
want that closed, I will switch to uploading to the final location instead of
adding locks.
--
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]
