JingsongLi commented on PR #10323: URL: https://github.com/apache/paimon/pull/10323#issuecomment-5965917048
The expiration-side direction (B alone, with the read-side recovery reverted) looks preferable for this PR. It preserves the invariant used by the existing readers and commit-user deduplication, and keeps recovery of already-broken tables as an explicit repair operation. The current `d8ef8b9` head still has the replay finding above; this is feedback on the proposed direction, not validation of an updated implementation. Please implement the protection as a cap on the entire expiration prefix, like the consumer boundary, rather than skipping only snapshot N. Keep the contiguous suffix from the hinted snapshot onward: retaining N while deleting N+1 would still make `findLatest` return the stale N. Apply the cap before any data/manifest deletion, including the time-retention early-return paths, and keep EARLIEST consistent with the actual retained range. One detail needs care: `HintFileUtils.readHint` currently returns null for both an absent hint and exhausted read retries. Expiration must not treat a transient hint-read failure as proof that there is no hint and delete the protected prefix. Please distinguish confirmed absence from an unreadable hint and conservatively skip deletion when that boundary cannot be established. Cover hint-read failures as well as hint-write failures, then successful recovery, concurrent commit/expiration, rollback, both replay conflict-check modes, streaming startup and timestamp-range reads. Skip the filesystem-hint policy only when the catalog actually supplies the snapshot state for this table. The expiration warning is useful; include the hinted ID, actual latest ID and retained boundary, and avoid repeating it without bound. A concise warning on the retry path that finds the snapshot already committed is also reasonable, with the completed snapshot ID and conditional wording such as “LATEST may not have been updated.” Keep it off the normal successful commit path and avoid claiming a failed hint write when the preceding exception does not establish that. Please update the PR and its tests to B before treating the current findings as resolved. -- 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]
