zeroshade commented on code in PR #716:
URL: https://github.com/apache/iceberg-go/pull/716#discussion_r2775597631
##########
table/metadata.go:
##########
@@ -485,26 +485,43 @@ func (b *MetadataBuilder) RemoveSnapshots(snapshotIds
[]int64) error {
return errors.New("current snapshot cannot be removed")
}
+ // Filter out snapshots that are referenced by any branch or tag.
+ // This prevents a race condition where a concurrent commit adds a ref
+ // to a snapshot that is being expired. Without this check, we could
+ // remove a snapshot that was just linked to a new branch/tag, leaving
+ // the metadata in a corrupt state with a dangling reference.
+ snapshotIds = slices.DeleteFunc(snapshotIds, func(id int64) bool {
+ return b.isSnapshotReferenced(id)
+ })
+
+ if len(snapshotIds) == 0 {
+ return nil
+ }
+
b.snapshotList = slices.DeleteFunc(b.snapshotList, func(e Snapshot)
bool {
return slices.Contains(snapshotIds, e.SnapshotID)
})
b.snapshotLog = slices.DeleteFunc(b.snapshotLog, func(e
SnapshotLogEntry) bool {
return slices.Contains(snapshotIds, e.SnapshotID)
})
- newRefs := make(map[string]SnapshotRef)
- for name, ref := range b.refs {
- if _, err := b.SnapshotByID(ref.SnapshotID); err == nil {
- newRefs[name] = ref
- }
- }
- b.refs = newRefs
-
b.updates = append(b.updates, NewRemoveSnapshotsUpdate(snapshotIds))
return nil
}
+// isSnapshotReferenced returns true if the given snapshot ID is referenced
+// by any branch or tag in the current metadata state.
+func (b *MetadataBuilder) isSnapshotReferenced(snapshotID int64) bool {
+ for _, ref := range b.refs {
+ if ref.SnapshotID == snapshotID {
+ return true
+ }
+ }
+
+ return false
Review Comment:
could this be replaced with just `slices.ContainsFunc(b.refs, func(r Ref)
bool { return r.SnapshotID == snapshotID })`?
--
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]
