namest504 opened a new issue, #18737:
URL: https://github.com/apache/iceberg/issues/18737
### Apache Iceberg version
1.12.0 (latest release). Also reproduced on 1.10.0. Not reproducible on
1.9.2.
### Query engine
Other
### Please describe the bug
When `expireSnapshots()` is used inside a `Transaction`, the expired
manifest lists, manifests and data files are deleted as soon as the
transaction-local `commit()` runs. Nothing has been committed to the catalog at
that point. If the process dies, or `commitTransaction()` fails, the table
still references snapshots whose files are gone.
Before 1.10.0 the deletes were deferred. `BaseTransaction.expireSnapshots()`
called `expire.deleteWith(enqueueDelete)` so the files were only removed by
`commitTransaction()`. #218 added that callback ("keeps track of deletes
instead of running them") and #9183 relies on it (the expired files "should be
deleted when the transaction commits").
#13631 replaced that method body with `appendUpdate(new
RemoveSnapshots(transactionOps))`. `appendUpdate` only wires the delete
callback for `SnapshotUpdate`:
```java
// core/src/main/java/org/apache/iceberg/BaseTransaction.java
if (update instanceof SnapshotUpdate) {
((SnapshotUpdate) update).deleteWith(enqueueDelete);
}
```
`RemoveSnapshots` implements `ExpireSnapshots`, not `SnapshotUpdate`, so
`deleteFunc` stays `null` and `RemoveSnapshots.cleanExpiredSnapshots()` hands
the files straight to `FileIO.deleteFile`. The existing test
`testRetainNAvailableSnapshotsWithTransaction` only counts files after
`commitTransaction()`, where both behaviours look the same.
### Reproduction
Standalone program against the Maven artifacts (HadoopCatalog on local FS,
real Parquet rows). `TxnExpireRepro.java`, `build.gradle` and `run-output.txt`
are in this gist:
https://gist.github.com/namest504/86f80badc49976a4b793f58cb9acd237. Run with
`./gradlew run -PicebergVersion=1.12.0` (add the Gradle wrapper or use a local
Gradle).
Scenario A, the maintenance job is killed between the two commits:
```java
Transaction txn = table.newTransaction();
txn.updateProperties().set("maintenance.last-run",
String.valueOf(cutoff)).commit();
txn.expireSnapshots().expireOlderThan(cutoff).retainLast(1).commit();
// JVM dies here, commitTransaction() never runs
```
1.12.0 and 1.10.0:
```
S1 in table metadata: true
S1 manifest list on disk: false
read VERSION AS OF S1: FAILED NotFoundException: Failed to open input
stream for file: .../metadata/snap-...avro
read current snapshot: OK, 200 rows
```
1.9.2: manifest list still on disk, time travel to S1 returns 100 rows.
Scenario B, another client tags S1 while the transaction is in flight.
`commitTransaction()` fails on re-apply with `IllegalArgumentException: Cannot
expire ... Still referenced by refs: [audit-2026-10]`. The table keeps S1 and
the tag, but S1's manifest list is already deleted, so reading the tag fails
with the same `NotFoundException`. On 1.9.2 the tag stays readable.
A unit test shows the difference without any of that setup: after
`removeSnapshots(tx.table()).expireOlderThan(t).retainLast(1).commit()` and
before `tx.commitTransaction()`, the expired manifest lists must still exist
and must be in `((BaseTransaction) tx).deletedFiles()`. That fails on main and
passes with the change below.
### Scope
Every release that contains #13631: 1.10.0, 1.10.1, 1.10.2, 1.11.0, 1.12.0
and main. It affects snapshot expiration through the Java `Transaction` API.
The built-in Spark expire action and the Flink maintenance operator call
`table.expireSnapshots()` and do not use that path, but applications running on
those engines can.
With file cleanup enabled (the default) the files that go away are the ones
the expiration would delete anyway, including manifest lists, unreferenced
manifests and data files only reachable from the expired snapshots. The problem
is that the catalog metadata still points at them, so time travel, tags,
branches and rollback to those snapshots break, and the next `expireSnapshots`
logs `NotFoundException` retries while reading the missing manifest lists.
### Fix
Wire `ExpireSnapshots` in `appendUpdate` the same way as `SnapshotUpdate`:
```java
if (update instanceof SnapshotUpdate) {
((SnapshotUpdate) update).deleteWith(enqueueDelete);
} else if (update instanceof ExpireSnapshots) {
((ExpireSnapshots) update).deleteWith(enqueueDelete);
}
```
That restores the pre-1.10 behaviour. I have a PR ready with the change and
a regression test in `TestRemoveSnapshots`, and can follow up with 1.10.x and
1.11.x backports.
One thing the restore does not cover, and that was already the case in
1.9.x: if `commitTransaction()` fails for good with a `CleanableFailure`,
`cleanUp()` deletes everything in `deletedFiles`, including the expired
snapshots' files, even though those snapshots are still in the table. I think
that deserves its own issue, since it needs a separate set that is only deleted
after a successful commit.
### Willingness to contribute
- [x] I can contribute a fix for this bug independently
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
