From 62ed65e41430f55d27eb6629e99ff9250f9688dd Mon Sep 17 00:00:00 2001 From: Zeying Zhu Date: Thu, 7 May 2026 18:32:23 -0400 Subject: [PATCH] fix: warm-tier SimpleEngine answers canonical quantile_over_time + rate queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `DDSketch` to `compatible_agg_types(Statistic::Quantile)` and `HLL` to `compatible_agg_types(Statistic::Cardinality)` so capability matching resolves runtime sketch-emitted aggs that the planner's canonical map (`map_statistic_to_precompute_operator`) does not emit. The MVP demo's controller plans `http_requests_total_latency_ms` as a `DDSketch` directly from `mvp-workload.yaml` and routes the delta payloads through the modified-OTLP wire format; pre-fix, `quantile_over_time(0.99, http_requests_total_latency_ms[1m])` would miss capability matching whenever the inference YAML's exact-string match didn't cover the live phi/range pair, and the warm tier returned `EngineError::CapabilityMiss`. The runtime accumulator surface (`DDSketchAccumulator::query_statistic`, `HllSketchAccumulator::query_statistic`) already supports these statistics — this PR just enumerates the types that match. Two regression tests pin the fix: - `asap_types::capability_matching::ddsketch_resolves_quantile_query_post_fix` unit-test for the matcher. - `query_engine_rust::tests/inference_yaml_pattern_coverage::canonical_mvp_demo_quantile_over_time_resolves_via_capability_matching` end-to-end through `SimpleEngine::handle_query_promql` with a DDSketch agg whose `query_config` deliberately mismatches the live request — forces the capability-matching branch. `five_sketch_canonical_statistics_in_compat_list` extended to assert the new DDSketch / HLL membership inline so future divergence fails the build. Pre-existing test failures (lib: 33 / asap_types: 1 flaky `avg_finds_sum_and_count` HashMap-iteration-order test / integration: `kll_envelope_round_trip_through_backend_adapter`) unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../rs/asap_types/src/capability_matching.rs | 95 ++++++++++++++++++- .../tests/inference_yaml_pattern_coverage.rs | 60 ++++++++++++ 2 files changed, 154 insertions(+), 1 deletion(-) diff --git a/asap-common/dependencies/rs/asap_types/src/capability_matching.rs b/asap-common/dependencies/rs/asap_types/src/capability_matching.rs index fcd78587..bcc8c3d8 100644 --- a/asap-common/dependencies/rs/asap_types/src/capability_matching.rs +++ b/asap-common/dependencies/rs/asap_types/src/capability_matching.rs @@ -131,13 +131,41 @@ pub fn compatible_agg_types(stat: Statistic) -> &'static [AggregationType] { Statistic::Min | Statistic::Max => { &[AggregationType::MinMax, AggregationType::MultipleMinMax] } - Statistic::Quantile => &[AggregationType::DatasketchesKLL, AggregationType::HydraKLL], + // Quantile: KLL (planner-emitted canonical pick) plus the + // sketch types whose accumulators answer `Statistic::Quantile` + // natively but that the planner's canonical map does not + // emit. The MVP demo's controller (`ASAPCollector/controller`) + // plans `http_requests_total_latency_ms` as a `DDSketch` + // directly from `mvp-workload.yaml` and routes the resulting + // delta payloads through the modified-OTLP wire format + // (DDSketch state landing in `DDSketchAccumulator`, which + // supports `Statistic::Quantile` — see + // `precompute_operators/dd_sketch_accumulator.rs`). Without + // DDSketch enumerated here, capability matching for an + // out-of-YAML query like `quantile_over_time(0.99, + // http_requests_total_latency_ms[1m])` would miss and the + // warm-tier engine returns `EngineError::CapabilityMiss`. + Statistic::Quantile => &[ + AggregationType::DatasketchesKLL, + AggregationType::HydraKLL, + AggregationType::DDSketch, + ], Statistic::Rate | Statistic::Increase => { &[AggregationType::Increase, AggregationType::MultipleIncrease] } + // Cardinality: SetAggregator / DeltaSetAggregator are the + // exact key trackers; HLL is the canonical approximator + // whose accumulator answers `Statistic::Cardinality` (and + // `Statistic::Count` as a cardinality alias) — see + // `precompute_operators/hll_sketch_accumulator.rs`. HLL is + // not in the planner's canonical map (it's wired in via + // modified-OTLP from the agent processors) but the runtime + // accumulator surface still resolves it, so list it here so + // capability matching can pick it up. Statistic::Cardinality => &[ AggregationType::SetAggregator, AggregationType::DeltaSetAggregator, + AggregationType::HLL, ], Statistic::Topk => &[AggregationType::CountMinSketchWithHeap], } @@ -1019,6 +1047,23 @@ mod tests { compatible_agg_types(Statistic::Quantile).contains(&AggregationType::DatasketchesKLL), "KLL must be a compatible type for Quantile", ); + // DDSketch → Quantile (Phase-3.1 fix). Required so the MVP demo's + // `quantile_over_time(0.99, http_requests_total_latency_ms[1m])` + // — which routes a DDSketch agg from `mvp-workload.yaml` and may + // miss the inference-YAML exact-string match — still resolves + // through capability matching instead of returning a 404. + assert!( + compatible_agg_types(Statistic::Quantile).contains(&AggregationType::DDSketch), + "DDSketch must be a compatible type for Quantile (Phase-3.1 fix)", + ); + // HLL → Cardinality (Phase-3.1 fix). HLL accumulators answer + // `Statistic::Cardinality` natively (and `Statistic::Count` as a + // cardinality alias); enumerating them here lets capability + // matching pick up an HLL-only deploy. + assert!( + compatible_agg_types(Statistic::Cardinality).contains(&AggregationType::HLL), + "HLL must be a compatible type for Cardinality (Phase-3.1 fix)", + ); // CMS → Sum (the headline bug fix that motivated this PR) assert!( compatible_agg_types(Statistic::Sum).contains(&AggregationType::CountMinSketch), @@ -1037,6 +1082,54 @@ mod tests { ); } + /// Phase-3.1 regression test for the canonical MVP-demo failure + /// described in `docs/spec-mvp-controller-driven-multi-stage-demo.md`: + /// the controller plans `http_requests_total_latency_ms` as a + /// `DDSketch` for the `quantile_over_time(0.99, + /// http_requests_total_latency_ms[1m])` query class. When the + /// inference YAML doesn't include an exact-string entry for the + /// query, `find_query_config` misses and the engine falls into + /// capability matching. Pre-fix, `compatible_agg_types(Quantile)` + /// listed only KLL types, so the DDSketch agg was filtered out + /// and the warm-tier engine returned a 404 / null; post-fix, + /// DDSketch is enumerated and capability matching resolves the + /// agg cleanly. + #[test] + fn ddsketch_resolves_quantile_query_post_fix() { + let mut configs = HashMap::new(); + configs.insert( + 42, + make_config( + 42, + "http_requests_total_latency_ms", + "DDSketch", + "", + 60, + "tumbling", + &[], + "", + ), + ); + let result = find_compatible_aggregation( + &configs, + &req( + "http_requests_total_latency_ms", + &[Statistic::Quantile], + Some(60_000), + &[], + "", + ), + ); + let info = result.expect( + "post-fix: capability matching must resolve quantile_over_time against a DDSketch-only config", + ); + assert_eq!(info.aggregation_id_for_value, 42); + assert_eq!(info.aggregation_type_for_value, AggregationType::DDSketch); + // DDSketch is single-population (not is_multi_population_value_type), + // so the matcher pairs it with itself for the key agg. + assert_eq!(info.aggregation_id_for_key, 42); + } + /// Regression test for the pre-fix bug: a query for `Statistic::Sum` /// against a CMS-only configuration must now resolve via capability /// matching, not fall through to the cold tier. Pre-fix, this returned diff --git a/asap-query-engine/tests/inference_yaml_pattern_coverage.rs b/asap-query-engine/tests/inference_yaml_pattern_coverage.rs index 4ef9f609..519813ca 100644 --- a/asap-query-engine/tests/inference_yaml_pattern_coverage.rs +++ b/asap-query-engine/tests/inference_yaml_pattern_coverage.rs @@ -467,6 +467,66 @@ fn spatial_sum_routes_through_warm_tier() { assert!(!elements.is_empty(), "sum() result should not be empty"); } +/// Phase-3.1 regression: pin the canonical MVP-demo PromQL query +/// `quantile_over_time(0.99, http_requests_total_latency_ms[1m])` +/// against a DDSketch-typed agg WITHOUT a matching `query_config` +/// exact-string entry. This forces capability-based matching +/// (`find_compatible_aggregation`) — pre-fix, this branch returned +/// `None` because `compatible_agg_types(Quantile)` listed only KLL +/// types. Post-fix, DDSketch is enumerated and the warm tier +/// answers the query cleanly. Mirrors the demo configuration +/// described in `ASAPCollector/docs/spec-mvp-controller-driven-multi-stage-demo.md` +/// (DDSketch for `_latency_ms` quantile-over-time at the edge). +/// +/// NOTE: schema labels and grouping labels are kept empty so the +/// `OnlyTemporal` `requirements.grouping_labels` (= all schema +/// labels) matches the config's grouping labels exactly under the +/// strict-equality `labels_compatible` check. The labels-superset +/// relaxation TODO'd in `capability_matching.rs:238` is out of +/// scope for this fix. +#[test] +fn canonical_mvp_demo_quantile_over_time_resolves_via_capability_matching() { + init_test_tracing(); + let acc = make_dd_acc(&[10.0, 20.0, 30.0, 40.0, 50.0, 60.0, 70.0, 80.0, 90.0, 100.0]); + let engine = build_engine( + "http_requests_total_latency_ms", + &[], + AggregationType::DDSketch, + &[], + 60, // 1m window + acc, + // `query_config` query string deliberately mismatches the live + // request below so `find_query_config` misses and the engine + // falls through to `find_compatible_aggregation`. + "quantile_over_time(0.5, http_requests_total_latency_ms[5m])", + ); + + let result = engine + .handle_query_promql( + // The MVP-demo canonical query — neither phi nor range + // overlaps with the registered query_config above. + "quantile_over_time(0.99, http_requests_total_latency_ms[1m])".to_string(), + QUERY_TIME_SEC, + ) + .expect( + "warm tier must resolve quantile_over_time against a DDSketch-only config via capability matching", + ); + let (_, qr) = result; + let elements = match qr { + query_engine_rust::engines::QueryResult::Vector(iv) => iv.values, + other => panic!("expected vector, got {other:?}"), + }; + assert!( + !elements.is_empty(), + "expected non-empty p99 result — capability matching now picks DDSketch for Quantile", + ); + let p99 = elements[0].value; + assert!( + p99.is_finite() && (50.0..=110.0).contains(&p99), + "p99 out of plausible range for [10..100]: {p99}", + ); +} + #[test] fn spatial_multi_quantile_routes_through_warm_tier() { // p50 spatial — pre-PR only p99 had an entry, so this would