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 1fbbae4c fix: read updated props when trimming metadata log (#639)
1fbbae4c is described below
commit 1fbbae4cf55d955863e059859e17e8230f503798
Author: Shubhendu <[email protected]>
AuthorDate: Tue Dec 9 23:06:56 2025 +0530
fix: read updated props when trimming metadata log (#639)
The metadata-log trimming logic was reading
write.metadata.previous-versions-max from the base metadata instead of
the updated properties being built. This caused the property to always
use the default value (100) even when explicitly set to a different
value during table updates.
## Motivation and Context
Fixes metadata file retention - when clients set
write.metadata.previous-versions-max=2, the metadata-log should contain
only 2 previous entries, not all historical entries.
---------
Signed-off-by: Shubhendu Ram Tripathi <[email protected]>
---
table/metadata.go | 2 +-
table/metadata_builder_internal_test.go | 79 +++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/table/metadata.go b/table/metadata.go
index 7dd803c8..d47d27ec 100644
--- a/table/metadata.go
+++ b/table/metadata.go
@@ -796,7 +796,7 @@ func (b *MetadataBuilder) buildCommonMetadata()
(*commonMetadata, error) {
if b.previousFileEntry != nil && b.HasChanges() {
maxMetadataLogEntries := max(1,
- b.base.Properties().GetInt(
+ b.props.GetInt(
MetadataPreviousVersionsMaxKey,
MetadataPreviousVersionsMaxDefault))
b.AppendMetadataLog(*b.previousFileEntry)
b.TrimMetadataLogs(maxMetadataLogEntries)
diff --git a/table/metadata_builder_internal_test.go
b/table/metadata_builder_internal_test.go
index 24e80e0e..28f62984 100644
--- a/table/metadata_builder_internal_test.go
+++ b/table/metadata_builder_internal_test.go
@@ -656,6 +656,85 @@ func TestExpireMetadataLog(t *testing.T) {
require.Len(t, meta.(*metadataV2).MetadataLog, 2)
}
+func TestMetadataLogTrimsWithUpdatedProperty(t *testing.T) {
+ // This test validates that when write.metadata.previous-versions-max is
+ // updated during a metadata build, the trimming logic uses the NEW
value
+ // from b.props, not the old value from b.base.Properties()
+
+ // Create initial metadata with default retention (100)
+ builder1 := builderWithoutChanges(2)
+ meta1, err := builder1.Build()
+ require.NoError(t, err)
+ require.Empty(t, meta1.(*metadataV2).MetadataLog)
+
+ // Build metadata #2 - this should create first metadata log entry
+ builder2, err := MetadataBuilderFromBase(meta1,
"s3://bucket/test/location/metadata/v2.json")
+ require.NoError(t, err)
+ err = builder2.SetProperties(map[string]string{
+ "test.prop": "value1",
+ })
+ require.NoError(t, err)
+ meta2, err := builder2.Build()
+ require.NoError(t, err)
+ require.Len(t, meta2.(*metadataV2).MetadataLog, 1)
+
+ // Build metadata #3 - this should have 2 entries
+ builder3, err := MetadataBuilderFromBase(meta2,
"s3://bucket/test/location/metadata/v3.json")
+ require.NoError(t, err)
+ err = builder3.SetProperties(map[string]string{
+ "test.prop": "value2",
+ })
+ require.NoError(t, err)
+ meta3, err := builder3.Build()
+ require.NoError(t, err)
+ require.Len(t, meta3.(*metadataV2).MetadataLog, 2)
+
+ // Build metadata #4 - this should have 3 entries
+ builder4, err := MetadataBuilderFromBase(meta3,
"s3://bucket/test/location/metadata/v4.json")
+ require.NoError(t, err)
+ err = builder4.SetProperties(map[string]string{
+ "test.prop": "value3",
+ })
+ require.NoError(t, err)
+ meta4, err := builder4.Build()
+ require.NoError(t, err)
+ require.Len(t, meta4.(*metadataV2).MetadataLog, 3)
+
+ // Build metadata #5 - NOW set write.metadata.previous-versions-max=2
+ // This is the key test: the trimming should happen immediately using
the
+ // NEW value (2), not the old/default value (100)
+ builder5, err := MetadataBuilderFromBase(meta4,
"s3://bucket/test/location/metadata/v5.json")
+ require.NoError(t, err)
+ err = builder5.SetProperties(map[string]string{
+ MetadataPreviousVersionsMaxKey: "2",
+ "test.prop": "value4",
+ })
+ require.NoError(t, err)
+ meta5, err := builder5.Build()
+ require.NoError(t, err)
+
+ // The bug was that it would keep all 4 entries because it read the
property
+ // from base metadata (which didn't have it set). After the fix, it
should
+ // correctly trim to 2 entries immediately.
+ require.Len(t, meta5.(*metadataV2).MetadataLog, 2,
+ "metadata log should be trimmed to 2 entries based on updated
property value")
+
+ // Verify the property was actually set
+ require.Equal(t, "2",
meta5.Properties()[MetadataPreviousVersionsMaxKey])
+
+ // Verify subsequent builds continue to respect the property
+ builder6, err := MetadataBuilderFromBase(meta5,
"s3://bucket/test/location/metadata/v6.json")
+ require.NoError(t, err)
+ err = builder6.SetProperties(map[string]string{
+ "test.prop": "value5",
+ })
+ require.NoError(t, err)
+ meta6, err := builder6.Build()
+ require.NoError(t, err)
+ require.Len(t, meta6.(*metadataV2).MetadataLog, 2,
+ "subsequent builds should continue to respect the max versions
limit")
+}
+
func TestV2SequenceNumberCannotDecrease(t *testing.T) {
builder := builderWithoutChanges(2)
schemaID := 0
