Skip to content

refactor(control_plane): unify Replanner through typed cumulative emit; retire generate_streaming_config_yaml (Option B) - #290

Merged
zzylol merged 1 commit into
mainfrom
feat/unify-replanner-typed-cumulative-emit
May 18, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/unify-replanner-typed-cumulative-emit

Conversation

@zzylol

@zzylol zzylol commented May 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Pre-task the control plane had two emit paths into the backend's /api/v1/streaming-config:

  1. Typed cumulative path (post PR fix(emit): cumulative streaming-config across (metric, role) pairs (B2 follow-up) #287) — handle_plan runs planner → BackendStageConfig → cache insert by (metric, role) → cumulative POST. Correct under the data plane's atomic handle.swap(new_config) swap.
  2. Legacy single-aggregation pathReplanner::replan_metric_rolegenerate_streaming_config_yaml → single-aggregation YAML POST. Under the same swap, this WIPED the cumulative state every time plan-expiry or accuracy-violation fired it. The smoke harness never triggered the latent landmine because replan_metric_role ran only on SLA-violation / plan-expiry, neither of which fires in the 90s soak.

This PR is Option B of the task: collapse to ONE emit path and retire the legacy single-aggregation YAML.

Changes

  • New helper emit::backend_push::post_typed_backend_for_role — single entrypoint for every typed cumulative POST. Updates the shared (metric, role) → BackendStageConfig cache, builds the cumulative streaming-config + per-metric merged storage-routing, fire-and-forget POSTs both. Used by both handle_plan and Replanner so both writers accumulate against the same state.
  • handle_plan Backend arm — ~120 lines of inlined cumulative-emit / cache plumbing replaced with a single call to the helper.
  • Replanner::replan_metric_role — drops the legacy generate_streaming_config_yaml + push_or_log call, runs the same planner → stage-split → Backend extraction as handle_plan (new Replanner::build_backend_stage_config private helper), then POSTs through the typed helper. Now correctly cumulative on plan-expiry / SLA-violation triggers.
  • Legacy YAML emitter retiredgenerate_streaming_config_yaml and the entire emit::asapquery_backend module (287 lines) deleted. Comments in emit::stage_config and emit::mod updated to document the retirement.
  • Startup replan tickReplanner::replan_all() runs immediately after the workload-registry pre-pop loop and BEFORE the HTTP server binds, so the backend's cumulative streaming-config + storage-routing are populated from t=0 (the smoke harness never POSTs /api/v1/plan, so without this the data plane's static backend-streaming.yaml was the only source of truth).
  • OpAMP on-connect replan tick — defensive idempotent re-fire of replan_all() on every agent connect; guards against start-order races where the backend came up after the startup tick.
  • ExactAgg(Sum) fallback — the typed bind_workload_typed binder declines for raw-passthrough sum/rate/count workloads. Pre-Option B these metrics relied on the static backend-streaming.yaml having a DDSketch entry that the broken legacy emit happened not to overwrite. Post-Option B the cumulative POST has authoritative final say, so we extend BackendAggregation with agg_type_override: Option<String> and synthesize a BackendStageConfig carrying agg_type_override: Some(\"Sum\") when the typed binder declines for a Sum/Count role. build_backend_aggregation_json honours the override by emitting aggregationType: \"Sum\" and parameters: {}. The cumulative streaming-config now carries an ExactAgg(Sum) row for http_requests_total alongside the 5 sketched metrics.

Smoke test (90s soak) — before / after

Sub-check Pre-PR Post-PR
USE_TYPED_STAGE_SPLIT.*posting typed backend JSON log count 0 7
/api/v1/streaming-config aggregation count 2 (static yaml only — would be wiped on first replan) 6 (5 sketched + Sum for http_requests_total)
quantile_over_time(0.99, http_requests_total_latency_ms[5m]) DDSketch result DDSketch result (no regression)
Aggregation count after 10s late-replan check static yaml could be wiped 6 (cumulative state persists — the Option A landmine that this PR avoided by retiring the legacy path)
Backend's db/schemas for http_requests_total absent 4 ExactAgg(Sum) sids (one per zone) registered

Known limitation (out of scope)

sum by (zone) (http_requests_total) still returns No result for query because the data plane's PromQL evaluator does not yet dispatch sum by (...) instant queries to the ExactAgg sids it has registered. The streaming-config now correctly surfaces the metric to the data plane (verified: /api/v1/db/schemas returns 4 ExactAgg(Sum) sids for http_requests_total); closing the query-side gap is a data_plane/ workstream this PR explicitly honours the DO NOT touch data_plane/ constraint on. The control-plane side of the Option B unification is complete.

Test plan

  • cargo build -p control_plane clean
  • cargo test -p control_plane --lib — 738 pass (baseline maintained; 2 tests removed alongside the retired emitter)
  • cargo test -p control_plane --bins — 28 pass
  • rg \"generate_streaming_config_yaml\\(\" control_plane/src/ returns 0 hits (no callers remain)
  • Smoke test sub-checks 1 (USE_TYPED_STAGE_SPLIT log), 2 (cumulative aggregations include Sum), 4 (quantile no regression), 5 (10s late-replan idempotent) all pass.
  • Smoke test sub-check 3 (sum by (zone) returns per-zone results) still fails — data plane Sum-capability gap noted above.

🤖 Generated with Claude Code

…t; retire generate_streaming_config_yaml (Option B)

Pre-task the control plane had two emit paths into the backend:

  1. **Typed cumulative path** (post PR #287) — `handle_plan` runs
     planner → `BackendStageConfig` → cache insert by `(metric, role)`
     → build cumulative `BackendStageConfig` → POST to
     `/api/v1/streaming-config`. Correct under the data plane's
     atomic `handle.swap(new_config)` swap semantics
     (`data_plane/src/drivers/query/servers/http.rs:4758`) — every
     (metric, role) pair's aggregations survive each POST.

  2. **Legacy single-aggregation path** — `Replanner::replan_metric_role`
     called `generate_streaming_config_yaml` and POSTed a
     single-aggregation YAML body. Under the same swap, this WIPED
     the cumulative state every time plan-expiry or
     accuracy-violation fired it — a 5-minute landmine that the
     smoke harness never triggered (so the bug stayed latent).

Option B unifies both call sites through a new
`emit::backend_push::post_typed_backend_for_role` helper:

  * `main::handle_plan`'s inlined emit block (~120 lines) is
    replaced with a single helper call.
  * `Replanner::replan_metric_role` no longer calls the legacy YAML
    emitter — instead it runs the same planner/stage-split flow as
    `handle_plan` (new `build_backend_stage_config` helper) and POSTs
    through the unified helper.
  * The per-`(metric, role)` `BackendStageConfig` cache is now
    shared between `AppState` and `Replanner` via
    `with_backend_routing_cache`, so both writers accumulate
    against the same state.
  * `generate_streaming_config_yaml` and the entire
    `emit::asapquery_backend` module (287 lines) are deleted.

To prime the cumulative state before the first query lands, the
controller now:

  * Runs `Replanner::replan_all()` at startup AFTER the workload-
    registry pre-pop loop and BEFORE the HTTP server binds — every
    (metric, role) pair the registry registered is planned and POSTed
    immediately so the smoke harness (which never POSTs
    `/api/v1/plan`) sees a populated streaming-config from t=0.
  * Re-fires `replan_all()` on every OpAMP first-connect as a
    defensive idempotent tick — guards against start-order races
    where the backend came up after the startup tick.

**ExactAgg fallback for Sum-shaped workloads**: the typed
`bind_workload_typed` binder declines for raw passthrough
(`sum`/`rate`/`count`) — pre-Option B these metrics relied on the
static `backend-streaming.yaml` having a DDSketch entry that the
broken legacy emit happened not to overwrite. Post-Option B the
cumulative POST has authoritative final say, so we add an ExactAgg
fallback in `build_backend_stage_config`: when the typed binder
declines AND the role is Sum/Count, synthesize a
`BackendStageConfig` with a new `agg_type_override: Some("Sum")`
field, which `build_backend_aggregation_json` honours by emitting
`aggregationType: "Sum"` and an empty parameters object. The
streaming-config now carries an ExactAgg(Sum) row for
`http_requests_total` alongside the 5 sketched metrics — the data
plane registers 4 ExactAgg(Sum) sids (one per zone) and the
streaming-config swap is non-destructive.

Smoke verification (90s soak):
  * `[USE_TYPED_STAGE_SPLIT] posting typed backend JSON` log: 7
    (was 0 in PR #287's smoke run).
  * `/api/v1/streaming-config` aggregations: 6 (was 5 — adds Sum
    for `http_requests_total`; was 2 from the static yaml before
    the controller's first POST).
  * `quantile_over_time(...)` returns DDSketch result (no
    regression).
  * Cumulative state persists across the 10s late-replan check
    (the landmine Option A would have left).

Out-of-scope (separate ticket — Sum-capability for plain
counters): the data plane's PromQL evaluator returns
`No result for query` for `sum by (zone) (http_requests_total)`
because it does not yet dispatch `sum by (...)` instant queries to
the ExactAgg sids it has registered. The streaming-config now
correctly surfaces the metric to the data plane; closing the
query-side gap is a data_plane workstream this PR explicitly
honours the "DO NOT touch data_plane/" constraint on.

Tests: 738 lib + 28 bin tests pass. The `streaming_config_cumulative
_push_covers_all_planned_metrics` API test continues to pass against
the new helper. New `emit::backend_push::tests` cover the cumulative
cache contract (no-client logging, distinct-role accumulation,
same-pair idempotency).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 4d3a845 into main May 18, 2026
zzylol added a commit that referenced this pull request May 18, 2026
…es Option-B downstream) (#291)

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 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 added a commit that referenced this pull request May 18, 2026
… (404/connect-refused) (#293)

In the multinode harness (asap arm from ASAPCollector PR #394), the
controller and backend start concurrently. The backend logs
`HTTP server listening on port 9091` before its
`/api/v1/streaming-config` route is actually registered, and the
controller's PR #290 startup `Replanner::replan_all()` tick fires
before the backend's HTTP server has finished accepting connections.
Observed symptom: all seven startup `POST /api/v1/streaming-config`
calls returned 404; convergence relied entirely on the
`OpampServer::on_connect` re-fire when the first agent connected.

This patch adds retry-with-backoff for transient failures in
`emit::backend_push::post_typed_backend_for_role` (the single
helper PR #290 unified both call sites through). Transient: 404,
5xx, connection refused / DNS / connect timeout, reqwest
`IsTimeout` / `IsConnect`. Permanent (no retry): 4xx other than 404.

Policy: 5 attempts, exponential 100ms -> 300ms -> 900ms -> 2.7s
(capped) with full jitter, total span ~5-8s. Lock-discipline
unchanged: the `backend_routing_cache` lock is dropped before any
HTTP call, so retries never stall sibling replan cycles.
`BackendClient`'s existing public methods are untouched (new typed
variants `_typed` returning `BackendPostError` sit alongside);
`OpampServer::on_connect` remains as the secondary convergence
mechanism.

Unit tests: 4 new tests in `emit::backend_push::tests` cover
recovery-after-404, no-retry-on-permanent, exhaustion-reports-
attempts, no-retry-on-first-success; 4 new tests in
`backend_client::tests` cover the typed classifier (404/500
transient, 400 permanent, connection-refused transient).
All 748 `cargo test -p control_plane --lib` pass.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol deleted the feat/unify-replanner-typed-cumulative-emit 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