JingsongLi commented on PR #10323: URL: https://github.com/apache/paimon/pull/10323#issuecomment-5944451719
Reviewed head `d8ef8b9aa0`. The stale-hint outage has clear production value, but this revision introduces a replay correctness problem. **[P1] Recover commit-user deduplication before allowing the commit to proceed** (`SnapshotManager.java:227–232`) `filterCommitted` still calls `latestSnapshotOfUser`, which starts from `latestSnapshotId()`. If LATEST points to an expired snapshot and its successor is also expired, that lookup stops at the missing snapshot and reports no previous commit. The new fallback then lets `tryCommit` find the real latest snapshot and publish the same files again. This is reachable in Flink's batch committer restart path: `CommitterOperator#commitUpToCheckpoint(END_INPUT_CHECKPOINT_ID)` calls `filterAndCommit(committables, false, true)`, deliberately disabling append-file conflict checks after deduplication. I reproduced this with an append table (`bucket=-1`), retained snapshot 7 containing the batch user's `Long.MAX_VALUE` commit, expired snapshots 1/2, and LATEST reset to 1. After reopening the table and replaying the original messages through `filterAndCommitMultiple(..., false)`, this head returns 1, publishes snapshot 8, and a real table read returns **8 rows instead of 7, including the same row twice**. With the baseline implementation, the same replay fails on the missing hinted snapshot before publishing. With append-file checks enabled, this head instead throws a duplicate-file conflict and cannot recover. Please make the commit-user lookup recover from the missing hint as well before concluding that a commit is new, and add replay coverage for both conflict-check modes. A failure-only fallback can preserve the normal-path I/O cost. There is also a remaining scope gap: `latestSnapshotId()` remains stale. Before another successful writer repairs LATEST, a fresh `scan.mode=latest` stream chose checkpoint 2 while the actual latest was 6 (expected checkpoint 7). The added test only checks reading after a successful commit repairs the hint. Please cover reader startup before that repair; this is an existing outage left unresolved, separate from the new duplicate-row regression. Validation: `SnapshotManagerTest` + `StaleLatestHintTest`: **45 tests passed**, including a final run without `fast-build` (Checkstyle/Spotless/enforcer enabled). Additional local replay/read probes reproduced the failures above; no probe changes were pushed. -- 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]
