rdblue commented on a change in pull request #497: Support retaining last N
snapshots
URL: https://github.com/apache/incubator-iceberg/pull/497#discussion_r329690374
##########
File path: core/src/main/java/org/apache/iceberg/RemoveSnapshots.java
##########
@@ -310,6 +323,19 @@ private void deleteDataFiles(Set<String> manifestsToScan,
Set<String> manifestsT
return filesToDelete;
}
+ private Snapshot findNthSnapshot(int snapshotIndex) {
+ Snapshot lastSnapshot = base.currentSnapshot();
+ int snapshotCount = 1;
+ while (lastSnapshot != null) {
+ if (snapshotCount == snapshotIndex) {
+ return lastSnapshot;
+ }
+ snapshotCount++;
+ lastSnapshot = (lastSnapshot.parentId() != null) ?
base.snapshot(lastSnapshot.parentId()) : null;
Review comment:
I think that this should include snapshots that are not ancestors of the
current table state.
We've added support in Iceberg for staging commits that will be audited
before becoming the current table state. That means that we can easily have
snapshots that are future versions that just haven't been validated yet. We
shouldn't remove those. I think that the best way to handle this is to just
keep the most recent versions, regardless of whether they are ancestors.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
