amogh-jahagirdar commented on code in PR #4578:
URL: https://github.com/apache/iceberg/pull/4578#discussion_r867546130
##########
core/src/main/java/org/apache/iceberg/RemoveSnapshots.java:
##########
@@ -161,21 +173,101 @@ public List<Snapshot> apply() {
private TableMetadata internalApply() {
this.base = ops.refresh();
+ if (base.refs().isEmpty()) {
+ return base;
+ }
Set<Long> idsToRetain = Sets.newHashSet();
- List<Long> ancestorIds = SnapshotUtil.ancestorIds(base.currentSnapshot(),
base::snapshot);
- if (minNumSnapshots >= ancestorIds.size()) {
- idsToRetain.addAll(ancestorIds);
- } else {
- idsToRetain.addAll(ancestorIds.subList(0, minNumSnapshots));
+ // Identify refs that should be removed
+ Set<String> retainedRefs = computeRetainedRefs(base.refs());
+ Set<Long> retainedRefIds = retainedRefs.stream()
+ .map(ref -> base.ref(ref).snapshotId())
+ .collect(Collectors.toSet());
+
+ for (long idToRemove : idsToRemove) {
+ Preconditions.checkArgument(!retainedRefIds.contains(idToRemove),
+ "Cannot expire %s. There is a reference to it", idToRemove);
+ }
+
+ idsToRetain.addAll(retainedRefIds);
+ Set<Long> branchSnapshotsToRetain =
computeAllBranchSnapshotsToRetain(base.refs());
+ idsToRetain.addAll(branchSnapshotsToRetain);
+ TableMetadata.Builder updatedMetaBuilder = TableMetadata.buildFrom(base);
+ idsToRemove.addAll(base.snapshots().stream().map(Snapshot::snapshotId)
+ .filter(snapshot ->
!idsToRetain.contains(snapshot)).collect(Collectors.toSet()));
+ base.refs().keySet().stream().filter(ref ->
!retainedRefs.contains(ref)).forEach(updatedMetaBuilder::removeRef);
+ updatedMetaBuilder.removeSnapshots(idsToRemove);
+ TableMetadata updatedMetadata = updatedMetaBuilder.build();
+ return updatedMetadata;
+ }
+
+ private boolean multipleLineages(TableMetadata metadata) {
+ List<Snapshot> allSnapshots = metadata.snapshots();
+ Set<Snapshot> mainSnapshots = Sets.newHashSet(SnapshotUtil.ancestorsOf(
+ metadata.ref(SnapshotRef.MAIN_BRANCH).snapshotId(),
metadata::snapshot));
+ return allSnapshots.stream()
+ .filter(snapshot -> !mainSnapshots.contains(snapshot))
+ .filter(snapshot -> snapshot.parentId() != null)
+ .filter(snapshot -> metadata.snapshot(snapshot.parentId()) == null ||
+ !mainSnapshots.contains(metadata.snapshot(snapshot.parentId())))
+ .findAny().isPresent();
Review Comment:
This should filter out any snapshots which are outside the main lineage,
which are not staged commits off of main, which have parent snapshots where the
parent snapshot is either outside of main OR the parent snapshot is
non-existent in the metadata since it was removed. The existence of such a
snapshot which meets this criteria means the procedure with cleanExpired cannot
be performed.
--
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]
