Skip to content

Add file size tracking to snapshot metrics#1187

Open
fm100 wants to merge 1 commit into
duckdb:mainfrom
fm100:enrich-iceberg-snapshot-metadata
Open

Add file size tracking to snapshot metrics#1187
fm100 wants to merge 1 commit into
duckdb:mainfrom
fm100:enrich-iceberg-snapshot-metadata

Conversation

@fm100

@fm100 fm100 commented Jul 17, 2026

Copy link
Copy Markdown

Summary

Add Iceberg snapshot summary metrics:

  • added-files-size
  • removed-files-size
  • total-files-size

Sizes are derived from manifest entries. For legacy snapshots without total-files-size, the value is reconstructed from live manifest entries.

Testing

Added coverage for appends, positional deletes, and data-file rewrites. Build, formatting, and targeted tests pass.

@Tishj Tishj changed the title Enrich iceberg snapshot metadata. Add file size tracking to snapshot metrics Jul 17, 2026
@@ -163,7 +193,6 @@ void IcebergAddSnapshot::CreateUpdate(DatabaseInstance &db, ClientContext &conte

//! Create a new manifest list, populate it with the content of the old manifest list (altered if necessary)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please move the rest of this down as well, you've broken this comment by separating the steps

commit_state.manifests.clear();
}

static void AddLiveFileSize(int64_t &total_files_size, const IcebergDataFile &data_file) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Duplicate of AddFileSizeChecked ?

idx_t deleted_rows_count = 0;
//! Size in bytes of content files added by this manifest. This is derived from
//! manifest entries and is not serialized in the manifest list.
int64_t added_files_size = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No, this is meant to mirror the manifest_file spec of https://iceberg.apache.org/spec/#manifest-lists, this information shouldn't live here.

manifest_file.added_rows_count = 0;
manifest_file.existing_rows_count = 0;
manifest_file.deleted_rows_count = 0;
manifest_file.added_files_size = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On top of that this shouldn't use manifest_file to carry this information, this is also the wrong place.
CreateFromEntries is used in many contexts, not just new manifests

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