JingsongLi commented on code in PR #9437:
URL: https://github.com/apache/paimon/pull/9437#discussion_r3912284758
##########
paimon-core/src/main/java/org/apache/paimon/utils/SnapshotManager.java:
##########
@@ -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;
Review Comment:
[P2] Retry if rollback moves the upper bound after the first equal probe
Resolving `latestSnapshot` only closes the race before the binary search
starts. With `0@(t-1), 1@t, 2@t, 3@t, 4@(t+1)`, `earlierOrEqualTimeMills(t)`
first records snapshot 1. If `RollbackHelper` retains snapshot 1 at that point,
it commits `LATEST=1` and deletes snapshots 4 through 2. This branch advances
`earliest` to 2, so the next `snapshot(2)` throws even though snapshot 1 is
still the correct result. I reproduced this deterministically by triggering
rollback immediately after the first equal probe.
Please read probes through `tryGetSnapshot` and, when a post-match probe
disappears, refresh the live upper bound and restart with a bounded retry. A
regression test should trigger rollback after the first equal midpoint has been
read.
##########
paimon-core/src/main/java/org/apache/paimon/utils/SnapshotManager.java:
##########
@@ -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;
Review Comment:
[P2] Retry if expiration moves the lower bound after the first equal probe
Resolving the initial earliest snapshot does not cover expiration that
starts during the binary search. With `0@(t-1), 1@t, 2@t, 3@t, 4@(t+1)`,
`laterOrEqualTimeMills(t)` first records snapshot 2. If expiration then retains
snapshot 2 and deletes snapshots 0 and 1, this branch moves `latest` to 1 and
the next `snapshot(1)` throws, although snapshot 2 is the correct earliest
surviving result. I reproduced this deterministically by expiring the prefix
immediately after the first equal probe.
Please handle a missing post-match probe by refreshing the live lower bound
and restarting with a bounded retry, and add a regression test where expiration
runs after the first equal midpoint has been read.
--
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]
