JingsongLi commented on code in PR #10625:
URL: https://github.com/apache/paimon/pull/10625#discussion_r4346639206
##########
paimon-core/src/main/java/org/apache/paimon/utils/TagManager.java:
##########
@@ -132,11 +161,127 @@ public void createTag(
List<TagCallback> callbacks,
boolean ignoreIfExists) {
checkArgument(!StringUtils.isNullOrWhitespaceOnly(tagName), "Tag name
shouldn't be blank.");
- if (tagExists(tagName)) {
- checkArgument(ignoreIfExists, "Tag '%s' already exists.", tagName);
+ // The existence check and metadata write must not race with snapshot
expiration, which
+ // deletes files based on a tag set collected under the same lock.
+ ReentrantLock coordinationLock = coordinationLock();
+ coordinationLock.lock();
+ try {
+ if (tagExists(tagName)) {
+ checkArgument(ignoreIfExists, "Tag '%s' already exists.",
tagName);
+ return;
Review Comment:
[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.
--
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]
