Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions data_plane/src/query_engines/asap_query_engine/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1036,6 +1036,43 @@ impl ASAPQueryEngine {
// Tests that don't attach a SketchStore now get `Ok(empty)`
// here. Anything deeper than smoke-test coverage was already
// setting one (M2.3.5b made it mandatory in production).
//
// ── KNOWN GAP — sketch-backed aggs return empty here ─────────
//
// `query_precomputes_by_agg` (called below) filters its
// candidate-sid scan with `matches!(&m.agg_kind,
// AggKind::ExactAgg { agg_type: t, .. } if *t == agg_type)`.
// It NEVER matches `AggKind::Sketch` — so for any sketch-
// backed aggregation (DDSketch / KLL / HLL / CountSketch /
// CountMinSketch arriving via OTLP and registered by
// `route_modified_otlp_sketches_to_precompute` with
// `AggKind::Sketch { kind, config, .. }`), this lookup
// returns an empty map. We then bubble up "No precomputed
// outputs found for metric: X, aggregation_id: Y" and
// `handle_query` returns `None`, which the HTTP layer
// renders as `errorType: bad_data` / `error: "No result
// for query"`.
//
// Diagnosed in the e2e test arc (#247 → #248 → #249 →
// #250 → engine-path debug session 2026-05). Sketches DO
// reach `SketchStore` — `runtime_info.earliest_timestamp_per_sid`
// shows them — but they're only readable via the sid-keyed
// `SketchStore::query_range(sid, ...)` path (sketch payloads
// filtered by `payload.as_sketch()`), not via the agg-keyed
// precomputes path consumed here.
//
// **The fix** is to dispatch by the agg's capability:
// * sketch-typed aggs route to a sketch-side query (a
// counterpart to `query_precomputes_by_agg` that scans
// `AggKind::Sketch` sids and assembles per-sketch results
// into the engine's expected `Box<dyn AggregateCore>`
// shape) — non-trivial since the existing pipeline expects
// precompute payload shapes.
// * OR route the legacy `handle_query` path through the
// newer `ASAPQueryEngine::execute(&str)` trait path
// (around line 3430), which already does
// `idx.sids_for_policy(fp)` + reducer dispatch and
// handles sketches natively via `SketchReducer::evaluate`.
let Some(idx) = self.sketch_index.as_ref() else {
return Ok(TimestampedBucketsMap::new());
};
Expand Down
26 changes: 26 additions & 0 deletions data_plane/src/storage_engines/sketch_db/index/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,32 @@ impl SketchStore {
// the engine's query-side filtering (label matchers) handles
// that. Returning the superset is correct; over-returning is
// just a perf cost the engine already absorbs.
//
// ── KNOWN GAP — sketch-backed aggs return empty here ────────
// The `matches!` predicate below ONLY matches
// `AggKind::ExactAgg`. Sketch-backed sids
// (`AggKind::Sketch { kind, config, .. }`, registered by
// `route_modified_otlp_sketches_to_precompute` for every
// OTLP DDSketch/KLL/HLL/CountSketch/CountMinSketch DP) are
// NEVER picked up — and the agg-keyed precompute query
// unconditionally returns an empty map for them. The
// legacy `ASAPQueryEngine::handle_query` path
// (`query_engines/asap_query_engine/engine.rs::execute_store_query`)
// falls through to "No precomputed outputs found" → the
// HTTP layer renders `errorType: bad_data` / `error: "No
// result for query"`. Sketches ARE in `SketchStore` and
// are readable via the sid-keyed `query_range(sid, ...)`
// path — they just aren't reachable via this agg-keyed
// precompute lookup. Closing the gap means either teaching
// this function to also collect sketch payloads (assemble
// `Box<dyn AggregateCore>` from `payload.as_sketch()`),
// OR routing `handle_query` through the newer
// `ASAPQueryEngine::execute(&str)` trait path which uses
// `idx.sids_for_policy(fp)` + reducer dispatch and
// already handles sketches natively.
//
// Diagnosed in the e2e test arc (#247 → #248 → #249 →
// #250 → engine-path debug session 2026-05).
let candidate_sids: Vec<u64> = {
let g = self.instances.read().unwrap();
g.iter()
Expand Down