amogh-jahagirdar commented on code in PR #13614:
URL: https://github.com/apache/iceberg/pull/13614#discussion_r2226791798
##########
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 don't think we'd ever be in a state where only a non main ref is defined
because at minimum the `main` branch would always be defined; even if it's
empty. so the exactly one check is OK i think. I could make it more explicit
though by comparing that it is main. `base.currentSnapshot()` would also not be
null at this point since we early return on 355 if there are no snapshots in
the metadata before, there'd be nothing to cleanup.
>We also need to ensure that there were no non-main snapshots before
expiration. Those need to be cleaned up through reachability analysis.
The ancestry check in the method in 407 would cover this because we'd check
that we can't do incremental cleanup for any snapshots that are outside of
main?
--
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
