Skip to content

Conversation

@dhananjaykrutika
Copy link
Contributor

ExpireSnapshots was adding a RemoveSnapshotsUpdate even when there were no snapshots to delete. This caused Commit to create a new metadata file on every call, even when nothing changed, leading to metadata file proliferation in maintenance jobs that leverage iceberg-go's snapshot expiration implementation.

ExpireSnapshots was adding a RemoveSnapshotsUpdate even when there were
no snapshots to delete. This caused Commit to create a new metadata file
on every call, even when nothing changed, leading to metadata file
proliferation in maintenance jobs that leverage iceberg-go's snapshot
expiration implementation.
Comment on lines 278 to 283
// Only add the update if there are actually snapshots to delete
if len(snapsToDelete) > 0 {
updates = append(updates, NewRemoveSnapshotsUpdate(snapsToDelete))
}

return t.apply(updates, reqs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably also have a length check on updates and simply return nil if len(updates) == 0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tx.Commit() does check the length of updates before calling table commit & post-commit ops. See https://github.com/apache/iceberg-go/blob/main/table/transaction.go#L551

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants