[mvanhorn]

Purpose

A tag created while snapshots are expiring can return success and then fail to read. Time travel through that tag hits a missing file, because manifest and data files it still references were deleted after the tag write. ExpireSnapshotsImpl records tagged snapshots once and reuses that fixed list for data-file and manifest cleanup, deleting snapshot metadata only afterward. TagManager.createTag writes the tag on its own, with no shared lock or later recheck, so a tag committed after that collection is left out of the skip set. A snapshot-existence check around the write still passes, because the snapshot file is removed only after those referenced files are already gone.

Each table path and branch has one reentrant lock, stored in a static map and shared by every TagManager for that key. ExpireSnapshotsImpl holds it from collectTaggedSnapshots through snapshot deletion, and TagManager.createTag holds it across the existence check, ensureSnapshotRetained, and the tag metadata write. ensureSnapshotRetained allows the write when the snapshot still exists or an existing tag already retains that snapshot id; otherwise it throws SnapshotNotExistException and writes no tag. Both paths release the lock in a finally block, including when expiration or the tag write fails.

Fixes #9220

Tests

mvn -B -ntp -pl paimon-core -am test -Dtest=ExpireSnapshotsTest -Dsurefire.failIfNoSpecifiedTests=false finished with ExpireSnapshotsTest at 41 tests, 0 failures, 0 errors, and 0 skipped.
The tests in paimon-core/src/test/java/org/apache/paimon/operation/ExpireSnapshotsTest.java fail without the fix in the paimon ExpireSnapshotsImpl change (checked by reverting the source changes).

[JingsongLi]

Requirement fit: PIVOT. The tag-retention race is real and worth fixing, but the current coordination does not cover normal independent writer/tag jobs. Implementation: FINDINGS.

Reviewed head 267636401eaf9e7d6fedfb3b22be668be0f4c9e7 in an isolated worktree. Local JDK 8 verification: ExpireSnapshotsTest, TagManagerTest, and TagTest passed (47 tests). A separate-JVM reproduction still leaves a successfully created tag unreadable, as detailed inline.

I also reproduced the CI failure locally with TagAutoManagerTest#testExpireTagsByTimeRetained: SnapshotNotExistException at line 447 for synthetic snapshot #5. Please update that fixture to provide a retained snapshot under the new validation, so its TTL behavior remains tested. Both Core JDK 8 and JDK 11 CI jobs currently fail on this test.

[JingsongLi]

[P1] Coordinate tag creation and expiration across JVMs

This static map only shares locks within one JVM/classloader. A continuously running writer expires snapshots in its JVM, while the documented standalone flink run ... create_tag action or a separate Spark/Java client creates tags in another JVM. Those operations take independent locks, and ensureSnapshotRetained still succeeds because the snapshot metadata is deleted after its data/manifests.

I reproduced this on this head using the existing expiration test fixture: pause expiration via setAfterTagCollection, create a tag for snapshot #2 in a separate Java process, then resume expiration. The child reports success; afterward tagExists=true, snapshotExists=false, deltaManifestExists=false, and reading the tag fails with Failed to read manifest-list-.... Thus the PR still permits the data-retention failure it claims to fix in #9220. Please use coordination/GC protection shared by all participating processes and add a deterministic independent-JVM regression test; snapshot-existence checks alone cannot close this window.

[mvanhorn]

Reworked in 9fdbd05 so tag creation and expiration coordinate through the file system instead of an in-JVM lock.

• Expiration writes an expire marker before its final tag scan.
• After writing its tag, a creator checks that the snapshot or manifest list files still exist and that no expire marker covers the snapshot, and rolls the tag back otherwise. A tag written by another in-flight creator no longer counts as protection.
• Snapshot files are still deleted at the end, as before.

Tests: testSeparateJvmTagCreatedDuringExpirationKeepsFiles creates a tag from a child JVM while expiration runs, and testConcurrentCreatorsAfterFinalTagScanRollBack runs two child-JVM creators after the final scan and checks both roll back. The TagAutoManagerTest TTL fixture is repaired too. The tag, expire and snapshot tests in paimon-core pass on JDK 11.

One case this does not cover: two expirations running at once on the same table can share a marker. I kept that out of scope since a table normally has one expirer, but can add it if you want.

[JingsongLi]

