Skip to content

[core] Fix snapshot lookup for duplicate commit timestamps #9437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ArnavBalyan wants to merge 1 commit into
base: master
Choose a base branch
from
+32 −4
Open

[core] Fix snapshot lookup for duplicate commit timestamps #9437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions paimon-core/src/main/java/org/apache/paimon/utils/SnapshotManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,8 @@ public long repairEarliestSnapshot(long snapshotId) {
earliest = mid + 1; // Search in the right half
finalSnapshot = snapshot;
} else {
finalSnapshot = snapshot; // Found the exact match
break;
finalSnapshot = snapshot;
earliest = mid + 1;

@JingsongLi JingsongLi Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Refresh the upper bound before searching past an exact match

latest is only an ID captured before the search. A concurrent rollback can update the latest hint and delete that snapshot after latestSnapshotId() returns. With snapshots 0/1/2 sharing this timestamp, if rollback removes snapshot 2 after we capture it, the first probe hits snapshot 1; this new continuation then probes snapshot 2 and throws Snapshot file ... does not exist, although snapshot 1 is still the correct answer. The old break returned snapshot 1.

Please resolve the upper boundary through the existing live-snapshot path (for example, latestSnapshot() plus an early boundary return) and add a rollback-race regression test.

}
}
return finalSnapshot;
Expand Down Expand Up @@ -415,8 +415,8 @@ public long repairEarliestSnapshot(long snapshotId) {
} else if (commitTime < timestampMills) {
earliest = mid + 1; // Search in the right half
} else {
finalSnapshot = snapshot; // Found the exact match
break;
finalSnapshot = snapshot;
latest = mid - 1;

@JingsongLi JingsongLi Aug 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Refresh the lower bound before searching past an exact match

earliest can become stale when expiration runs between earliestSnapshotId() and the binary search. With snapshots 0/1/2 sharing this timestamp, expiration can delete snapshot 0 after the ID is read; the search first hits snapshot 1, then this new continuation probes snapshot 0 and fails even though snapshot 1 is the earliest surviving match. The old break returned snapshot 1.

Please obtain the lower bound through earliestSnapshot(latest), which already retries concurrent earliest deletion, return it directly when it satisfies the query, and cover this race in the test.

}
}
return finalSnapshot;
Expand Down
28 changes: 28 additions & 0 deletions paimon-core/src/test/java/org/apache/paimon/utils/SnapshotManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,34 @@ public void testLaterOrEqualTimeMills() throws IOException {
assertThat(snapshotManager.laterOrEqualTimeMills(millis + 10001)).isNull();
}

@Test
public void testEarlierOrEqualTimeMillsWithDuplicateCommitTimes() throws IOException {
long millis = 1684726826L;
FileIO localFileIO = LocalFileIO.create();
SnapshotManager snapshotManager =
newSnapshotManager(localFileIO, new Path(tempDir.toString()));
for (long i = 0; i < 3; i++) {
Snapshot snapshot = createSnapshotWithMillis(i, millis);
localFileIO.tryToWriteAtomic(snapshotManager.snapshotPath(i), snapshot.toJson());
}

assertThat(snapshotManager.earlierOrEqualTimeMills(millis).id()).isEqualTo(2);
}

@Test
public void testLaterOrEqualTimeMillsWithDuplicateCommitTimes() throws IOException {
long millis = 1684726826L;
FileIO localFileIO = LocalFileIO.create();
SnapshotManager snapshotManager =
newSnapshotManager(localFileIO, new Path(tempDir.toString()));
for (long i = 0; i < 3; i++) {
Snapshot snapshot = createSnapshotWithMillis(i, millis);
localFileIO.tryToWriteAtomic(snapshotManager.snapshotPath(i), snapshot.toJson());
}

assertThat(snapshotManager.laterOrEqualTimeMills(millis).id()).isEqualTo(0);
}

@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testLaterOrEqualWatermark(boolean isRaceCondition) throws IOException {
Expand Down
Loading