Skip to content

fix(warm-engine): answer replay queries under empty-schema deploy (was status=error) - #111

Merged
zzylol merged 1 commit into
mainfrom
fix/warm-engine-error-on-replay-queries
May 8, 2026
Merged

zzylol merged 1 commit into
mainfrom
fix/warm-engine-error-on-replay-queries

Conversation

@zzylol

@zzylol zzylol commented May 8, 2026

Copy link
Copy Markdown
Contributor

Summary

The MVP demo's replay.jsonl (/tmp/asap-mvp-rerun-postE/asap/measurements/replay.jsonl) showed the warm-tier engine returning status=error for nearly every replay query shape:

kind replay query rows status
quantile quantile_over_time(0.99, http_requests_total_latency_ms[1m]) 686/686 error
count_unique count(unique_users_per_min) 343/343 error
frequency rate(endpoint_request_freq[5m]) 342/342 error
topk topk(5, top_endpoint_qps) 342/342 error
sum (instant) sum by (zone) (http_requests_total) 343/343 error
sum (rate) sum by (zone) (rate(http_requests_total[5m])) 0/343 success (cold tier)

PR #108's existing regression tests passed but did not mirror the production deploy shape — they seeded a populated PromQLSchema plus QueryConfig templates, so the engine never hit the schema-empty fallback paths the MVP harness exercises.

Root cause

The production warm-tier binary launches with --streaming-config=/etc/asap/streaming.yaml and no --config, so inference_config.schema = PromQLSchema::new() (empty) and inference_config.query_configs = []. Two engine paths blow up on this:

  1. build_query_requirements_promql looked up the metric in inference_config.schema, fell back to KeyByLabelNames::empty() when missing, and built req.grouping_labels = []. The strict-equality labels_compatible then mismatched every controller-emitted agg config's [zone] grouping → capability miss → handle_query_promql returns Noneformat_unsupported_query_response writes status=error with http_code=200 (the exact replay.jsonl shape).

  2. build_promql_execution_context_tail had its OWN schema lookup that returned None and short-circuited the whole query — even after capability matching had succeeded for queries like sum by (zone) (counter).

  3. compatible_agg_types(Statistic::Count) did not list HLL, so count(<HLL-metric>) capability-missed even though HllSketchAccumulator answers Statistic::Count as a cardinality alias (hll_sketch_accumulator.rs:220).

  4. compatible_agg_types(Statistic::Topk) did not list CountSketch, so topk(K, <CountSketch-metric>) capability-missed even though CountSketchAccumulator answers Statistic::Topk (count_sketch_accumulator.rs:284).

Fixes

  • New SimpleEngine::resolve_metric_labels(metric) helper (engines/simple/engine.rs) that prefers the user-supplied schema and falls back to a deterministic union of grouping_labels across every StreamingConfig agg matching the metric. Both downstream call sites (build_query_requirements_promql, build_promql_execution_context_tail) now route through it.

  • Relax labels_compatible (asap_types/src/capability_matching.rs) from strict-eq to subset (req ⊆ config). The TODO on this function asked for exactly this; the MVP demo's count(metric) / topk(K, metric) queries (no by (...) modifier) translate to req.grouping_labels = [] and must match a [zone] agg via the merge path. Direction is asymmetric: req ⊃ config is still rejected (engine cannot invent partitions).

  • Add HLL to Statistic::Count compat list + add CountSketch to Statistic::Topk compat list.

  • Tests: 5 new production_conditions_* tests in tests/datafusion/warm_engine_replay_regression_tests.rs reproducing each replay failure shape against an inference_config with empty PromQLSchema (mirroring the warm-tier deploy). 3 pass; 2 are #[ignore]'d as out-of-scope follow-ups (CountSketch needs paired SetAggregator under is_multi_population_value_type — PR harden(ingest): validate inbound CMS/CountSketch wire dimensions (defensive) #344 declares the capability on the controller side; CMS does not yet implement Statistic::Rate).

Test plan

  • cargo test --release --lib -p query_engine_rust warm_engine_replay_regression_tests → 10 pass / 0 fail / 2 ignored (was 7 pass / 0 fail / 0 ignored on origin/main; 4 new prod-conditions tests, 3 pass, 1 ignored)
  • cargo test --release --lib -p asap_types capability_matching → 38 pass / 0 fail (modulo a pre-existing flaky test avg_finds_sum_and_count that depends on HashMap iteration order, also flaky on origin/main)
  • cargo test --release -p query_engine_rust --test inference_yaml_pattern_coverage → 11/11 pass (no regressions)
  • cargo build --release → green
  • Full cargo test --release --lib -p query_engine_rust failure set unchanged from origin/main (33 pre-existing failures, none related to this fix)
  • Live curl evidence: not captured — running the MVP demo end-to-end requires rebuilding asap/query-backend:dev and re-running the soak harness; the unit tests pin the wire shape (expect("warm engine must answer …") against handle_query_promql).

