Skip to content

feat(query): dispatch sum by (...) PromQL to ExactAgg(Sum) sids (closes Option-B downstream) - #291

Merged
zzylol merged 1 commit into
mainfrom
feat/sum-dispatch-to-exactagg-sids
May 18, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/sum-dispatch-to-exactagg-sids

Conversation

@zzylol

@zzylol zzylol commented May 18, 2026

Copy link
Copy Markdown
Contributor

Summary

After PR #290's Option B unified emit, the control plane mints
ExactAgg(Sum) sids for sum PromQL candidates and the engine resolves
them correctly via instances_matching — but sum by (zone) (http_requests_total)
still returned {\"error\":\"No result for query\"} because
SketchReducer::evaluate short-circuits with UnsupportedFunction(\"sum\"):
its function_to_family dispatch table only knows sketch-backed
families (quantile / cardinality / topk / frequency). ExactAgg sids
carry Box<dyn AggregateCore> payloads (SumAccumulator /
IncreaseAccumulator / …) rather than opaque sketch bytes, so they
need a parallel reducer entry point.

What changed

  • SketchStore::query_exact_agg_range(sid, t0, t1) — sister of
    query_range that yields per-(label_values_map, samples)
    Box<dyn AggregateCore> payloads with the full label keys
    preserved (unlike query_precomputes_by_agg, which flattens to a
    keys-erased KeyByLabelValues).
  • SketchReducer::evaluate_exact_agg(sids, agg_type, group_by_keys, t0, t1)
    projects each sid's window state onto the requested
    group_by_keys subset, merges per-(group, window) accumulators
    via AggregateCore::merge_with, and reads Statistic::Sum for
    additive types (Sum, Increase, plus the Multiple* variants).
    MinMax is deferred — disambiguating min vs max needs the outer
    PromQL function name, which the analyzer doesn't thread through
    yet; today the analyzer doesn't produce ExactAgg(MinMax) for
    sum by, so the defensive UnsupportedCapability is fine.
  • ASAPQueryEngine::execute and execute_range_promql_modern branch
    on candidate.required_capabilityExactAgg(_) → new path;
    sketch capabilities → unchanged SketchReducer::evaluate.

Smoke-test before / after

Before:

sum by (zone) (http_requests_total)
→ {\"error\":\"No result for query\"}

After (bash /mydata/mvp-smoke-test/run_smoke.sh):

sum by (zone) (http_requests_total)
→ [z0=48608, z1=48604, z2=44868, z3=44864]
   (resultType: vector, data_source: asap_query)

The B-2 / quantile / quantile_over_time queries still return the
expected per-zone DDSketch values (both instant and range forms) —
their dispatch path is unchanged (SketchReducer::evaluate for
sketch capabilities).

Known follow-up gaps (not in scope)

  • topk(5, sum by (zone) (rate(http_requests_total[5m]))) — the
    outer rate-over-Sum composition is not analyzer-supported as a
    single ASAP-tier candidate today; the query falls over to archive.
    Tracked separately.
  • count(http_requests_total) against the ExactAgg(Sum) sids —
    same answer-shape as sum, but the analyzer routes count to
    ExactAgg(Sum) only when paired with an inner that lowers to
    Sum; a bare count(metric) against a Sum-only metric isn't
    routed to ASAP yet.
  • ExactAgg(MinMax) requires threading the outer PromQL function
    name to disambiguate min vs max; deferred.

Test plan

  • cargo build -p data_plane — clean
  • cargo test -p data_plane --lib732 passed, 2 ignored, 0 failed
    (was 727 + 2 ignored; the 5 added: 4 reducer unit tests + 1 engine
    end-to-end test for execute(&str) → ExactAgg)
  • bash /mydata/mvp-smoke-test/run_smoke.sh — sum-by-zone passes,
    quantile baseline preserved

🤖 Generated with Claude Code

…es Option-B downstream)

After PR #290 the control plane mints ExactAgg(Sum) sids for `sum`
PromQL candidates, the analyzer hands the engine
`Capability::ExactAgg(Sum)`, and `instances_matching` resolves the
per-zone sids correctly — but `SketchReducer::evaluate`
short-circuits with `UnsupportedFunction("sum")` because its
`function_to_family` table only knows sketch-backed families. The
result was `{"error":"No result for query"}` for
`sum by (zone) (http_requests_total)` despite the sids existing.

This wires a parallel reducer entry point for ExactAgg capabilities:

* `SketchStore::query_exact_agg_range(sid, t0, t1)` — sister of
  `query_range` that yields per-(label_values_map, samples)
  `Box<dyn AggregateCore>` payloads with full label keys preserved
  (unlike `query_precomputes_by_agg`, which flattens to a
  keys-erased `KeyByLabelValues`).
* `SketchReducer::evaluate_exact_agg(sids, agg_type, group_by_keys,
  t0, t1)` — projects each sid's window state onto the requested
  `group_by_keys` subset, merges per-(group, window) accumulators
  via `AggregateCore::merge_with`, and reads `Statistic::Sum` for
  additive types (`Sum`, `Increase`, plus the `Multiple*` variants).
* `ASAPQueryEngine::execute` and `execute_range_promql_modern`
  branch on `candidate.required_capability` — ExactAgg → new path;
  sketch capabilities → unchanged `SketchReducer::evaluate`.

Regression coverage in
`storage_engines::sketch_db::query::tests`:
* `evaluate_exact_agg_sums_per_group_across_zones` — the smoke-test
  shape (4 zones × Sum, each its own sid).
* `evaluate_exact_agg_collapses_subgroups_into_requested_groups` —
  multi-rack subgroups collapse to a single zone group.
* `evaluate_exact_agg_unsupported_capability_for_minmax` — MinMax
  defers (no outer-function disambiguation yet) so it falls over to
  archive cleanly.
* `evaluate_exact_agg_no_data_when_window_empty` — empty window
  surfaces NoData (router falls over).
* `execute_sum_by_zone_dispatches_to_exact_agg_reducer` —
  end-to-end via the `execute(&str)` trait surface mirroring the
  MVP smoke test's Axis C.

Smoke test (`bash /mydata/mvp-smoke-test/run_smoke.sh`) verified
post-fix:
* sum-by-zone returns 4 entries (z0=48608, z1=48604, z2=44868,
  z3=44864) routed via `data_source: asap_query`.
* quantile instant + range still return per-zone DDSketch values.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 0b8b826 into main May 18, 2026
zzylol added a commit that referenced this pull request May 18, 2026
…inode topk dispatch) (#292)

Closes the rate-over-Sum gap PR #291 documented as the remaining
blocker for the multinode demo's `topk(5, sum by (zone)
(rate(http_requests_total[5m])))` query. The three target shapes
now land on the ExactAgg(Sum) sids the control plane mints for
counter metrics (no more falling over to the archive's `NoData`
stub):

* `rate(http_requests_total[5m])`
* `sum by (zone) (rate(http_requests_total[5m]))`
* `topk(5, sum by (zone) (rate(http_requests_total[5m])))`

Three data-plane changes:

1. `SketchReducer::evaluate_exact_agg_rate` (sister of #291's
   `evaluate_exact_agg`) — folds EVERY in-window sub-window
   accumulator per group into ONE merged accumulator, reads
   `Statistic::Sum`, then divides by `range_seconds` to produce
   events-per-second. Emits one sample per group, timestamped at
   the right edge of the request window (instant-rate semantics).
   Guards `range_seconds == 0` and the MinMax agg-type as
   `UnsupportedCapability`.

2. `ASAPQueryEngine::execute` + `execute_range_promql_modern`
   compose rule — when a candidate is `ExactAgg(Sum-family)` with
   `range_seconds > 0` AND the raw PromQL contains a
   `rate(...)` / `irate(...)` call (walked via `promql_parser`),
   dispatch to `evaluate_exact_agg_rate` instead of the plain
   per-window `evaluate_exact_agg`. The PromQL walker is the
   disambiguator the analyzer can't provide on its own:
   `sum by (zone) (rate(metric[5m]))` and
   `sum by (zone) (sum_over_time(metric[5m]))` both arrive at the
   engine as `function="sum"` + `range_seconds=300` + `ExactAgg(Sum)`
   but only the first one should divide by 300.

3. `ASAPQueryEngine::try_topk_over_rate_fallback` — the
   control-plane analyzer rewrites `topk(K, sum by (gbk)
   (rate(metric[r])))` into `FrequencyTopk` + `FrequencyEstimate`
   candidates (PromQL `topk` lowers to `AggIntent::TopK`, which the
   optimizer's CMS-topk binder pins to a frequency family). Those
   capabilities don't match ExactAgg(Sum) sids — the analyzer-
   driven path would CapabilityMiss. The fallback lifts
   `(K, metric, group_by_keys, range_seconds, is_topk)` from the
   raw PromQL, finds ExactAgg(Sum) sids via `instances_matching`,
   runs `evaluate_exact_agg_rate`, then post-applies the descending
   top-K slice (or ascending bottom-K) in-engine.

Plus a routing-table override: `resolve_metric_storage` in the
HTTP server now flips the routing decision from `Gorilla` →
`SketchStore` when the table sends a `RatePostHoc` / `Topk`-shaped
query to the archive AND the sketch index has ExactAgg(Sum) sids
for the metric. The control plane's `build_routing_entry`
unconditionally puts `rate_post_hoc` on the archive's claim list
(based on the sketch-family-only assumption that pre-dated
ExactAgg sids) — flipping it back at request time avoids the
control-plane churn the prompt's `DO NOT touch control_plane`
constraint mandated.

Test coverage:

* 7 new `sketch_reducer` tests pin `evaluate_exact_agg_rate`:
  divide-by-range, per-group across zones, rack→zone subgroup
  collapse, no-gbk per-sid preservation, zero-range and MinMax
  unsupported-capability, empty-window NoData.
* 4 new `asap_query_engine` end-to-end tests pin the engine
  dispatch: `rate(...)` direct, `sum by (zone) (rate(...))`,
  `topk(5, ...)` fallback (K≥n), `topk(2, ...)` fallback truncation.

Smoke-test verification (`bash /mydata/mvp-smoke-test/run_smoke.sh`):
* `rate(http_requests_total[5m])` → 4 zones × per-zone rate
  (~226.74, 226.74, 209.28, 209.26 events/sec) via
  `data_source: asap_query` (previously empty thanos_query).
* `sum by (zone) (rate(http_requests_total[5m]))` → same 4
  zones via `data_source: asap_query`.
* `topk(5, sum by (zone) (rate(http_requests_total[5m])))` →
  all 4 zones (k≥n) descending via `data_source: asap_query`.
* `topk(2, ...)` → top 2 entries only.
* REGRESSION: `sum by (zone) (http_requests_total)` (PR #291)
  and `quantile_over_time(0.99, http_requests_total_latency_ms[5m])`
  (PR #290) — both still return correct per-zone results via
  `data_source: asap_query`.

`cargo test -p data_plane --lib`: 743 passed (732 baseline +
7 reducer + 4 engine), 0 failed, 2 ignored. Control plane
untouched (738 passed, unchanged).

`irate` falls out of this for free (the PromQL walker matches
both `rate` and `irate`).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol deleted the feat/sum-dispatch-to-exactagg-sids branch July 17, 2026 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant