fix(bench): query the corpus tenant in b2/otel-demo (was always empty) - #116
Conversation
The b2/otel-demo arm hardcoded `tenant: "a"`, but the corpus loader writes every record under `bench-tenant` (corpus.rs BENCH_TENANT). So the query hit the RFC0007.5 empty-result early return and "benchmarked" a 0-row query (~1.1 µs) instead of scanning the corpus — a meaningless number the arm had always produced (env-gated, never validated against a real corpus). - `BuiltStore` now carries the `tenant` it wrote under (the loader is single-tenant), so the bench queries the right one. - b2's `template_exact` takes a tenant; the otel-demo arm uses `built.tenant` and probes the query up front, asserting it returns `busiest_template_rows` so any future tenant/template mismatch fails loudly rather than silently timing an empty query. The probe line also logs the scanned/pruned row groups + bytes for the run. Validated on the frozen otel-demo corpora (fetched via `gh release download`): v1 → 783 µs scanning 1/1 row groups, v4 → 2.9 ms scanning 5/5. (5/5 is itself a finding — a template recurs across all partitions, so template_id alone prunes nothing on real data; logged in scratch.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@coderabbitai review |
|
Warning Review limit reached
More reviews will be available in 13 minutes and 22 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ 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 |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
This PR fixes the b2/otel-demo Criterion bench arm so it queries the same tenant that the corpus loader writes under, preventing silent “0-row” measurements and adding an up-front probe that fails loudly if the bench is miswired.
Changes:
- Extend
BuiltStoreto carry the tenant used during corpus load/write, and populate it fromcorpus::BENCH_TENANT. - Update the
b2bench helper to buildQueryRequests with an explicit tenant, and wireb2/otel-demoto usebuilt.tenant. - Add an
otel-demoprobe assertion plus additional logging of query scan/prune stats to surface empty/incorrect queries immediately.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/ourios-bench/src/store.rs | Adds BuiltStore.tenant and returns the loader’s BENCH_TENANT so downstream benches can query the correct tenant. |
| crates/ourios-bench/benches/b2.rs | Threads tenant into template_exact, uses built.tenant for otel-demo, and probes/logs query results and scan stats before benchmarking. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What
Fixes a latent bug in the
b2/otel-demobench arm that made it silently measure an empty query, and surfaced it by running the bench against the real frozen otel-demo corpus (thecorpus/otel-demo-v*release assets — fetched withgh release download, no container runtime needed).The bug
b2/otel-demohardcodedtenant: "a", but the corpus loader writes every record underbench-tenant(corpus.rsBENCH_TENANT). So the query hit the RFC0007.5 empty-result early return and "benchmarked" a 0-row query at ~1.1 µs — a meaningless number the arm had always produced (it's env-gated, so it had never been validated against a real corpus). Theb2/syntheticarm was unaffected (it writes its own records under"a").Fix
BuiltStorenow carries thetenantit wrote under (&'static str; the loader is single-tenant).template_exacttakes a tenant;b2/otel-demousesbuilt.tenantand probes the query up front, asserting it returnsbusiest_template_rows— so a future tenant/template mismatch fails loudly instead of silently timing an empty query. The probe also logs scanned/pruned row groups + bytes.Validated against the real corpus (indicative, laptop — NOT §9)
Headline finding (the synthetic bench hid it): on real data a template-exact query prunes nothing (v4 scanned 5/5 row groups) — a template recurs in every time partition, so every row group's
template_idmin/max matches.b2/syntheticisolated each template to its own file, which made pruning look perfect; real logs don't. The inverted-index-collapse locality needs the time-range predicate + partition pruning (the querier's explicitly-deferred refinement). Captured inscratch/b2-otel-demo-findings.md; this shapes the real B2 measurement (a time-windowed query) and is a strong argument for prioritizing partition-level time pruning.Tests
store.rsrejects_a_non_empty_bucketstill green;BuiltStore.tenantpopulated from the loader constant.cargo fmt --all --check,cargo clippy --workspace --all-targets --all-features -D warnings,cargo test --workspaceall green.Part of epic #81. Follow-up: a CI workflow that fetches the frozen corpus and runs b1/b2 (the b1/otel-demo arm wiring + partition-time-pruning are separate).
🤖 Generated with Claude Code