Honest follow-ups (couldn't fix in one PR)

  • topk(5, top_endpoint_qps): standalone CountSketch is is_multi_population_value_type and requires a paired SetAggregator/DeltaSetAggregator. The right structural fix is for the controller to plan top_endpoint_qps as CountMinSketchWithHeap (the integrated CMS+heap accumulator); PR harden(ingest): validate inbound CMS/CountSketch wire dimensions (defensive) #344 declares that capability on the controller side. Engine-side test marked #[ignore].
  • rate(endpoint_request_freq[5m]): CountMinSketchAccumulator does not yet implement Statistic::Rate. Tracking via #[ignore]'d test + TODO note.
  • sum_by_zone_instant_does_not_error flaky test (asap_types::capability_matching::tests::avg_finds_sum_and_count): pre-existing HashMap-ordering flake; should be stabilised by adding a tie-breaker to aggregation_priority. Out of scope for this PR.

🤖 Generated with Claude Code

…s status=error)

The MVP demo's `replay.jsonl` showed the warm-tier engine returning
`status=error` for nearly every query shape — quantile_over_time
(686/686), instant sum (343/343), count-of-HLL (343/343), topk
(342/342), frequency (342/342). PR #110's `_quantile` alias resolver
was correct in isolation but the production deploy hits a downstream
mismatch this PR diagnoses and fixes.

Root cause: the production warm-tier launches with `--streaming-config`
only and no `--config`, so `inference_config.schema = PromQLSchema::new()`
(empty). Two engine paths blow up on this:

1. `build_query_requirements_promql` falls back to
   `KeyByLabelNames::empty()` for `req.grouping_labels` → strict
   `labels_compatible` mismatches every agg config's `[zone]` →
   capability miss → `handle_query_promql` returns `None` →
   `format_unsupported_query_response` writes `status=error`.

2. `build_promql_execution_context_tail` had its own schema lookup
   (`schema.get_labels(metric)`) that returned `None` for empty
   schemas — even after capability matching succeeded — and short-
   circuited the entire query.

Fixes:

- New `SimpleEngine::resolve_metric_labels(metric)` helper that
  prefers the user-supplied schema and falls back to a deterministic
  union of `grouping_labels` across every `StreamingConfig` agg
  matching the metric. Both call sites above now use it, so the
  schema-empty deploy auto-derives sane labels from the controller-
  pushed agg configs.

- `labels_compatible` relaxed from strict-eq to subset (`req ⊆
  config`). The TODO on this function asked for exactly this; the
  MVP demo's `count(metric)` / `topk(K, metric)` queries with no
  `by (...)` modifier translate to `req.grouping_labels = []` and
  must match a `[zone]` agg via the merge path. Direction is
  asymmetric: `req ⊃ config` is still rejected (engine cannot
  invent partitions). The pre-existing `label_strict_superset_rejected`
  test is replaced with `label_superset_config_accepts_subset_query`
  + `label_subset_config_rejects_superset_query` to pin both halves.

- `compatible_agg_types(Statistic::Count)` now lists `HLL`. The
  HLL accumulator answers `Statistic::Count` as a cardinality alias
  (`hll_sketch_accumulator.rs:220`); without HLL in the compat list
  every `count(<HLL-metric>)` capability-missed.

- `compatible_agg_types(Statistic::Topk)` now lists `CountSketch`.
  Symmetric with the Count fix — `count_sketch_accumulator.rs:284`
  answers `Statistic::Topk`. (Note: a standalone CountSketch agg
  still fails downstream because `is_multi_population_value_type`
  requires a paired SetAggregator. The right structural fix for
  `top_endpoint_qps` is for the controller to plan it as
  `CountMinSketchWithHeap`; PR #344 declares that capability on
  the controller side. The engine-side test is `#[ignore]`'d
  with a follow-up note.)

Tests:

