[s3] Avoid ListBucket requirement in snapshot commit path#8263
Conversation
|
I think this still breaks commit conflict handling on object stores. The latest snapshot is read before acquiring the commit lock, and |
|
Thanks for catching this.The previous object-store branch could skip the stale snapshot guard and overwrite an existing I updated the PR to keep the no-overwrite conflict signal without requiring
Added regression tests for the stale commit conflict path and S3 412 handling. Verified with focused Maven tests, and with Ceph RGW using a temporary user without |
| // On object stores, probing a missing object can require list permission. Trust the | ||
| // committed hint and let the commit retry path handle a concurrently newer snapshot. | ||
| if (fileIO.isObjectStore()) { | ||
| return snapshotId; |
There was a problem hiding this comment.
This still falls back to listing when LATEST is absent (for example on the first commit of a new table, or after the hint file is lost). FileStoreCommitImpl calls snapshotManager.latestSnapshot() before it writes snapshot-1, so with an S3 user that has GetObject/PutObject but no ListBucket, an empty table will fail here before the new conditional PUT path is reached. The new object-store test writes LATEST first, so it does not cover this case. For object stores we need a no-list path for a missing hint as well, such as treating the missing hint as no latest snapshot and relying on the no-overwrite snapshot write to reject an unexpected existing snapshot-1.
Purpose
Fixes #8261.
Avoid requiring
ListBucketfor S3-compatible object-store snapshot commits, while preserving the stale snapshot conflict handling.The change skips the
exists(snapshot-N)probe on object stores, but writessnapshot-Nwith no-overwrite semantics. For S3 this usesIf-None-Match: *, and maps412 PreconditionFailedtoFileAlreadyExistsException, so stale commits retry instead of overwriting existing snapshot metadata.Non-object-store behavior is unchanged.
Tests
Added regression tests for:
exists(snapshot-N)snapshot-N412 PreconditionFailedis treated as the file-exists conflict signalVerified with:
Also verified against Ceph RGW with a temporary user without
ListBucket: first conditional PUT succeeds, second returns 412, and the original object