You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by ze...@apache.org on 2026/04/13 20:43:11 UTC
(iceberg-go) branch main updated: fix(table): delete statistics files in ExpireSnapshots PostCommit #837 (#882)
This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new 98366073 fix(table): delete statistics files in ExpireSnapshots PostCommit #837 (#882)
98366073 is described below
commit 9836607384031a9db802e311cb6e4fb27d5163f9
Author: Tanmay Rauth <ta...@gmail.com>
AuthorDate: Mon Apr 13 13:43:06 2026 -0700
fix(table): delete statistics files in ExpireSnapshots PostCommit #837 (#882)
PostCommit was collecting manifest lists, manifests, and data files for
deletion when expiring snapshots, but never collected StatisticsFile or
PartitionStatisticsFile paths. Statistics files for expired snapshots
were never deleted, leaking storage.
Note: This PR depends on #873 (MetadataBuilderFromBase preserving
StatisticsList/PartitionStatsList). Until that merges, the subtraction
at PostCommit (lines 517–523 of updates.go) relies on
postTable.Metadata().Statistics( returning surviving entries — which it
won't in the current main for a mixed expiry scenario.
Fixes #837
---
table/updates.go | 25 ++++++++
table/updates_test.go | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 182 insertions(+)
diff --git a/table/updates.go b/table/updates.go
index f79fe00e..db708ffc 100644
--- a/table/updates.go
+++ b/table/updates.go
@@ -450,6 +450,23 @@ func (u *removeSnapshotsUpdate) PostCommit(ctx context.Context, preTable *Table,
filesToDelete[snap.ManifestList] = struct{}{}
}
+ expiredIDs := make(map[int64]struct{}, len(u.SnapshotIDs))
+ for _, id := range u.SnapshotIDs {
+ expiredIDs[id] = struct{}{}
+ }
+
+ for sf := range preTable.Metadata().Statistics() {
+ if _, ok := expiredIDs[sf.SnapshotID]; ok && sf.StatisticsPath != "" {
+ filesToDelete[sf.StatisticsPath] = struct{}{}
+ }
+ }
+
+ for psf := range preTable.Metadata().PartitionStatistics() {
+ if _, ok := expiredIDs[psf.SnapshotID]; ok && psf.StatisticsPath != "" {
+ filesToDelete[psf.StatisticsPath] = struct{}{}
+ }
+ }
+
for _, snapId := range u.SnapshotIDs {
snap := preTable.SnapshotByID(snapId)
if snap == nil {
@@ -497,6 +514,14 @@ func (u *removeSnapshotsUpdate) PostCommit(ctx context.Context, preTable *Table,
}
}
+ for sf := range postTable.Metadata().Statistics() {
+ delete(filesToDelete, sf.StatisticsPath)
+ }
+
+ for psf := range postTable.Metadata().PartitionStatistics() {
+ delete(filesToDelete, psf.StatisticsPath)
+ }
+
var res error
for f := range filesToDelete {
diff --git a/table/updates_test.go b/table/updates_test.go
index 92d01c62..48d8a566 100644
--- a/table/updates_test.go
+++ b/table/updates_test.go
@@ -37,6 +37,163 @@ func TestRemoveSnapshotsPostCommitSkipped(t *testing.T) {
assert.NoError(t, err)
}
+func TestRemoveSnapshotsPostCommitDeletesStatisticsFiles(t *testing.T) {
+ // preTable has snapshot 1 with associated statistics and partition statistics files.
+ // postTable has no snapshots and no statistics (snapshot 1 has been expired).
+ // PostCommit must delete the statistics paths that belonged to the expired snapshot.
+ const preMeta = `{
+ "format-version": 2,
+ "table-uuid": "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
+ "location": "s3://bucket/table",
+ "last-sequence-number": 1,
+ "last-updated-ms": 1000,
+ "last-column-id": 1,
+ "current-schema-id": 0,
+ "schemas": [{"type":"struct","schema-id":0,"fields":[{"id":1,"name":"x","required":true,"type":"long"}]}],
+ "default-spec-id": 0,
+ "partition-specs": [{"spec-id":0,"fields":[]}],
+ "last-partition-id": 0,
+ "default-sort-order-id": 0,
+ "sort-orders": [{"order-id":0,"fields":[]}],
+ "current-snapshot-id": 1,
+ "snapshots": [{"snapshot-id":1,"timestamp-ms":1000,"sequence-number":1,"schema-id":0}],
+ "snapshot-log": [],
+ "metadata-log": [],
+ "statistics": [{"snapshot-id":1,"statistics-path":"s3://bucket/stats/snap1.puffin","file-size-in-bytes":100,"file-footer-size-in-bytes":10,"blob-metadata":[]}],
+ "partition-statistics": [{"snapshot-id":1,"statistics-path":"s3://bucket/stats/snap1-part.puffin","file-size-in-bytes":50}]
+ }`
+ const postMeta = `{
+ "format-version": 2,
+ "table-uuid": "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
+ "location": "s3://bucket/table",
+ "last-sequence-number": 1,
+ "last-updated-ms": 2000,
+ "last-column-id": 1,
+ "current-schema-id": 0,
+ "schemas": [{"type":"struct","schema-id":0,"fields":[{"id":1,"name":"x","required":true,"type":"long"}]}],
+ "default-spec-id": 0,
+ "partition-specs": [{"spec-id":0,"fields":[]}],
+ "last-partition-id": 0,
+ "default-sort-order-id": 0,
+ "sort-orders": [{"order-id":0,"fields":[]}],
+ "snapshot-log": [],
+ "metadata-log": []
+ }`
+
+ pre, err := ParseMetadataString(preMeta)
+ require.NoError(t, err)
+ post, err := ParseMetadataString(postMeta)
+ require.NoError(t, err)
+
+ // Pass *trackingIO to createTestTransaction — Go converts it to iceio.IO implicitly,
+ tio := newTrackingIO()
+ tio.files["s3://bucket/stats/snap1.puffin"] = []byte("puffin")
+ tio.files["s3://bucket/stats/snap1-part.puffin"] = []byte("puffin")
+
+ txn := createTestTransaction(t, tio, iceberg.NewPartitionSpec())
+ fsF := txn.tbl.fsF
+ preTable := New(Identifier{"ns", "tbl"}, pre, "metadata.json", fsF, nil)
+ postTable := New(Identifier{"ns", "tbl"}, post, "metadata.json", fsF, nil)
+
+ update := NewRemoveSnapshotsUpdate([]int64{1}, true)
+ err = update.PostCommit(context.Background(), preTable, postTable)
+ require.NoError(t, err)
+
+ assert.NotContains(t, tio.files, "s3://bucket/stats/snap1.puffin")
+ assert.NotContains(t, tio.files, "s3://bucket/stats/snap1-part.puffin")
+}
+
+func TestRemoveSnapshotsPostCommitPreservesStatisticsOfSurvivingSnapshots(t *testing.T) {
+ // pre: snapshots 1 and 2, statistics for both.
+ // post: snapshot 1 expired, snapshot 2 kept with its statistics still present.
+ // PostCommit must delete only snap1's statistics files; snap2's must survive.
+ const preMeta = `{
+ "format-version": 2,
+ "table-uuid": "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
+ "location": "s3://bucket/table",
+ "last-sequence-number": 2,
+ "last-updated-ms": 1000,
+ "last-column-id": 1,
+ "current-schema-id": 0,
+ "schemas": [{"type":"struct","schema-id":0,"fields":[{"id":1,"name":"x","required":true,"type":"long"}]}],
+ "default-spec-id": 0,
+ "partition-specs": [{"spec-id":0,"fields":[]}],
+ "last-partition-id": 0,
+ "default-sort-order-id": 0,
+ "sort-orders": [{"order-id":0,"fields":[]}],
+ "current-snapshot-id": 2,
+ "snapshots": [
+ {"snapshot-id":1,"timestamp-ms":1000,"sequence-number":1,"schema-id":0},
+ {"snapshot-id":2,"timestamp-ms":2000,"sequence-number":2,"schema-id":0}
+ ],
+ "snapshot-log": [],
+ "metadata-log": [],
+ "statistics": [
+ {"snapshot-id":1,"statistics-path":"s3://bucket/stats/snap1.puffin","file-size-in-bytes":100,"file-footer-size-in-bytes":10,"blob-metadata":[]},
+ {"snapshot-id":2,"statistics-path":"s3://bucket/stats/snap2.puffin","file-size-in-bytes":100,"file-footer-size-in-bytes":10,"blob-metadata":[]}
+ ],
+ "partition-statistics": [
+ {"snapshot-id":1,"statistics-path":"s3://bucket/stats/snap1-part.puffin","file-size-in-bytes":50},
+ {"snapshot-id":2,"statistics-path":"s3://bucket/stats/snap2-part.puffin","file-size-in-bytes":50}
+ ]
+ }`
+ const postMeta = `{
+ "format-version": 2,
+ "table-uuid": "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
+ "location": "s3://bucket/table",
+ "last-sequence-number": 2,
+ "last-updated-ms": 3000,
+ "last-column-id": 1,
+ "current-schema-id": 0,
+ "schemas": [{"type":"struct","schema-id":0,"fields":[{"id":1,"name":"x","required":true,"type":"long"}]}],
+ "default-spec-id": 0,
+ "partition-specs": [{"spec-id":0,"fields":[]}],
+ "last-partition-id": 0,
+ "default-sort-order-id": 0,
+ "sort-orders": [{"order-id":0,"fields":[]}],
+ "current-snapshot-id": 2,
+ "snapshots": [
+ {"snapshot-id":2,"timestamp-ms":2000,"sequence-number":2,"schema-id":0}
+ ],
+ "snapshot-log": [],
+ "metadata-log": [],
+ "statistics": [
+ {"snapshot-id":2,"statistics-path":"s3://bucket/stats/snap2.puffin","file-size-in-bytes":100,"file-footer-size-in-bytes":10,"blob-metadata":[]}
+ ],
+ "partition-statistics": [
+ {"snapshot-id":2,"statistics-path":"s3://bucket/stats/snap2-part.puffin","file-size-in-bytes":50}
+ ]
+ }`
+
+ pre, err := ParseMetadataString(preMeta)
+ require.NoError(t, err)
+ post, err := ParseMetadataString(postMeta)
+ require.NoError(t, err)
+
+ tio := newTrackingIO()
+ tio.files["s3://bucket/stats/snap1.puffin"] = []byte("puffin")
+ tio.files["s3://bucket/stats/snap1-part.puffin"] = []byte("puffin")
+ tio.files["s3://bucket/stats/snap2.puffin"] = []byte("puffin")
+ tio.files["s3://bucket/stats/snap2-part.puffin"] = []byte("puffin")
+
+ txn := createTestTransaction(t, tio, iceberg.NewPartitionSpec())
+ fsF := txn.tbl.fsF
+ preTable := New(Identifier{"ns", "tbl"}, pre, "metadata.json", fsF, nil)
+ postTable := New(Identifier{"ns", "tbl"}, post, "metadata.json", fsF, nil)
+
+ update := NewRemoveSnapshotsUpdate([]int64{1}, true)
+ err = update.PostCommit(context.Background(), preTable, postTable)
+ require.NoError(t, err)
+
+ // snap1's statistics must be deleted
+ assert.NotContains(t, tio.files, "s3://bucket/stats/snap1.puffin")
+ assert.NotContains(t, tio.files, "s3://bucket/stats/snap1-part.puffin")
+
+ // snap2's statistics must survive
+ assert.Contains(t, tio.files, "s3://bucket/stats/snap2.puffin")
+ assert.Contains(t, tio.files, "s3://bucket/stats/snap2-part.puffin")
+}
+
func TestUnmarshalUpdates(t *testing.T) {
spec := iceberg.NewPartitionSpecID(3,
iceberg.PartitionField{