test(bench): RFC 0031 — pin promoted service.name column + pruning behavior - #483
Conversation
…rative store (rfc 0022) Grounding for the "promote service.name in the comparative store" slice found the premise already satisfied: RFC 0022 §3.1 promotes service.name implicitly and non-removably on every Writer constructor, including the Writer::open path build_comparative_store uses, and the querier's two-arm compile is union-schema-gated — so the comparative store's files already carry a stats-bearing resource.service.name column and the DSL `service ==` predicate already compiles to the promoted arm, not the JSON-substring LIKE fallback. No writer-side change is needed (threading PromotedAttributes through build_comparative_store would be a behavioral no-op), so this lands the parity pin as tests instead. Parity rationale (RFC 0031 §2 anti-strawman discipline): Loki's stock OTLP ingest promotes service.name to the service_name stream label by default, so service-scoped LogQL queries are label-indexed. The tests pin that Ourios holds the same ground — if a writer or store-builder change ever demoted the promoted column, the comparative harness would silently hold Ourios to a harder version of the service-scoped question than Loki answers. Two fixture-based tests (no containers), two-service two-hour corpus: - comparative_store_promotes_the_service_name_column: every published data file's footer declares resource.service.name, field-identical to the writer's default promoted schema. - service_predicate_prunes_on_the_promoted_column: `service == "svc-a"` matches exactly the 3 svc-a rows with row_groups_pruned = 1 (svc-b's row group skipped via promoted-column statistics) and bytes_read 284 vs 322 for the full-window scan. Measured while grounding, for the run #7 "service queries read whole partitions" observation: with both services interleaved in one hour partition the same query shows row_groups_pruned = 0 — matching rows sit in every row group, so its min/max spans both services and nothing can be skipped. The whole-partition reads are a data-layout property (no service clustering within row groups), not a predicate-compilation gap; a fix belongs to write-time layout (sorting/clustering), which is RFC-level, not a harness knob. Adds parquet (same workspace major, arrow feature) as a dev-dependency for the footer-level schema assertion. B1/B2's build_query_store path and the single-service fixture tests are untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 32 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 ignored due to path filters (1)
📒 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
Adds regression/spec tests in ourios-bench to pin that the comparative store already includes the promoted resource.service.name column (RFC 0022 §3.1) and that service == ... queries can prune using promoted-column statistics when the data layout permits it (supporting RFC 0031’s comparative invariants).
Changes:
- Add a two-service corpus builder and new tests asserting the promoted service column exists in comparative-store Parquet footers and that service predicates prune and read fewer bytes than full scans.
- Add a
parquetdev-dependency (Arrow reader) to assert footer schema directly. - Update
Cargo.lockto include the new dev-dependency.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/ourios-bench/tests/rfc0031_comparative.rs | Adds helper corpus + two regression tests for promoted service.name schema presence and pruning behavior. |
| crates/ourios-bench/Cargo.toml | Adds parquet as a dev-dependency to read Parquet footer schemas in tests. |
| Cargo.lock | Records the new parquet dependency in the lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y
What
Tests only — regression pins for a premise this slice falsified while investigating run #7's whole-partition reads:
comparative_store_promotes_the_service_name_column— every data file the comparative store publishes carries the promotedresource.service.namecolumn (RFC 0022 §3.1 makes it non-removable:PromotedAttributes::default()always includes it, and everyWriterconstructor funnels through it).service_predicate_prunes_on_the_promoted_column— on a two-service, two-hour corpus,service == "svc-a"returns exactly svc-a's rows (compare_lines), reportsrow_groups_pruned ≥ 1, and reads strictly fewer bytes than the full-window scan — proving the querier's union-schema-gated promoted arm compiles and prunes.The finding this pins
The planned change ("promote service.name in the comparative store") was a no-op: the column is already there and already pruning where the layout allows. The actual cause of run #7's whole-partition reads is data layout: otel-demo-v8 interleaves all services within each hour partition, so every row group's
service.namemin/max spans the service alphabet and statistics cannot prune (verified: the same query on an interleaved same-hour layout reportsrow_groups_pruned = 0). The fix lever is write-time layout (service clustering/sorting within row groups, or per-service file splitting) — pillar-adjacent write-path design that needs its own RFC and a maintainer decision, tracked in the RFC 0031 goals discussion.The parity note stands and is recorded at the test banner: Loki's stock OTLP ingest promotes
service.nameto theservice_namestream label by default, so both systems get a promoted service dimension out of the box.Invariants / hazards
Test-only + one dev-dependency (
parquet, workspace major, for the footer schema assertion). No production code touched; B1/B2 store path untouched.Checks run
cargo fmt --all --check,cargo clippy --all-targets --all-features -- -D warnings(workspace-wide),cargo nextest run -p ourios-bench(129 passed, 13 skipped) — re-verified after rebasing onto #481+#482.🤖 Generated with Claude Code
https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y