NestDream opened a new pull request, #10089:
URL: https://github.com/apache/paimon/pull/10089
### Purpose
fix #10088
`CALL sys.expire_snapshots(`table` => 'default.T', retain_min => 0,
older_than => '2030-01-01 00:00:00')` is accepted by the Flink procedure and by
the Flink `expire_snapshots` action, and so is any negative `retain_min`; the
Spark procedure passes `retain_min` through the same
`ProcedureUtils.fillInSnapshotOptions` into the same `ExpireSnapshotsImpl`.
`ExpireSnapshotsImpl.expire()` only checks `retainMax >= retainMin`, so with
`retain_min => 0` the exclusive upper bound `latestSnapshotId - retainMin + 1`
becomes `latestSnapshotId + 1`. `innerExpireUntil` then collects every existing
snapshot, deletes the changelog files of all of them including the latest one,
and deletes the data files that a later snapshot marks as removed even though
the older snapshots in the range still reference them. Only after that it
notices that the end snapshot `latestSnapshotId + 1` does not exist, returns 0
and leaves every snapshot file and manifest in place. The table ends up with
all its snapshots
listed in `$snapshots` and some of them pointing at files that are gone. The
table option `snapshot.num-retained.min` is validated to be at least 1 in
`SchemaValidation`; the procedure arguments bypass that.
I reproduced it on a local Flink 1.20.1 MiniCluster against the unmodified
module jar (master 54d8596ce). On a `changelog-producer = input` table with
three inserts the call above returns `0`, `$snapshots` still lists 1, 2, 3, the
three changelog files are deleted, and a batch read of the changelog of
snapshot 3 (`/*+ OPTIONS('incremental-between'='2,3',
'incremental-between-scan-mode'='changelog') */`) fails with
`FileNotFoundException: File '.../bucket-0/changelog-....parquet' not found`.
On a table where snapshot 3 is an `INSERT OVERWRITE`, the same call deletes the
two data files of snapshots 1 and 2 while keeping their snapshot files, so
`SELECT * FROM T /*+ OPTIONS('scan.snapshot-id'='2') */` fails with the same
`FileNotFoundException`. `retain_min => -1` behaves the same way. In
`ExpireSnapshotsTest` with five commits on a `changelog-producer = input`
store, `expire()` with `snapshotRetainMin(0)` returns 0, keeps all five
snapshot files and deletes all 20 changelog files.
This PR adds `checkArgument(retainMin >= 1, ...)` next to the existing
`retainMax >= retainMin` check in `ExpireSnapshotsImpl`, with a message in the
style of the existing one. With the fix the call above fails with
`IllegalArgumentException: retainMin (0) must be at least 1.` before anything
is deleted. `retain_max` alone needs no new check: with `retainMin >= 1`
enforced first, the existing check already rejects `retainMax <= 0`.
### Tests
New `ExpireSnapshotsTest#testExpireRejectsNonPositiveRetainMin`: five
commits on a `changelog-producer = input` store, then `expire()` with
`snapshotRetainMin(0)` and `snapshotRetainMin(-1)`; asserts the
`IllegalArgumentException` with the message above, that the set of files under
the table is unchanged, that all five snapshots are still readable, and
`assertCleaned()`. Without the fix it fails at the first assertion (`Expecting
code to raise a throwable`), and the same setup deletes all 20 changelog files
while every snapshot file survives. The whole `ExpireSnapshotsTest` class
passes (38 tests including the new one).
The MiniCluster scenario above (three inserts plus `retain_min => 0`, three
inserts plus `retain_min => -1`, the `INSERT OVERWRITE` table, and `retain_min
=> 1` as a control) was run three times each against the unmodified and the
fixed module jar. Unmodified: `0` returned, files deleted,
`FileNotFoundException` on read, in all three runs. Fixed: the argument error,
no file deleted, snapshot 3's changelog and snapshot 2 read back correctly. The
control expires snapshots 1 and 2 and keeps 3 on both.
--
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]
