Repository navigation
feat(parquet): report decompression time and bytes in ArrowReaderMetrics - #11532
Open
zhuqi-lucas wants to merge 2 commits into
Open
zhuqi-lucas wants to merge 2 commits into
zhuqi-lucas wants to merge 2 commits into
Conversation
Decompression happens inside decode_page, under both decoding and skipping, and no metric exposes it: a scan slow because of its codec cannot be told apart from one slow because there is a lot to decode. Add DecompressionMetrics (parquet::file::metrics), split by page kind (dictionary or data) and by phase (pages decompressed to read values or to skip past them). SerializedPageReader takes a handle via with_decompression_metrics and records in decode_page. ArrowReaderMetrics owns one and hands it to every page reader of a scan, on both the sync reader and the push decoder (and so the async reader). Disabled by default with ArrowReaderMetrics; when disabled no clock is read.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Skip-phase attribution can substantially misclassify decompression later used to return selected values.
1 open finding
What changed in this PR
Adds opt-in Parquet decompression metrics to distinguish dictionary/data page costs across read and skip phases.
Changes:
- Introduces shared decompression time, byte, and page counters.
- Instruments page decoding and phase attribution.
- Wires metrics through synchronous, push, and asynchronous Arrow readers with tests.
| File | Description |
|---|---|
parquet/src/file/serialized_reader.rs |
Records decompression metrics during page decoding. |
parquet/src/file/mod.rs |
Exposes the metrics module. |
parquet/src/file/metrics.rs |
Defines metrics API, counters, and tests. |
parquet/src/file/metadata/dictionary.rs |
Disables scan metrics for metadata dictionary reads. |
parquet/src/column/reader.rs |
Sets read or skip decompression phase. |
parquet/src/column/page.rs |
Extends PageReader with phase attribution. |
parquet/src/arrow/push_decoder/reader_builder/mod.rs |
Wires metrics into push-decoder row groups. |
parquet/src/arrow/push_decoder/reader_builder/data.rs |
Initializes row-group metrics state. |
parquet/src/arrow/in_memory_row_group.rs |
Propagates metrics to in-memory page readers. |
parquet/src/arrow/arrow_reader/mod.rs |
Wires synchronous reader metrics. |
parquet/src/arrow/arrow_reader/metrics.rs |
Adds decompression accessors to Arrow metrics. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A page first loaded by skip_records was counted as Skip even when its values were then read: skip(1) followed by reading the rest of the page reported the whole page as thrown away. Hold a page decompressed while skipping, and the chunk's dictionary, in a DecompressionRecorder until it is known whether values are read from it (PageReader::current_page_read, called by the column reader when read_records starts on a page that still has values). A page the reader moves past, or drops, without reading counts as Skip. Tests: recorder state transitions; skip one row then read the rest counts as Read; a selective read over a list column with no offset index counts pages with a kept row as Read and pages walked past as Skip.
Contributor
Author
etseidl
self-requested a review
October 14, 2026 05:28
Contributor
|
I will try and check it out |
Contributor
|
👍🏻 I think pretty much the more metrics the merrier |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
EXPLAIN ANALYZE), which needs this collection side first.skip_recordseagerly decompresses dictionary pages for column chunks it never decodes #11154 / Defer dictionary page decode on the skip path #11168 (dictionary decompression on the skip path, found only with a CPU profiler).Rationale for this change
Decompression happens inside
decode_page, under both decoding and skipping, and nothing reports it. A scan that is slow because of its codec cannot be told apart from one that is slow because there is a lot to decode, and the cost of decompressing pages only to walk past them is invisible. On our data servers dictionary decompression on the skip path was 14-18% of CPU, and finding it took a profiler on a production pod.Measured with a local prototype (1M rows, 40k distinct strings, zstd(3)):
skip_recordsWhat changes are included in this PR?
parquet::file::metrics::DecompressionMetrics: time, uncompressed bytes and page count, split by page kind (dictionary / data) and by phase (Read: pages decompressed to decode values;Skip: pages decompressed only to walk past values). Lives infile, notarrow, becausedecode_pageis in the non-arrowfilemodule.SerializedPageReader::with_decompression_metrics;decode_pagerecords into it. The column reader marks the phase.ArrowReaderMetricsowns one handle and gives it to every page reader of a scan, on the sync reader and on the push decoder (and so the async reader). New accessors:decompression(),dictionary_decompress_time(),data_decompress_time(),dictionary_decompressed_bytes(),data_decompressed_bytes().Decompression is nested inside decoding and skipping, so these numbers are a share of that time, not a stage to add next to it; the docs say so.
Are these changes tested?
Yes. Unit tests for the counters, and end-to-end tests on a real zstd file: a full read records everything under
Read; a selective read records walked pages underSkip; disabled metrics reportNone; the push decoder records the same bytes and page counts as the sync reader (this test fails if the push decoder is not wired).Overhead: collection is opt-in through
ArrowReaderMetrics, off by default. When off,decode_pagetests one enum discriminant and reads no clock. When on, with 1,000-row pages (1,000 decompress calls per read), 40 interleaved reads each: median 8.05 ms off vs 8.08 ms on (+0.4%, within noise).Are there any user-facing changes?
New public API listed above. No behaviour change when metrics are disabled.