dev-donghwan commented on PR #10323:
URL: https://github.com/apache/paimon/pull/10323#issuecomment-5981468508
Thanks for the direction. I force-pushed the PR to implement B only, so the
read-side
recovery commits are gone and reads and commit-user deduplication are
unchanged from master.
**What this PR changes**
- Snapshot expiration caps the end of the expiration range at the LATEST
hint, like the
consumer boundary, so the hinted snapshot and all later ones are kept.
- The hint is read strictly right before deleting. If it cannot be read,
nothing is deleted
this time.
- If the hinted snapshot is already missing, the snapshot after it and all
later ones are
kept. If that one is missing too, nothing is deleted.
- The protection is skipped when the catalog manages the snapshots.
- Warnings are logged on expiration while the hint is behind, and on the
commit retry path
that finds the snapshot already committed.
**How each of your points is addressed**
1. **Cap the entire prefix, keep the contiguous suffix.** The cap applies to
the whole range,
so retaining N while deleting N+1 cannot happen.
(`testHintedSnapshotIsNotExpired`)
2. **Apply the cap before any deletion, including the time-retention
paths.** Both the
time-retention early return and the retained-count path call
`expireUntil`, and the cap
is applied there before `innerExpireUntil` starts deleting data,
changelog and manifest
files. (`testBothExpirationPathsKeepTheHintedSnapshot`)
3. **Keep EARLIEST consistent.** EARLIEST points to the first retained
snapshot.
(`testEarliestHintMatchesTheKeptSnapshots`)
4. **Distinguish absent and unreadable hints, skip when the boundary cannot
be
established.** Following `readOverwrittenFileUtf8`, the hint is returned
as an `Optional`
(present or absent), and an unreadable hint throws an `IOException` after
the same
retries as `readHint`. Then nothing is deleted. A hint that is not a
positive number is
treated as absent, like `findLatest` does.
(`testUnreadableHintSkipsExpiration`,
`testTransientHintReadFailureIsRetried`, `testUnusableHintIsIgnored`)
5. **Test coverage.**
- hint write failures and recovery: `testCommitsGoOnWhileHintWritesFail`,
`testExpireResumesAfterHintWritesRecover`
- concurrent commit and expiration: `testCommitDuringExpiration`
- rollback: `testRollback`, `testRollbackDuringExpiration`
- replay in both conflict-check modes: `testReplayCommittedCommit`, and
`CommitterOperatorTest#testReplayEndInputCommitWhileLatestHintIsBehind`
for the Flink
batch restart path
- streaming startup before the hint is repaired:
`testStreamingReadFromLatest`
- timestamp-range reads: `testIncrementalBetweenTimestamps`
6. **Skip the policy only when the catalog supplies the snapshot state.** It
uses the same
condition as `CatalogEnvironment#snapshotCommit`.
(`testCatalogManagedSnapshotsIgnoreTheHintFile`)
7. **Expiration warning.** It includes the hinted ID, the actual latest ID
and the retained
boundary, and is logged once per hint value. The warning for an
unreadable hint is logged
once until the hint can be read again. (`testExpirationWarnsOncePerHint`,
`testReadFailureWarnsOnceUntilTheHintCanBeRead`)
8. **Retry warning.** It is logged only on the retry path that finds the
snapshot already
committed, with the snapshot ID and "The LATEST hint may not have been
updated." That
path is also reached when the atomic commit fails without an exception,
so the wording
does not claim one. (`testRetryWarningOnlyWhenTheCommitIsFoundCommitted`)
**In addition**
I handled the case where the hinted snapshot is already missing. Without it,
expiring the
snapshot after it as well makes the table stuck, and on an already stuck
table, expiration
deletes only part of the data files, so the remaining older snapshots can no
longer be read.
(`testHintOnMissingSnapshotKeepsTheNextOne`,
`testHintOnMissingSnapshotKeepsDataFilesOfTheNextOne`,
`testHintOnMissingSnapshotsKeepsDataFilesOfOlderSnapshots`)
**Behavior changes**
While the hint is behind, more snapshots than `snapshot.num-retained.max`
can be retained,
like with consumers, and the `expire_snapshots` procedures can expire fewer
snapshots. Each
expiration also reads the LATEST hint once more; it is read right before
deleting to keep the
window against concurrent commits and rollbacks small. These are also listed
in the PR
description.
--
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]
