amogh-jahagirdar commented on code in PR #4578:
URL: https://github.com/apache/iceberg/pull/4578#discussion_r867546362
##########
core/src/main/java/org/apache/iceberg/RemoveSnapshots.java:
##########
@@ -190,6 +282,10 @@ public void commit() {
.onlyRetryOn(CommitFailedException.class)
.run(item -> {
TableMetadata updated = internalApply();
+ if (cleanExpiredFiles && (updated.refs().size() > 1 ||
multipleLineages(updated))) {
Review Comment:
The check is done after the computation of snapshots to remove but before
the commit. I think this has to be done here, since we need the updated commit
graph after what the expiration would look like if we were to commit it to
determine if there will be any new detached lineages just before committing. If
the check is done before the computation I think the procedure would miss some
cases where the new commit graph is detached.
--
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]
