vaibhavbodhane opened a new pull request, #26920: URL: https://github.com/apache/datafusion/pull/26920
## Which issue does this PR close? - Part of #22189. ## Rationale for this change Each parquet file the opener reads registered more than one set of `ParquetFileMetrics`: - **Opener:** `ParquetMorselizer::prepare_open_file` registers one set. - **Reader factory:** `DefaultParquetFileReaderFactory` / `CachedParquetFileReaderFactory::create_reader` registers another set for the reader it builds. - **Bloom filter path:** when a file goes through Bloom filter pruning, the replacement reader created for decoding registers a third. Each set is about 20 filename-labelled metrics. The plan's `ExecutionPlanMetricsSet` keeps all of them until the query ends. On scans of many files this is a noticeable share of memory. On DataFusion 54.1.0, a 27-day scan of a 488-column table (~29,000 small files) peaked at ~915 MB, and ~85% of it was under `ParquetFileMetrics::new` (details in https://github.com/apache/datafusion/issues/22189#issuecomment-6490924131). As discussed in the issue, this PR removes the duplicates without changing what is reported. A follow-up will propose an option for per-partition metrics on scans of very many files. ## What changes are included in this PR? - **New provided method:** `ParquetFileReaderFactory::create_reader_with_file_metrics(…, file_metrics: ParquetFileMetrics)`. It builds a reader that records into a `ParquetFileMetrics` the caller has already registered. - The default implementation ignores `file_metrics` and calls `create_reader`, so existing implementations are unaffected. - **Built-in factories:** `DefaultParquetFileReaderFactory` and `CachedParquetFileReaderFactory` implement it by passing the given metrics to `ParquetFileReader`. Their `create_reader` registers a set and delegates to it, so direct callers see no change. - **Opener:** both places that create a reader — `prepare_open_file`, and the replacement reader after Bloom filter pruning — call `create_reader_with_file_metrics` with the file's existing `ParquetFileMetrics`. `ParquetFileMetrics` is `Clone` and its counters are shared, so the opener and its readers update the same metrics. ## Are these changes tested? Yes: - **New test:** `opening_a_file_registers_one_set_of_file_metrics` opens a file that goes through Bloom filter pruning, so a replacement reader is built. It checks that `bytes_scanned` and `metadata_load_time` are registered once, and that `bytes_scanned` still counts the reader's I/O. - **Existing tests:** the tests that assert on `bytes_scanned`, including `fully_matched_row_groups_skip_bloom_filter_reads`, pass unchanged. ## Are there any user-facing changes? - **Metric output:** the same metric names and totals per file are reported. Totals from `MetricsSet::sum_by_name` are unchanged. - The only difference is that each file now appears once, where it previously appeared two or three times with the same `filename` label. - Per-file values that were split across the duplicate entries are now combined into one entry. For example, the opener recorded `metadata_load_time` and the reader recorded `bytes_scanned`, each into its own set. - **API:** `ParquetFileReaderFactory` gains a provided method, so the change is additive and no existing implementation needs to change. cc @alamb @xudong963 (as discussed in #22189) 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
