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
Original file line number Diff line number Diff line change
Expand Up @@ -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],
}
Expand Down Expand Up @@ -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),
Expand All @@ -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
Expand Down
60 changes: 60 additions & 0 deletions asap-query-engine/tests/inference_yaml_pattern_coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down