docs(engine): pin the sketch-vs-precompute query gap with file:line citations - #252
Merged
Merged
Conversation
…itations Engine-path debug session diagnosis (post-#247 → #248 → #249 → #250). The query layer's "No result for query" symptom for sketch-backed metrics has a precise root cause: `query_precomputes_by_agg` (called unconditionally by `execute_store_query`) filters its candidate-sid scan with `matches!(&m.agg_kind, AggKind::ExactAgg { … })` — never matching `AggKind::Sketch`. Since OTLP-arriving DDSketch / KLL / HLL / CountSketch / CountMinSketch DPs are registered with `AggKind::Sketch { kind, config, .. }`, the lookup always returns an empty map for them, the engine bubbles up "No precomputed outputs found", and `handle_query` returns `None` → HTTP responds `errorType: bad_data` / `error: "No result for query"`. Sketches DO reach `SketchStore` — they're readable via the sid-keyed `query_range(sid, ...)` path which filters on `payload.as_sketch()`. The agg-keyed precompute lookup is the gap. This PR adds doc-comment blocks at both the call site (`engine.rs::execute_store_query`) and the function definition (`mod.rs::query_precomputes_by_agg`) flagging the gap with file:line citations and pointing at the two viable fixes: * teach `query_precomputes_by_agg` to also collect sketch payloads (assemble `Box<dyn AggregateCore>` from `payload.as_sketch()`) — non-trivial; payload shapes diverge. * route the legacy `handle_query` path through the newer `ASAPQueryEngine::execute(&str)` trait dispatcher (around engine.rs:3430) which already handles sketches via `idx.sids_for_policy(fp)` + reducer dispatch. No behaviour change. 690 lib + 27 binary tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
zzylol
added a commit
that referenced
this pull request
May 15, 2026
…path (#253) Closes the gap pinned in #252: the legacy ASAPQueryEngine::handle_query path can't read sketch-backed sids (`query_precomputes_by_agg` only matches `AggKind::ExactAgg`), but the modern `execute(&str)` trait path (engine.rs:3430) DOES — it dispatches via `find_matching_policies` → `idx.sids_for_policy(fp)` → `SketchReducer::evaluate` and handles sketches natively. `process_via_simple_engine` (the HTTP handler for SketchStore-routed queries) now falls back to `execute(&str)` when `handle_query` returns None. Trait dispatch loses `KeyByLabelNames`; we surface `KeyByLabelNames::default()` (mirroring `process_via_router`'s identical handling on line 1171) — the Prometheus adapter renders the empty `metric: {}` shape, valid PromQL response. ## Setup-side fix (the actual bug Test 3 surfaced) The diagnostic also caught a missing `.with_sketch_index(idx)` call in the test harness — without it, the engine's `sketch_index` is `None` and EVERY fast path (`execute_store_query`, `execute(&str)`, `query_range`, …) is silently skipped because they all guard with `let Some(idx) = self.sketch_index.as_ref() else { return … };`. This is a load-bearing wiring step that production `data_plane/main.rs` gets right but `start_test_server` callers were missing. Both `start_backend_http_server` and `start_full_stack` now thread `with_sketch_index(sketch_index.clone())` so the engine's reads see the same store the OTLP receiver / precompute engine writes to. ## Test 3 strict assertion now active `controller_plan_to_query_full_roundtrip_ddsketch` previously soft-checked the response had a `status` field. After this PR it asserts `status == "success"` — the FULL e2e path (controller plan → POST /api/v1/streaming-config → OTLP DDSketch DPs → window close → GET /api/v1/query) now works end-to-end. This is the first time we have a green-bar e2e test for the gateway-less data path. ## Tests - `cargo test --test e2e_controller_plans_and_backend_serves`: 3 passed, 0 failed (was 2/3 with Test 3 soft-checked). - Full sweep: lib 690 + bins 27 + integration tests across the workspace all green. ## Out of scope The legacy `query_precomputes_by_agg` filter still doesn't include `AggKind::Sketch` (per #252's diagnostic comment). Closing that fully — so `handle_query` itself works for sketches — would let us delete this fallback. Not blocking; the fallback is a stable bridge. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Engine-path debug session result. Diagnoses precisely why the e2e tests' query path returns
bad_data/ "No result for query" even after #247–#250 fixed the wire-format and L5-walk gaps.Root cause
query_precomputes_by_agg(storage_engines/sketch_db/index/mod.rs:432, called unconditionally byengine.rs::execute_store_query) filters its candidate-sid scan with:It NEVER matches
AggKind::Sketch. OTLP-arriving DDSketch / KLL / HLL / CountSketch / CountMinSketch DPs register withAggKind::Sketch { kind, config, .. }(perroute_modified_otlp_sketches_to_precomputeatotel.rs:1081), so this lookup always returns empty for them.Sketches DO reach
SketchStore— they're readable via the sid-keyedquery_range(sid, ...)path (filters bypayload.as_sketch()). The agg-keyed precompute lookup is the gap.What this PR delivers
Doc-comment blocks at both:
engine.rs::execute_store_query) — explains why the call returns empty for sketch-backed aggsmod.rs::query_precomputes_by_agg) — explains why the filter excludes sketchesBoth citations include file:line pointers + the diagnosis trail (#247 → #248 → #249 → #250 → engine-path debug session).
Two fix paths flagged for the next PR
Teach
query_precomputes_by_aggto also collect sketch payloads (assembleBox<dyn AggregateCore>frompayload.as_sketch()). Non-trivial because the existing pipeline expects precompute payload shapes.Route
handle_querythroughASAPQueryEngine::execute(&str)trait dispatcher (engine.rs:3430) which already doesidx.sids_for_policy(fp)+ reducer dispatch and handles sketches natively viaSketchReducer::evaluate.Test plan
cargo checkclean (no behaviour change)cargo test --lib --tests: 690 + 27 + 62 + 768 passed; 0 failed🤖 Generated with Claude Code