Re-reviewed the revised filesystem-marker protocol on this head. The retention problem has end-to-end value, and all 67 selected ExpireSnapshots/TagManager/Tag/TagAutoManager tests passed on JDK 8, including the new separate-JVM tests. Two additional executable interleavings still leave an unreadable tag: overlapping expirers can remove each other's fences, and a transient I/O failure during post-publication validation leaves an unvalidated tag that a retry accepts. Details below.

[JingsongLi]

[P1] Preserve fences owned by other active expirations

This deletes a marker that may belong to another still-running expirer. I reproduced this with three independent JDK 8 JVMs: A marks snapshot 2 and pauses after its final tag scan; C publishes a tag for 2 and pauses before its post-write check; B sees C's tag, deletes A's marker here, preserves the tag's files, and successfully expires [1,3). C then sees no marker and surviving manifests, so tag creation succeeds. A resumes with its earlier tag set and deletes those manifests, leaving the successfully created tag unreadable. Both expirations returned successfully, with no injected failure. Automatic writer expiration can overlap an independent Spark/Flink expire_snapshots call. Please serialize expirers across processes or give each run its own fence ownership, keeping creators fenced until every expirer that missed the tag has finished. The deletion at line 345 and cleanup of borrowed markers in finally also need the same ownership guarantee.

[JingsongLi]

[P1] Roll back a published tag when retention validation fails with I/O

Only SnapshotNotExistException removes the tag written at line 310. If the new post-write marker/manifest existence check throws IOException, ensureSnapshotRetained wraps it as UncheckedIOException and the published tag survives without completing the retention handshake. I reproduced this with a creator in a separate JVM started after the expirer’s final tag scan: inject one transient IOException on exists(expire-marker/2), let expiration finish, then retry createTag(..., ignoreIfExists=true). The first call fails but leaves tagExists=true; expiration deletes snapshot 2's manifests; the retry returns successfully because the tag already exists; reading that tag fails with a missing manifest-list. Auto/batch tag creators use ignoreIfExists=true. Please clean up this call's newly published tag on every failed post-write retention validation (preserving the original error), so a retry cannot acknowledge an unvalidated dangling tag.

[mvanhorn]

Thanks for the two interleavings. Both are fixed in b995317, which also merges in current master.

• Each expiration run now writes its own marker under expire-marker// and deletes only that one, in the cleanup path too. A creator rolls its tag back while any run's marker is still there, so in your three-JVM case B can no longer clear A's fence.
• createTag now deletes the tag it just published when the post-write check fails for any reason, IOException included, and rethrows the original error, so a retry with ignoreIfExists=true does not find a leftover tag.

New tests: testExpireKeepsAnotherExpirerFence and testCreateTagRollsBackWhenMarkerCheckFails. ExpireSnapshotsTest, TagAutoManagerTest, TagManagerTest and TagTest pass on JDK 11 (69 tests), and spotless and checkstyle pass on paimon-core.

One limit: an expirer that crashes before its cleanup leaves its marker behind, and that marker keeps fencing the snapshot until it is removed.

[JingsongLi]

Requirement fit: SUPPORTED. Protecting concurrently created tags from snapshot expiration has clear retention and time-travel value. Owner-scoped markers and rollback on marker-check failure address the previously reported cases.
Implementation: FINDINGS.

On this exact head, Java 8 ExpireSnapshotsTest, TagManagerTest, TagTest and TagAutoManagerTest all pass: 69 tests, zero skips, normal checks enabled. A new three-process real-filesystem reproduction still finds the same-name idempotent retry window below. It runs actual ExpireSnapshotsImpl expiration with two independent creator JVMs and injects only a scheduling pause after successful tag publication, with no I/O failure or fabricated metadata.

The current Spark Scala 2.13 CI failure is a Maven Central connection-reset/artifact-resolution failure; this review does not claim that CI job has been rerun or resolved.

[JingsongLi]

[P2] Do not acknowledge another creator's provisional tag as committed

The added post-write validation/rollback makes a visible tag file provisional, but this ignoreIfExists branch still treats any visible file as completed. With expiration paused after its final tag scan, JVM A publishes tag-same and pauses before validation; JVM B calls createTag for the same name with ignoreIfExists=true and successfully returns here. A then resumes, detects the active expiration marker and deletes tag-same; actual expiration finishes and B's successfully acknowledged tag does not exist. I reproduced this with real table snapshots and two creator JVMs, without I/O errors. Coordinate the existing-tag path with validation/publication ownership so it cannot succeed on an in-flight tag that can still roll back, and add a same-name cross-process retry test.