JohnFitzpatrick44 opened a new issue, #17304: URL: https://github.com/apache/iceberg/issues/17304
### Apache Iceberg version 1.11.0 (latest release) ### Query engine None ### Please describe the bug 🐞 If a data file is removed and re-added in the same commit, e.g. during a `RewriteFiles` commit where `addFile` and `deleteFile` are called on the same file path, that data file (which is still live) can be deleted when expiring snapshots, corrupting the table. This occurs when `IncrementalFileCleanup.findFilesToDelete` (https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/IncrementalFileCleanup.java#L302-L305) removes files from snapshots that are `DELETED` without examining other manifest entries. This can result in a live file being deleted. Here's the order of events: 1. A writer does a `RewriteFiles` commit. The commit removes file `F` and adds file `F` at the same path. Snapshot `S1` contains one `DELETED` and `ADDED` entry for `F`. 2. `F` remains live, and readers continue using `F` correctly. 3. Another snapshot `S2` is created. 4. `expireSnapshots` removes `S1`. Conditions are met so that `IncrementalFileCleanup` is used. 5. The cleanup finds the `DELETED` entry for `F` and deletes it. 6. `S2` still refers to `F`, so readers now fail. ```java // table with file F live in the current snapshot table.newRewrite() .rewriteFiles(Set.of(F), Set.of(F)) // remove F, add F at the same path .commit(); // snapshot S1 table.newAppend().appendFile(G).commit(); // snapshot S2 table.expireSnapshots() .expireOlderThan(System.currentTimeMillis()) .commit(); // deletes F from storage // reads now fail, as F is referenced but not found ``` We can address this in the `RewriteFile` commit step or `ExpireSnapshots` step. 1. `MergingSnapshotProducer` rejects or no-ops on commits that remove and add the same file. 2. `IncrementalFileCleanup` does not delete files that are removed and added in the same step. Alternatively, `ReachableFileCleanup` is used more frequently, but this would deteriorate performance. From https://github.com/apache/iceberg/issues/13568 / https://github.com/apache/iceberg/pull/13614, it seems that option 2 would be preferred, and/or we should deprecate `IncrementalFileCleanup`. ### Willingness to contribute - [ ] I can contribute a fix for this bug independently - [x] I would be willing to contribute a fix for this bug with guidance from the Iceberg community - [ ] I cannot contribute a fix for this bug at this time -- 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]
