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 >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