Skip to content

[s3] Avoid ListBucket requirement in snapshot commit path#8263

Open
hhhizzz wants to merge 3 commits into
apache:masterfrom
hhhizzz:fix-s3-snapshot-access
Open

[s3] Avoid ListBucket requirement in snapshot commit path#8263
hhhizzz wants to merge 3 commits into
apache:masterfrom
hhhizzz:fix-s3-snapshot-access

Conversation

@hhhizzz

@hhhizzz hhhizzz commented Jun 17, 2026

Copy link
Copy Markdown

Purpose

Fixes #8261.

Avoid requiring ListBucket for 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 writes snapshot-N with no-overwrite semantics. For S3 this uses If-None-Match: *, and maps 412 PreconditionFailed to FileAlreadyExistsException, so stale commits retry instead of overwriting existing snapshot metadata.

Non-object-store behavior is unchanged.

Tests

Added regression tests for:

  • object-store snapshot commit without probing exists(snapshot-N)
  • stale object-store commit conflict does not overwrite an existing snapshot-N
  • S3 no-overwrite snapshot write uses direct PUT
  • S3 412 PreconditionFailed is treated as the file-exists conflict signal

Verified with:

mvn -nsu -pl paimon-core -am -DskipITs -Dcheckstyle.skip -Dspotless.check.skip=true -Drat.skip=true -DfailIfNoTests=false -Dtest=RenamingSnapshotCommitTest,HintFileUtilsTest test
mvn -nsu -pl paimon-filesystems/paimon-s3-impl -am -DskipITs -Dcheckstyle.skip -Dspotless.check.skip=true -Drat.skip=true -DfailIfNoTests=false -Dtest=S3FileIOTest test

Also verified against Ceph RGW with a temporary user without ListBucket: first conditional PUT succeeds, second returns 412, and the original object

@JingsongLi

Copy link
Copy Markdown
Contributor

I think this still breaks commit conflict handling on object stores. The latest snapshot is read before acquiring the commit lock, and newSnapshotPath is computed from that value. Previously, the !fileIO.exists(newSnapshotPath) check inside lock.runWithLock was the stale-snapshot guard: if another writer committed the same snapshot id while this writer was waiting for the lock, this returned false and FileStoreCommitImpl retried with the newer latest snapshot. With the new object-store branch, we skip that check and then write snapshot-N with overwrite=true, so two serialized object-store commits that both planned snapshot N can both return success, with the second one replacing the first snapshot file. That can lose a committed snapshot even when the external lock is working. We need to preserve the no-overwrite/conflict signal for an existing snapshot file without requiring ListBucket, rather than blindly overwriting snapshot metadata on object stores.

@hhhizzz

hhhizzz commented Jun 19, 2026

Copy link
Copy Markdown
Author

Thanks for catching this.The previous object-store branch could skip the stale snapshot guard and overwrite an existing snapshot-N.

I updated the PR to keep the no-overwrite conflict signal without requiring ListBucket:

  • object stores still skip exists(snapshot-N)
  • snapshot-N is written with no-overwrite semantics
  • S3 uses If-None-Match: * for that write
  • 412 PreconditionFailed is mapped to FileAlreadyExistsException, so the commit returns false and retries instead of overwriting

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 ListBucket: first conditional PUT succeeds, second returns 412, and the original object is preserved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] S3 snapshot commit requires ListBucket due to missing-object status checks

2 participants