rdblue commented on code in PR #13614:
URL: https://github.com/apache/iceberg/pull/13614#discussion_r2229043800
##########
core/src/main/java/org/apache/iceberg/RemoveSnapshots.java:
##########
@@ -390,4 +386,43 @@ private void cleanExpiredSnapshots() {
cleanupStrategy.cleanFiles(base, current);
}
+
+ private void validateCleanupCanBeIncremental(TableMetadata current) {
Review Comment:
I think maybe we can do a little better. I think your argument for tags
main's ancestors is a good one. We can do that for branches, too.
What about these rules to determine if incremental is safe?
1. No specified snapshots to remove
2. No snapshots to remove that are not ancestors of main
3. No snapshots after expiration that are not ancestors of main
I think the combination of 2 and 3 cover branch and tag cases. Tags function
like branches with just one snapshot, so reasoning about branches will cover
them. Any branch either has a snapshot outside of main or it doesn't. If it
doesn't, then incremental cleanup will work. If there is a snapshot outside of
main, then it is either going to be caught be 2 (if it ages off) or 3 (if it
does not age off).
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
