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
55 changes: 52 additions & 3 deletions data_plane/src/query_engines/asap_query_engine/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3424,10 +3424,42 @@ fn stitch_warm_and_archive(

fn asap_tier_result_to_query_result(
result: crate::storage_engines::sketch_db::query::ASAPTierResult,
_now_ms: u64,
now_ms: u64,
is_range_query: bool,
) -> crate::query_engines::query_result::QueryResult {
use crate::storage_engines::types::KeyByLabelValues;
use crate::query_engines::query_result::{QueryResult, RangeVectorElement};
use crate::query_engines::query_result::{
InstantVectorElement, QueryResult, RangeVectorElement,
};

// Instant-query result-shape: the Prometheus adapter's
// `format_success_response` rejects `Matrix` for queries the
// analyzer marked as instant (`range_seconds == 0`) — produces a
// 500 ”shape mismatch”. Project the per-series last sample into
// an `InstantVectorElement` and wrap as `Vector` so the wire
// response carries `resultType: vector` matching the request.
if !is_range_query {
let mut elements: Vec<InstantVectorElement> = Vec::with_capacity(result.series.len());
for (label_values, samples) in result.series {
let (_keys, values): (Vec<String>, Vec<String>) = label_values.into_iter().unzip();
let labels = KeyByLabelValues::new_with_labels(values);
// Take the latest sample (the reducer returns one per
// window_end; for instant readout we want the most recent).
// `InstantVectorElement` doesn't carry a per-element
// `label_keys_override` today (only `RangeVectorElement`
// does, for the topk-`item`-key case) — labels render
// with whatever query-scoped `KeyByLabelNames` the
// serializer holds. That's correct for the cardinality
// shape that's the only instant-vector consumer at the
// moment; if a future instant-vector readout needs
// per-element key remapping, add the override field on
// `InstantVectorElement` then plumb `keys` here.
if let Some((_, value)) = samples.into_iter().last() {
elements.push(InstantVectorElement::new(labels, value));
}
}
return QueryResult::vector(elements, now_ms);
}

let mut elements: Vec<RangeVectorElement> = Vec::with_capacity(result.series.len());
for (label_values, samples) in result.series {
Expand Down Expand Up @@ -3553,6 +3585,16 @@ impl crate::query_engines::routing::query_engine_routing::QueryEngine for ASAPQu
let mut combined_result: Option<crate::storage_engines::sketch_db::query::ASAPTierResult> =
None;
let mut combined_t0: u64 = u64::MAX;
// Track whether ANY candidate is range-vector-shaped
// (`range_seconds > 0`). Drives the Vector-vs-Matrix
// result-shape choice in `asap_tier_result_to_query_result`
// below — instant queries (`count(metric)`,
// `quantile(...)` without `_over_time` etc.) need
// `QueryResult::Vector` so the Prometheus adapter's
// `format_success_response` wraps them as `resultType:
// vector`. Returning `Matrix` for an instant query
// produces a 500 (adapter rejects the shape mismatch).
let mut any_range_candidate = false;

// Snapshot the streaming config once for this query's
// policy lookups. Hot-reload swaps the underlying Arc; the
Expand All @@ -3561,6 +3603,9 @@ impl crate::query_engines::routing::query_engine_routing::QueryEngine for ASAPQu
let policy_registry = streaming_snap.policy_registry();

for candidate in &analysis.candidates {
if candidate.range_seconds > 0 {
any_range_candidate = true;
}
// Content-addressed sid lookup: find matching policies
// in the registry → resolve each policy_fp → {sids}
// via the reverse index. Both hops are O(1)-amortized.
Expand Down Expand Up @@ -3722,7 +3767,11 @@ impl crate::query_engines::routing::query_engine_routing::QueryEngine for ASAPQu
// QueryResult and run the hybrid-stitch path if archive
// is wired and warm coverage is narrower than request.
if let Some(result) = combined_result {
let warm_qr = asap_tier_result_to_query_result(result.clone(), now_ms);
let warm_qr = asap_tier_result_to_query_result(
result.clone(),
now_ms,
any_range_candidate,
);
if let (Some((cov_lo, cov_hi)), Some(archive)) =
(result.coverage, self.archive_engine.as_ref())
{
Expand Down
40 changes: 10 additions & 30 deletions data_plane/tests/e2e_controller_plans_and_backend_serves.rs
Original file line number Diff line number Diff line change
Expand Up @@ -968,37 +968,17 @@ async fn controller_plan_to_query_full_roundtrip_kll() {
//
// HLL backs the cardinality readout. The workload pins HLL via
// `sketch_type_override: Some(SketchType::HLL)`. The OTLP DP carries
// a `HllSketchDataPoint` with `HyperLogLogState`.
// a `HllSketchDataPoint` with `HyperLogLogState`. PromQL's
// `count(metric)` is the spec's distinct-counting idiom — returns
// the number of distinct label sets in the result vector — which
// the analyzer routes to `Capability::CardinalityApprox` and the
// reducer dispatches to the HLL cardinality readout.
//
// PromQL's `count(metric)` is the spec's distinct-counting idiom —
// it returns the number of distinct label sets in the result vector.
// Three engine-side fixes were needed (alongside this PR):
//
// 1. **Analyzer (`walk_qe::Expr::VectorSelector`)** — gated the
// implicit `Aggregate(Sum)` wrapper on `!ctx.outer_count` so a
// bare selector under `count(...)` doesn't synthesize a spurious
// `ExactAgg(Sum)` candidate that fails the engine's
// "all candidates must succeed" loop.
// 2. **Reducer (`function_to_family`)** — added `"count"` as an
// alias for `QueryFamily::Cardinality`.
// 3. **Test setup** — OTLP DP precision must match what the
// controller plans (`HLLDefaults`); start_time must be near
// end_time so the stored window falls within the query's
// lookback range.
//
// **Currently `#[ignore]`'d.** With all three fixes in place the
// engine path now goes the distance: streaming-config registers,
// OTLP DP lands in `SketchStore`, sids share the right `policy_fp`,
// reducer.evaluate returns `Ok(...)`. But the HTTP response body
// comes back empty / fails JSON decode (`reqwest::Error: EOF while
// parsing a value`) — the response-serialization path for
// instant-vector cardinality results has a separate bug worth its
// own follow-up. Tracked via the diagnostic comments above and the
// engine-debug prints kept in the engine path's git history.

#[ignore = "HLL roundtrip — analyzer, policy match, reducer all succeed; \
HTTP response body is empty. Separate serialization bug \
in the cardinality response path."]
// Closed by a chain of fixes:
// * `count(metric)` analyzer fix (PR #255)
// * `count` reducer alias (PR #255)
// * Vector-vs-Matrix instant-query response shape fix (this PR)

#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
async fn controller_plan_to_query_full_roundtrip_hll() {
let stack = start_full_stack(19_565, 19_566).await;
Expand Down