Skip to content

fix: expose Iceberg work unit feed metrics - #763

Draft
alexanderbianchi wants to merge 3 commits into
datafusion-contrib:mainfrom
alexanderbianchi:fix/iceberg-work-unit-metrics
Draft

alexanderbianchi wants to merge 3 commits into
datafusion-contrib:mainfrom
alexanderbianchi:fix/iceberg-work-unit-metrics

Conversation

@alexanderbianchi

@alexanderbianchi alexanderbianchi commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Use the work unit feed as the shared metrics registry for Iceberg scans, whether execution is local or distributed.

  • Give the local Iceberg feed a persistent metrics set.
  • Register scan baseline metrics through IcebergDataSource::metrics(), which returns the feed’s shared set.
  • Remove the separate data-source metrics field and codec-only initialization.

This exposes feed metrics without returning a fresh registry on each call, preserving later registrations such as DataFusion’s batches_split counter.

Validation

  • cargo test -p datafusion-distributed-iceberg --features integration
  • Formatting and diff checks passed.
  • Temporary checks verified shared local registrations and local/distributed EXPLAIN ANALYZE metrics; these checks were removed and are not part of the PR.

Comment thread iceberg/src/codec.rs Outdated
partitioning,
fetch,
metrics: Default::default(),
metrics: feed.metrics(),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This code here is just the codec. There's a chance that this code is not even reached in a normal execution, and this will still miss the coordinator-collected metrics.

Instead, we should be joining both DataSource and WorkUnitFeed metrics in the metrics() method:

impl DataSource for IcebergDataSource {
    ...
    fn metrics(&self) -> ExecutionPlanMetricsSet {
        let mut all_metrics = self.feed.metrics().clone_inner();
        all_metrics.extend(self.metrics.clone_inner());
        ExecutionPlanMetricsSet::from(all_metrics)
    }
    ...
}

@alexanderbianchi
alexanderbianchi marked this pull request as draft October 4, 2026 00:37

This branch has not been deployed

No deployments
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