- `production_conditions_quantile_over_time_does_not_error`
- `production_conditions_sum_by_zone_instant_does_not_error`
- `production_conditions_count_against_hll_does_not_error`
- `production_conditions_topk_against_count_sketch_does_not_error`
  (`#[ignore]`'d — follow-up; see comment)
- `production_conditions_rate_against_cms_does_not_error`
  (`#[ignore]`'d — CountMinSketchAccumulator has no
  `Statistic::Rate` answer yet)
- `label_superset_config_accepts_subset_query` /
  `label_subset_config_rejects_superset_query`

Each new test seeds `inference_config.schema = PromQLSchema::new()`
to mirror the production warm-tier deploy shape.

Refs ProjectASAP/ASAPCollector#46.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 754562d into main May 8, 2026
zzylol added a commit that referenced this pull request May 9, 2026
…acts (#113)

Issue ProjectASAP/ASAPCollector#46 — agent emits 5 sketch envelopes (DDSketch /
KLL / HLL / CountSketch / CountMin) over the modified-OTLP wire, but four of
them never produced a non-empty answer from the warm tier even when the
streaming-config registered the matching aggregation. Two backend gaps:

1. **HLL `_hll` alias was not resolved.** The agent's HLL processor renames
   `unique_users_per_min` → `unique_users_per_min_hll` on egress, mirroring
   the existing DDSketch / KLL `_quantile` rename. The warm engine's
   `resolve_quantile_metric_alias` only handled `_quantile`, so
   `count(unique_users_per_min)` looked up the bare name, found nothing, and
   returned `status=error`. Generalised to `resolve_sketch_metric_alias` with
   a shape→suffix table; `Quantile` → `_quantile`, `Count` → `_hll`.

2. **`Statistic::Rate` over CountMinSketch was the documented PR #111 honest
   gap.** `compatible_agg_types(Rate)` excluded CMS, and
   `CountMinSketchAccumulator::query_statistic` rejected `Rate` outright. Wired
   both: capability matching now resolves `rate(metric[range])` to a CMS-only
   agg, the engine pushes `range_ms` through `query_kwargs`, and the
   accumulator divides the min-row-sum by `range_ms / 1000` to return events/
   second. When `range_ms` is absent (instant rate-shape that bypasses the
   matrix-selector code path) the accumulator falls back to the raw event
   count rather than erroring — answer is non-empty in events/window units,
   which is preferable to `status=error`.

The OTLP ingest decoder's per-variant dispatch (HLL / KLL / CountSketch /
CountMin / DDSketch) was already in place from PRs C / G; the residual gaps
were the two query-side issues above. Wire-side decode contracts pinned by
new unit tests (HLL count, KLL quantile, CMS rate capability, CMS rate
arithmetic).

PR #111 honest-gap call-outs that **remain open after this PR**:

* `topk(K, top_endpoint_qps)` over `CountSketch` still requires a paired
  `SetAggregator` to surface the keys. `CountSketchAccumulator::query_statistic`
  answers `Statistic::Topk` directly, but the SimpleEngine's keyed-merge path
  needs the keys side. Tracked under PR #111.
* `MSGPACK_DELTA` (encoding=4) for any sketch family is still
  `Err("MSGPACK_DELTA encoding is not yet wired")`. Tracked under PR I.

Test coverage:

* `precompute_operators::count_min_sketch_accumulator::tests` — 4 new tests
  pinning `Statistic::Rate` with/without `range_ms`, `Statistic::Increase`,
  and the invalid-`range_ms` error.
* `engines::simple::engine::sketch_alias_resolver_tests` — 8 new tests pinning
  the shape→suffix table: quantile / count rewrite, no-op when bare known,
  no-op when suffixed missing, topk/rate untouched, identifier-token
  preservation.
* `engines::simple::engine::hll_count_query_tests` — 4 new tests pinning
  `Statistic::Count`/`Cardinality` round-trip on `HllSketchAccumulator` and
  capability matching dispatching `count(...)` to HLL.
* `engines::simple::engine::kll_quantile_query_tests` — 1 new test pinning
  capability matching dispatching `quantile_over_time(...)` to
  DatasketchesKLL.
* `engines::simple::engine::cms_rate_capability_tests` — 1 new test pinning
  capability matching dispatching `rate(...)` to CountMinSketch and verifying
  `range_ms` lands in `query_kwargs`.

Total: +18 passing tests; baseline 932 → 950 lib tests passing. No regressions.

Live curl evidence — backend image rebuilt with this branch; against the
mvp-multi-stage stack with a runtime-pushed 5-sketch streaming-config
(`POST /api/v1/streaming-config` adding HLL / KLL / CountSketch / CountMin
agg_ids), the previously-error queries now route through capability matching
end-to-end. Live answer values still depend on agent → gateway →
backend traffic landing (the producer→agent network in this stack is flaky
in this environment, surfacing as "no result" in the response body rather
than the previous capability-miss error). The ingest + capability + query
contracts are pinned by the new unit tests.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol deleted the fix/warm-engine-error-on-replay-queries branch May 9, 2026 18:00
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