feat: per-Capability sketch reducer — warm-tier query evaluator - #124
Merged
Merged
Conversation
Replaces the all-Hit `CapabilityMiss` fall-through in the warm-tier classify hook (added in #122) with real sketch evaluation off SketchIndex.query_range. Queries answerable from sketches now skip the archive forward entirely. ## What landed ### `engines/warm_tier/sketch_reducer.rs` (559 lines) ```rust pub struct SketchReducer<'a> { pub index: &'a SketchIndex, } pub enum WarmTierError { UnsupportedFunction(String), UnsupportedCapability { function: String, capability: Capability }, DeserializeFailure { sid: u64, encoding: SketchEncoding, reason: String }, NoData { metric_name: String }, } pub struct WarmTierResult { pub series: Vec<(BTreeMap<String,String>, Vec<(i64, f64)>)>, } impl<'a> SketchReducer<'a> { pub fn evaluate( &self, sids: &[u64], function_name: &str, function_args: &[f64], t0_ms: u64, t1_ms: u64, ) -> Result<WarmTierResult, WarmTierError>; } ``` Per-Capability dispatch: | Capability | Function | Sketch lib calls | |---|---|---| | `QuantileApprox(DDSketch)` | `quantile_over_time`, `histogram_quantile` | `DdSketch::from_raw` + `DdSketch::quantile(q)` | | `QuantileApprox(Kll)` | `quantile_over_time`, `histogram_quantile` | `KllSketch::new(k)` + replay items + `quantile(q)` | | `CardinalityApprox` | `count_distinct_over_time`, `cardinality_estimate` | `HllSketch::from_raw` + `estimate()` | | `FrequencyTopk(CountMin/CountSketch)` | `topk`, `topk_over_time` | TODO — surfaced as `UnsupportedCapability` for now | Decoders cover `ProtoFull` (always) and `MsgpackFull` (DD/KLL/HLL). Delta encodings (`ProtoDelta` / `MsgpackDelta`) surface as `DeserializeFailure` because applying a delta requires the prior base snapshot, which `query_range` doesn't stitch. ### `engines/warm_tier/promql_extract.rs` (159 lines) `extract_promql_call(query) -> Option<PromqlCall>` walks the `promql_parser` AST and returns the outermost call's function name + leading numeric args. Supported shapes: `Call(func, args)`, `Aggregate(op, param, expr)`, unwrapping `Paren` and `Subquery`. Bare `VectorSelector` / `MatrixSelector` → empty function name (treated as CapabilityMiss). Binary ops / nested calls beyond outermost → None (CapabilityMiss). ### `engines/simple/engine.rs` hook The Phase 5 warm-tier classify branch (added in #122) now does: ```rust if all_hit { let reducer = SketchReducer::new(idx); match reducer.evaluate(&candidates, &fn_name, &fn_args, t0_ms, t1_ms) { Ok(result) => return Ok(warm_tier_result_to_query_result(result)), Err(WarmTierError::UnsupportedFunction(_) | WarmTierError::UnsupportedCapability { .. } | WarmTierError::DeserializeFailure { .. } | WarmTierError::NoData { .. }) => { return Err(EngineError::CapabilityMiss(SketchWarmTier, ...)); } } } ``` Mismatch / decode failure / no data → CapabilityMiss → archive failover (existing EngineRouter behavior). ## Build + test - `cargo build --release -p query_engine_rust` — clean. - New tests (14, all pass): DDSketch quantile_over_time within ±5% rel-error; KLL exact for k ≤ 50 items; HLL within 5σ envelope of true cardinality; capability mismatch → UnsupportedCapability; empty/no-data → NoData; unsupported function → UnsupportedFunction; garbage proto → DeserializeFailure; multi-series shape; 5 promql_extract tests covering quantile/histogram/topk/bare/binary. - PR #122's 3 warm_tier_classify_tests still green. ## Follow-ups - Per-window merge for `*_over_time` queries within `[t0, t1]`. - `FrequencyTopk` + `topk(k, foo)` — needs `CmsWithHeap` SketchKindHandle variant. - Delta encoding stitching — needs base-snapshot lookup. - Hybrid stitch (warm `[t0..t1']` + archive `[t1'..t1]`) — needs `QueryResult` to carry timestamp coverage metadata. - `KeyByLabelValues` projection currently flattens to value-only Vec<String>; revisit if label-key recovery is needed downstream. ## Depends on This PR will need to rebase onto #123 (datafusion removal + SketchIndex into sketch_db) when that lands. The rebase changes are mechanical: import paths `crate::stores::sketch_index::*` → `crate::stores::sketch_db::sketch_index::*` at two sites in `sketch_reducer.rs` + one site in `tests.rs`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
zzylol
added a commit
that referenced
this pull request
May 10, 2026
#124 merged before rebasing onto #123, which moved stores/sketch_index.rs → stores/sketch_db/sketch_index.rs. Mechanical fix: rewrite three import sites in the warm_tier module from crate::stores::sketch_index::* → crate::stores::sketch_db::sketch_index::*. Build clean post-fix; warm_tier tests still 14/14. Co-authored-by: zz_y <zz_y@node0.zz-y-304941.softmeasure-pg0.clemson.cloudlab.us> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
zzylol
added a commit
that referenced
this pull request
May 10, 2026
…stitch (#126) Three follow-ups from #124 land in one bundle. The warm-tier reducer now covers all four sketch families end-to-end (DD/KLL quantile, HLL cardinality, CMS-with-heap topk), applies delta-encoded windows by folding from the prior Full, and stitches with the archive engine when the warm tier's coverage is narrower than the query range. ## TODO 1 — FrequencyTopk (CMS-with-heap) [IMPLEMENTED] - New `SketchKindHandle::CmsWithHeap` variant in `stores/sketch_db/sketch_index.rs`. - `drivers/ingest/otel.rs::sketch_kind_handle_for` detects CmsWithHeap by attempting `CountMinSketchWithHeap::deserialize_msgpack(&dp.sketch)` on msgpack payloads and checking `topk_heap_items().is_empty()`. The heap is embedded in the msgpack wrapper, not on the OTLP wire (confirmed by inspecting metrics.proto::CountMinSketch). - New `engines/warm_tier/decoders.rs::decode_cms_with_heap_from_msgpack`: calls `asap_sketchlib::sketches::countminsketch_topk:: CountMinSketchWithHeap::deserialize_msgpack`, then `topk_heap_items()` for the heap. - CMS / CountSketch without heap surface as new `WarmTierError::MissingHeap { sid, sketch_kind }` — clearly distinguished from `UnsupportedCapability` so the EngineRouter's failover semantics can tell "no warm-tier handler for this function" from "warm tier has the data but lacks topk metadata". - `topk` output shape: one `(label_values, samples)` per item where `label_values["item"] = key` and `samples = [(window_end, estimated_count)]`. `k` from `function_args[0]`, default 10. ## TODO 2 — Delta encoding stitching [IMPLEMENTED] - New `engines/warm_tier/delta_apply.rs`: `RollingState` (DDSketch / KLL / HLL variants) + `per_window_evaluate` + `cumulative_evaluate`. - Sketch-lib calls used: `DdSketch::merge`, `KllSketch::merge`, `HllSketch::merge`, `HllSketch::apply_delta(&HllSketchDelta)` for proto register deltas, `HllSketch::deserialize_msgpack` for msgpack delta fragments. - Mode pick by function name: quantile_over_time / count_distinct_over_time / topk_over_time → cumulative (one scalar over [t0, t1]) quantile / histogram_quantile / cardinality_estimate → per-window - Leading deltas without a base Full are skipped (don't error); cumulative mode starts at the first Full in range. Tests verify DDSketch + HLL cumulative Full+Delta(s) round-trip within accuracy envelope of fresh-sketch ground truth. ## TODO 3 — Hybrid warm + archive stitch [IMPLEMENTED] - `WarmTierResult.coverage: Option<(u64, u64)>` populated from observed window-end timestamps. - `SimpleEngine.archive_engine: Option<Arc<dyn QueryEngine>>` field + `with_archive_engine` builder. The trait `execute` adapter inspects `result.coverage` post-evaluate; if narrower than `[t0, now]`, it calls `archive.execute(query)` and stitches. - Chose option (b): in-line stitch in `SimpleEngine` rather than a new `EngineError::PartialHit` variant. Reason: the existing `EngineRouter` is variant-agnostic; introducing PartialHit would force every other engine to also handle it. - Stitch logic: index warm series by `Vec<String>` label key, for each archive series merge by timestamp — warm wins on `[cov_lo, cov_hi]`, archive fills prefix/suffix. New private `stitch_warm_and_archive` helper. - Tests: `stitch_fills_archive_prefix_and_suffix` and `stitch_keeps_archive_only_series_in_full` verify both contracts. ## New API surface - `SketchKindHandle::CmsWithHeap` variant - `WarmTierError::MissingHeap { sid, sketch_kind }` variant - `WarmTierResult.coverage: Option<(u64, u64)>` field - `SimpleEngine::with_archive_engine(archive: Arc<dyn QueryEngine>)` builder - `engines::warm_tier::decoders` (new module) — 5 decode helpers - `engines::warm_tier::delta_apply` (new module) — `DeltaSketchKind`, `RollingState`, `per_window_evaluate`, `cumulative_evaluate` ## Build + test - `cargo build --release -p query_engine_rust` — clean (3 pre-existing warnings) - Targeted run (24/24 pass): engines::warm_tier + the new hybrid_stitch_tests + warm_tier_classify_tests - Full lib run: 814 ok, 4 pre-existing failed (schema_timeline_dispatch + persistence_integration), 1 pre-existing hang (hard_cap_back_pressure) ## Migration note Existing `quantile_over_time` test had to flip its function name to `quantile` (per-window) to match the new cumulative semantics — caller code in production may need similar migration if it depended on per-window emit from `quantile_over_time`. ## Remaining follow-ups - KllSketch proto-delta wire path: today `delta_apply` decodes `Delta` as "Full fragment of matching encoding" then merges. The asap_sketchlib KLL proto wire `KllState.items[]` is sparse-friendly but there's no canonical `KllDelta` proto. If downstream agents start emitting a true sparse KLL delta, revisit. - `decoders::decode_cms_from_{proto,msgpack}`, `decode_cs_from_{proto,msgpack}` are dead-coded for now (kept for the eventual point-query `frequency(key, foo)` path). Co-authored-by: zz_y <zz_y@node0.zz-y-304941.softmeasure-pg0.clemson.cloudlab.us> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
pushed a commit
that referenced
this pull request
May 11, 2026
#124's warm-tier topk reducer synthesizes an `"item"` label key for each top-k entry (the item identity), but the `warm_tier_result_to_query_result` adapter then collapsed the result's `BTreeMap<key,value>` to a `KeyByLabelValues` (values only — `Vec<String>`). The HTTP serializer pairs those values with KEYS from a query-scoped `KeyByLabelNames`, which carries the PromQL group-by clause — and the PromQL `topk(5, foo)` has NO group-by. So the synthesized `"item"` key disappeared and the PromQL response showed `"metric": {}` for every top-k entry. Fix: - `RangeVectorElement` gains an optional `label_keys_override: Option<Vec<String>>` field. Default `None` — all existing constructors unaffected. - `convert_range_result_to_prometheus` uses the override when present, falls back to the query-scoped `label_names` otherwise. - `warm_tier_result_to_query_result` populates the override from the BTreeMap's keys (BTreeMap iteration is key-sorted, so it pairs correctly with the values). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
zzylol
added a commit
that referenced
this pull request
May 11, 2026
#124's warm-tier topk reducer synthesizes an `"item"` label key for each top-k entry (the item identity), but the `warm_tier_result_to_query_result` adapter then collapsed the result's `BTreeMap<key,value>` to a `KeyByLabelValues` (values only — `Vec<String>`). The HTTP serializer pairs those values with KEYS from a query-scoped `KeyByLabelNames`, which carries the PromQL group-by clause — and the PromQL `topk(5, foo)` has NO group-by. So the synthesized `"item"` key disappeared and the PromQL response showed `"metric": {}` for every top-k entry. Fix: - `RangeVectorElement` gains an optional `label_keys_override: Option<Vec<String>>` field. Default `None` — all existing constructors unaffected. - `convert_range_result_to_prometheus` uses the override when present, falls back to the query-scoped `label_names` otherwise. - `warm_tier_result_to_query_result` populates the override from the BTreeMap's keys (BTreeMap iteration is key-sorted, so it pairs correctly with the values). Co-authored-by: zz_y <zz_y@node0.zz-y-304941.softmeasure-pg0.clemson.cloudlab.us> 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
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
Replaces the all-
HitCapabilityMissfall-through in the warm-tier classify hook (added in #122) with real sketch evaluation offSketchIndex.query_range. Queries answerable from sketches now skip the archive forward entirely.This is the last substantive piece of the centralized-sid refactor before MVP-demo end-to-end testing. With this in place, B0 / B1 / ASAP arms can answer the same PromQL on the same metric name and the demo's "ASAP gives same answer with less bandwidth" claim becomes testable.
Depends on
This PR will rebase onto #123 (datafusion removal + SketchIndex into sketch_db) once that merges. The rebase changes are mechanical: import paths
crate::stores::sketch_index::*→crate::stores::sketch_db::sketch_index::*at two sites insketch_reducer.rs+ one site intests.rs.What landed
engines/warm_tier/sketch_reducer.rs(559 lines)Per-Capability dispatch
QuantileApprox(DDSketch)quantile_over_time,histogram_quantileDdSketch::from_raw+DdSketch::quantile(q)QuantileApprox(Kll)quantile_over_time,histogram_quantileKllSketch::new(k)+ replay items +quantile(q)(matches precompute path's lossy proto reconstruction)CardinalityApproxcount_distinct_over_time,cardinality_estimateHllSketch::from_raw+estimate()FrequencyTopk(CountMin/CountSketch)topk,topk_over_timeUnsupportedCapabilityfor now (CMS-with-heap needs aSketchKindHandle::CmsWithHeapvariant)Decoders cover
ProtoFullalways andMsgpackFullfor DD/KLL/HLL. Delta encodings (ProtoDelta/MsgpackDelta) surface asDeserializeFailurebecause applying a delta requires the prior base snapshot, whichquery_rangedoesn't stitch yet.engines/warm_tier/promql_extract.rs(159 lines)extract_promql_call(query) -> Option<PromqlCall>walks thepromql_parserAST and returns the outermost call's function name + leading numeric args:Call(func, args)→("quantile_over_time", [0.99])etc.Aggregate(op, param, expr)→("topk", [5.0])etc.Paren/Subqueryunwrap.VectorSelector/MatrixSelector→func = ""(CapabilityMiss).None(CapabilityMiss).engines/simple/engine.rshookThe Phase 5 warm-tier classify branch (#122) now does:
Mismatch / decode failure / no data →
CapabilityMiss→ archive failover (existing EngineRouter behavior).Build + test
cargo build --release -p query_engine_rust— clean (only pre-existing warnings on the controller crate).quantile_over_time(3 windows, ±5% rel-error)quantile_over_time(1 window, exact for ≤ 50 items)cardinality_estimate(1000 distinct, 5σ envelope)topkonQuantileApprox) →UnsupportedCapabilityNoDatarate) →UnsupportedFunctionDeserializeFailurehost=values, 2 result series)promql_extracttests covering quantile/histogram/topk/bare/binarywarm_tier_classify_testsstill green.cargo test --release -p query_engine_rust --lib: 915 passed, 33 failed. The 33 failures are all pre-existing (28 datafusion tests going away in cleanup: remove datafusion + integrate SketchIndex into sketch_db #123, 2 schema_timeline_dispatch flakes, 2 persistence integration, 1 async-flaky e2e_feedback_loop_tests).Follow-ups
*_over_timequeries within[t0, t1](today: one scalar per window_end_unix_ms).FrequencyTopk+topk(k, foo)— needsCmsWithHeapSketchKindHandlevariant. CMS / CountSketch decoders are stubbed under#[allow(dead_code)]for trivial future-merge.[t0..t1']+ archive[t1'..t1]) — needsQueryResultto carry timestamp coverage metadata.QueryEngine::execute— trait surface doesn't pass(t0, t1); today the dispatch uses[now - 5min, now]. Range-query path should callSketchReducer::evaluatedirectly with its own bounds.🤖 Generated with Claude Code