dev-donghwan opened a new pull request, #10323:
URL: https://github.com/apache/paimon/pull/10323
### Purpose
A stale `LATEST` hint should be harmless, because the hint is only a cache.
Today it can leave a table permanently unable to commit or read, even though
all newer snapshot files are still there.
#### The problem
`HintFileUtils.findLatest` reads the hint `N` and returns it if `snapshot-(N
+ 1)` does not exist. It never checks that `snapshot-N` itself exists:
```java
Long snapshotId = readHint(fileIO, LATEST, dir);
if (snapshotId != null && snapshotId > 0) {
long nextSnapshot = snapshotId + 1;
// it is the latest only there is no next one
if (!fileIO.exists(file.apply(nextSnapshot))) {
return snapshotId;
}
}
return findByListFiles(fileIO, Math::max, dir, prefix);
```
A missing `N + 1` can mean two things: it has not been created yet, or it
was created and then expired. `findLatest` assumes the first. If the hint stops
moving while commits and expiration go on, expiration eventually removes both
`N` and `N + 1`:
```
snapshot-13 ... snapshot-30 (10, 11 and 12 expired)
LATEST = 10
```
`findLatest` sees that `snapshot-11` is missing and returns 10, which no
longer exists, instead of 30. The directory listing is never reached.
The table cannot recover on its own:
1. Only the commit path writes `LATEST`.
2. A commit first calls `latestSnapshot()`, which gets 10 from `findLatest`
and fails with `Snapshot file .../snapshot-10 does not exist. It might have
been expired by other jobs operating on this table. ...`.
3. The commit never gets far enough to rewrite the hint, so every later
commit reads the same stale hint.
The retry added in feabf2c6d does not help, because the second `findLatest`
call returns the same id. Callers that use only `latestSnapshotId()`, such as
expiration and the streaming starting scanners, get the wrong id as well.
#### Why this was safe before
`findLatest` and `findEarliest` were introduced together in FLINK-26778
(apache/flink-table-store#56). Each hint only guards against the gap its own
writer can leave:
| | `EARLIEST` | `LATEST` |
|---|---|---|
| Written by | expiration | commit |
| Order | delete snapshots, then write hint | create snapshot, then write
hint |
| Gap in between | hint points to a deleted snapshot | a newer snapshot
exists |
| Check | `snapshot-N` exists | `snapshot-(N + 1)` does not exist |
The review of #56 assumed that `LATEST` "is only changed by the commit
operation and that value should be precise". A failed hint write also failed
the commit. Under that assumption, expiration could never reach the snapshot
`LATEST` points to, so checking that it exists was unnecessary.
That assumption no longer always holds. Since #5771 (1.3.0), a commit retry
that finds its snapshot already written treats the commit as successful. This
is the "Check if the commit has been completed" path in `FileStoreCommitImpl`.
It correctly avoids a duplicate commit, but it does not rewrite the hint. So
`LATEST` can now stay behind across many successful commits. Once it falls
behind by more than the retention window, the hinted snapshot is expired.
Writing the hint on that path would help, but it cannot cover hint writes
that keep failing. This PR therefore makes `findLatest` recover from a stale
hint, whatever made it stale.
#### How we hit this
We run Paimon 1.4.2 on Flink 2.2.1, with a Hive catalog and the warehouse on
an S3-compatible object store. The table keeps `snapshot.num-retained.max=20`
with 30s checkpoints, which is about 6 minutes of snapshots.
The trigger was a problem in our own setup. S3A was loaded from
`/opt/flink/lib` rather than as a Flink plugin. After a job restart closed the
user classloader, S3A copies on a long-lived TaskManager kept succeeding on the
server but failing on the client side. Every snapshot rename threw even though
the snapshot file was written, and the retry path above reported success
without writing the hint. `LATEST` stayed at the same id for 21 consecutive
commits. We have since fixed this by moving S3A into the plugin directory.
That trigger alone should have been temporary. The permanent outage came
from `findLatest`. About 6 minutes after the hint stopped moving, expiration
removed the hinted snapshot. From then on, the writer failed in
`FileStoreCommitImpl.tryCommit` and in `FileSystemWriteRestore.restoreFiles`,
and a downstream streaming reader failed with `OutOfRangeException`. The job
restarted about 6,000 times over two days. Overwriting `LATEST` by hand was the
only way out.
Any failure that keeps `LATEST` from moving for longer than the retention
window leads to the same state, for example repeated hint write failures, or a
filesystem that keeps reporting errors after successful renames. With a small
`snapshot.num-retained.max`, that window can be only a few minutes.
#### Change
`findLatest` now trusts the hint only if `snapshot-(N + 1)` does not exist
and `snapshot-N` does exist. Otherwise it lists the snapshot directory, as it
already does when `N + 1` exists:
| State of hint `N` | Before | After |
|---|---|---|
| `N` is the latest | `N` | `N` |
| `N + 1` exists (hint slightly behind) | list | list |
| `N` and `N + 1` both expired | `N` (missing file) | list, finds the real
latest |
The `N + 1` check stays, since it is what catches a hint that is only
slightly behind. The new check is the one `findEarliest` already does, because
both hints can now be invalidated by expiration. Once `findLatest` returns the
real latest snapshot, the next commit succeeds and rewrites the hint, so the
table recovers by itself. `ChangelogManager` uses the same `findLatest` and
gets the same fix.
The cost is one extra `exists` call when the hint is valid. The `N + 1`
check runs first, so a hint that is already behind costs nothing extra. No
listing is added while the hint is valid.
### Tests
- [x] `SnapshotManagerTest#testLatestSnapshotWithExpiredLatestHint`: the
hint points to an expired snapshot while snapshots 5–10 exist.
`latestSnapshotId()` returned 2 before this change and returns 10 after it.
- [x] `StaleLatestHintTest#testCommitAfterLatestHintExpired`: drives the
real commit and expire paths with a `FileIO` that skips the `LATEST` write,
then lets hint writes recover. Before this change, commits and reads keep
failing after the recovery. After it, the next commit succeeds and refreshes
the hint.
- [x] All tests under `org.apache.paimon.utils` and
`org.apache.paimon.table.source`, plus the snapshot expiration tests in
`org.apache.paimon.operation`: 621 tests pass.
- [x] Spotless and Checkstyle for `paimon-core`
--
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]
