vaibhavbodhane commented on issue #22189: URL: https://github.com/apache/datafusion/issues/22189#issuecomment-6490924131
We hit this one as a memory problem rather than a latency one, so here are some numbers in case they help. **Setup:** DataFusion 54.1.0, via Sail 0.7.1. - **Query:** `select x, count(*) … group by x` over a hive-partitioned parquet table. - **Table:** a wide schema (488 columns, the union of many sources). Most files are small, about 1,100 files per day of data. - **Server:** one process with a 3.9 GB cgroup limit, 4 partitions, statistics collection off. **What we saw.** Peak memory grew linearly with the number of files scanned, about 32 MB per day (about 29 KB per file). A 27-day scan peaked at about 915 MB. A jemalloc heap profile, unsampled, put about 85% of the peak under `ParquetFileMetrics::new`. The plan's `ExecutionPlanMetricsSet` keeps every registered metric until the query ends, so these sets add up over the whole scan rather than being freed file by file. **Two things we noticed in 54.1.0:** 1. **Duplicate registration:** each opened file gets two `ParquetFileMetrics`, one from the reader factory (`DefaultParquetFileReaderFactory::create_reader`) and one from `ParquetMorselizer::prepare_open_file`, plus a `BaselineMetrics` per file. That is about 40 filename-labelled metrics per file. Sharing a single instance per file would halve this without changing what users see, unless I'm missing a reason they're separate. 2. **#22406 doesn't change the count:** sharing the label string removes the copies, but the number of `Metric` entries still grows with the number of files. **What we tried (local patch, not proposed as-is).** Each scan partition registers **one unlabelled** `ParquetFileMetrics` and one `BaselineMetrics` on first use. Every file opened on that partition adds to the same counters. The reader keeps its own per-file byte count, so the scan-efficiency ratio stays correct. Results, same query and data, run alternately: | Days scanned | 54.1.0 peak | patched peak | time | |---|---|---|---| | 14 | 505 MB | 188 MB | 103.5 s → 101.8 s | | 27 | 913–921 MB | 344–350 MB | 205–231 s → 179–197 s (13–15% faster) | Results were identical. Metrics are still registered eagerly and visible during execution, which I understand was the concern with #22353, and `EXPLAIN ANALYZE` totals don't change. **The trade-off** is that per-file metric rows and the `filename` label disappear, so this changes observable output. That's fine for us, because we only look at aggregates, but it probably shouldn't be the default. **A possible shape upstream:** a parquet option, say `per_file_metrics` (default `true`), that falls back to per-partition metrics on scans of very many files. That, together with (1), would cover the case of wide tables with many small files. Happy to share the patch or turn this into a PR if one of these directions sounds acceptable. -- 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]
