perf(querier): RFC 0031 — late materialization via filter pushdown on the materialize scan - #486
Conversation
…erialize scan The RFC 0031 run #8 decomposition showed row materialization dominating per-query object-storage reads: rendering even ONE matching row re-read the hot partition's page-index-matched window of every column chunk whole (severity query, 1 row: 3,146,731 B materialize vs 609,498 B count scan on otel-demo-v8). Enable DataFusion's parquet filter pushdown (pushdown_filters + reorder_filters, off by default in DF 54) on the materialize scan in Querier::collect_records: the predicate now evaluates during Parquet decode, and the writer's offset index lets the reader fetch only the heavy-column (body / params / attributes) pages the selected rows live in. Results are bit-identical (the regression test pins the exact LineKey; the full querier + bench suites pass unmodified). Scoped to the materialize scan only. Enabled on the count scan too, pushdown lets statistics-fully-matched row groups answer with zero bytes scanned (correct rows, bytes_scanned=0) — a further win, but it conflicts with honest_total_bytes_breaks_down_additively's "count_scan_bytes > 0" assertion and would hollow out the stats.bytes_read semantics the B1/B2 gates assert on, so the count scan keeps the default (conflict reported, test untouched). Empirical basis (new deterministic 70,000-row / 3-hour-partition store, hot hour 60,000 rows, 1-row `severity >= 17 | limit 10` query, regression-pinned in ourios-bench): before: count_scan=148,392 materialize=1,664,654 registry=8,972 after: count_scan=148,392 materialize= 742,036 registry=8,972 2.24x fewer materialize bytes; the regression test asserts the component stays under a 1 MiB ceiling the whole-window behavior violates by ~1.6x while the page-selective scan clears it with ~1.4x headroom. (An earlier 20,000-row hot hour showed only 690,181 -> 623,049: parquet-rs's 20,000-row default page limit made each chunk a single page, so the fixture is sized past it — page-selective reads need >1 page per chunk to be observable.) Hazard §4.6: no DataFusion type crosses the public surface — the knob is internal to the scan session. Invariant §3.3 unaffected: rendering still goes through the read-time registry; the regression test asserts the returned body byte-for-byte. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements RFC 0031’s “late materialization” lever for the querier’s row materialization scan by enabling DataFusion 54 Parquet filter pushdown (and filter reordering) only for that scan. This allows the materialization pass to read just the Parquet pages that contain the selected rows (using offset/page indexes), rather than re-reading wider column-chunk windows, and adds a regression test that pins both correctness and the expected IO reduction.
Changes:
- Enable DataFusion Parquet
pushdown_filters+reorder_filtersspecifically for the materialization scan insidecollect_records, without changing the count scan’s semantics/metrics. - Add a deterministic, >20k-row/page synthetic corpus and a regression test that asserts the 1-row materialization path stays below a 1 MiB IO ceiling while returning a bit-identical answer.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/ourios-querier/src/lib.rs | Enables Parquet filter pushdown + reorder for the materialization scan by rebuilding the SessionState with updated Parquet execution options. |
| crates/ourios-bench/src/comparative.rs | Adds a deterministic late-materialization corpus + regression test asserting correctness and that materialization IO stays below a ceiling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What
Late materialization on the row-materialization scan (RFC 0031 lever 1 — the dominant run-#8 cost): enables DataFusion 54's
pushdown_filters+reorder_filtersoncollect_records' scan, so rendering N matching rows reads the pages containing those rows instead of every unpruned row group's whole heavy column chunks. The writer's page-level statistics (EnabledStatistics::Page, RFC 0005 §3.5) finally earn their keep on the read side.Why / measured effect
Run #8 measured materialization at 3.15 MB to render ONE row (severity pair) and 7.66 MB for k=2000. Locally, on a deterministic 70,000-row store (hot hour 60,000 rows, one ERROR row, high-entropy bodies + realistic attributes), the 1-row query's materialize component drops 1,664,654 → 742,036 B (2.24×), with scan metrics confirming
pushdown_rows_matched=1, pushdown_rows_pruned=20000. Correctness pinned byte-for-byte: the regression test asserts the exact returned LineKey (timestamp + reconstructed body). Run #10 (post-merge dispatch) measures the isolated v8 delta.Empirical footnote documented in the fixture: parquet-rs's 20,000-row default page limit means smaller stores have single-page chunks where page-selective reads are invisible — the fixture is deliberately sized past it.
Scope decision (surfaced, not hidden)
Pushdown is enabled on the materialize scan only. Enabling it on the count scan makes statistics-fully-matched row groups report
bytes_scanned = 0(DataFusion's fully-matched path skips all page reads for a projection-free count) — correct rows, but it would flip the bench spec testhonest_total_bytes_breaks_down_additivelyand hollow out thestats.bytes_readsemantics B1/B2 assert on. Per CLAUDE.md §6.2 that is a contract change needing explicit approval, so the count scan keeps defaults; flagged in the code comment as a candidate follow-up. (Post-#485 the harness elides the count scan on complete results anyway, so for comparative measurement the question is mostly moot.)Composition with #485
Rebased over the single-pass merge; the elision-equality regression tests (materialize plan's row-group counts == count scan's) pass with pushdown enabled, so the two levers compose — verified, not assumed.
Invariants / hazards
Read-side only: no schema, miner, or ingest change; bit-identical results pinned by test. Hazard 6: nothing leaks into the DSL. Pillar #1's "query performance comes from skipping" now applies within row groups, not just across them.
Checks run
cargo fmt --all --check,cargo clippy --all-targets --all-features -- -D warnings(workspace-wide),cargo nextest run -p ourios-querier -p ourios-bench(297 passed, 14 skipped) — re-run after the rebase over #485.🤖 Generated with Claude Code
https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y