amogh-jahagirdar commented on PR #8520: URL: https://github.com/apache/iceberg/pull/8520#issuecomment-1714024844
@rdblue As I was looking into writing tests for this, I think in practice we end up not hitting the scenario described (at least as far as I understand). The reason is currently after the transaction is committed successfully the clean up only cleans up uncommitted files. https://github.com/apache/iceberg/blob/master/core/src/main/java/org/apache/iceberg/BaseTransaction.java#L455 . The same logic exists at the `SnapshotProducer` level as well. https://github.com/apache/iceberg/blob/master/core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java#L931 . So if a manifest file were to get reused and successfully committed in the transaction, it would be part of the `committedFiles` and never be deleted. All that said, I think it's still probably a good invariant to have to clear the files to delete on every transaction attempt in case some new logic gets added in the future. -- 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]
