rdblue commented on a change in pull request #3457:
URL: https://github.com/apache/iceberg/pull/3457#discussion_r744336454
##########
File path:
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/actions/BaseExpireSnapshotsSparkAction.java
##########
@@ -170,13 +177,15 @@ public BaseExpireSnapshotsSparkAction
deleteWith(Consumer<String> newDeleteFunc)
expireSnapshots = expireSnapshots.retainLast(retainLastValue);
}
+ List<Snapshot> expired = expireSnapshots.apply();
Review comment:
There is no guarantee that `apply` followed by `commit` will produce the
same result, so this is unsafe. It may work most of the time, but there will
probably be leaks when `expired` here doesn't contain a snapshot that was
actually removed. That's why we always refresh and compare against the version
that was actually committed.
--
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]
