Skip to content

fix(emit): cumulative streaming-config across (metric, role) pairs (B2 follow-up) - #287

Merged
zzylol merged 2 commits into
mainfrom
b2-cumulative-streaming-config-emit
May 18, 2026
Merged

zzylol merged 2 commits into
mainfrom
b2-cumulative-streaming-config-emit

Conversation

@zzylol

@zzylol zzylol commented May 18, 2026

Copy link
Copy Markdown
Contributor

Summary

PR #283 made WorkloadStore and PlanStore (metric, role)-keyed so a single metric can carry multiple aggregation roles (e.g. http_requests_total post-B2 has both DDSketch-Quantile from quantile_over_time(...) AND ExactAgg-Sum from sum by (zone) (http_requests_total)). But the streaming-config emit path in control_plane/src/main.rs was still metric-keyed and emitted the current iteration's single-role BackendStageConfig alone.

The data plane's POST /api/v1/streaming-config handler is an atomic full handle.swap(new_config) (see data_plane/src/drivers/query/servers/http.rs:4758), so the second per-role POST destroys the first role's aggregations on the backend → sum by (zone) (http_requests_total) returns ExactAgg(Sum) capability not satisfied.

Changes (all in control_plane/src/main.rs)

  • Rekey backend_routing_cache from HashMap<String, BackendStageConfig> to HashMap<(String, AggRole), BackendStageConfig>. Imports AggRole from control_plane::workload.
  • Insert with role: uses the role derived just above in handle_plan (already in scope via derive_agg_role on a synthesized WorkloadEntry from the request).
  • Cumulative streaming-config emit: builds one BackendStageConfig by concatenating aggregations + readouts across every cache entry, then passes that single cumulative config to emit_backend_streaming_config_json. The data plane's swap installs every role's aggregations atomically.
  • Per-metric merge for storage-routing: groups cache entries by metric name and merges each metric's BackendStageConfigs (concat aggregations + readouts) before passing to emit_backend_storage_routing. The routing classifier reads cfg.aggregations to derive shape routing, so a metric carrying both DDSketch + Sum now routes both shape families correctly.
  • Doc update on backend_routing_cache reflecting the new (metric, role) keying and cumulative-across-all-roles emit semantic.
  • Deterministic ordering: cache iteration sorts by metric then role so emitted JSON is reproducible across runs (avoids flaky regression assertions on captured bodies).

Constraints honored: no data_plane/ changes, no signature changes to emit_backend_streaming_config_json or emit_backend_storage_routing (merge happens at the call site), fire-and-forget POST contract preserved.

Regression test

api_tests::streaming_config_cumulative_push_covers_all_planned_metrics — replays the demo's 5-metric plan-POST sequence against a mock backend, captures every streaming-config body, asserts the LAST body (the one the data plane's atomic swap installs) carries aggregations for ALL 5 metrics. Pre-fix this fails with "missing aggregations for metric X" because only the most recent plan-POST survives.

Sibling to the existing storage_routing_cumulative_push_covers_all_5_sketched_metrics test which covers the storage-routing analogue.

Test results

  • cargo build -p control_plane — clean.
  • cargo test -p control_plane --lib — 740 passed, 0 failed.
  • cargo test -p control_plane --bin control_plane — 28 passed (including the new regression test and the existing storage_routing cumulative test).

Smoke test — partial result, separate root cause

The MVP smoke test does NOT exercise this fix because the smoke harness never POSTs to /api/v1/plan — it relies on the controller's startup pre-pop loop + agent-connect path, neither of which calls the typed Backend-stage emit. As a result:

  • The mock-backend GET on /api/v1/streaming-config returns the STATIC YAML the backend loaded from backend-streaming.yaml at boot (two DDSketch entries for http_requests_total and http_requests_total_latency_ms), not anything the controller posted.
  • quantile_over_time(0.99, http_requests_total_latency_ms[5m]) PASSES (4 zones with values).
  • sum by (zone) (http_requests_total) returns "No result for query" because the backend's http_requests_total aggregation is statically DDSketch, NOT ExactAgg(Sum) — the controller never pushed an ExactAgg(Sum) entry over it.

This is a separate gap (controller pre-pop loop + agent-connect path don't post cumulative streaming-config). My PR fixes the handle_plan cumulative emit path the task describes — it's correct + tested in isolation, but the smoke harness exercises a different code path that needs its own follow-up. Not regressing anything that previously worked.

Test plan

  • cargo build clean
  • cargo test -p control_plane --lib + --bin (768 tests passed)
  • New regression test covers cumulative streaming-config across plan POSTs
  • Smoke test sum by (zone) (http_requests_total) — blocked by separate gap (controller's pre-pop + agent-connect paths don't trigger cumulative streaming-config push to backend; this PR only fixes the handle_plan HTTP POST path)

🤖 Generated with Claude Code

zzylol and others added 2 commits May 18, 2026 05:41
…2 follow-up)

PR #283 made `WorkloadStore` and `PlanStore` (metric, role)-keyed so a
single metric can carry multiple aggregation roles (e.g.
`http_requests_total` post-B2 has both DDSketch-Quantile from
`quantile_over_time(...)` AND ExactAgg-Sum from
`sum by (zone) (http_requests_total)`). But the streaming-config emit
path in `control_plane/src/main.rs` stayed metric-keyed and emitted the
current iteration's single-role `BackendStageConfig` alone. The data
plane's `POST /api/v1/streaming-config` handler is an atomic full
`handle.swap(new_config)`, so the second per-role POST destroyed the
first role's aggregations on the backend and
`sum by (zone) (http_requests_total)` failed with
`ExactAgg(Sum) capability not satisfied`.

This commit:
* Rekeys `backend_routing_cache` from `HashMap<String, _>` to
  `HashMap<(String, AggRole), _>`.
* Inserts with the role derived in `handle_plan`'s existing
  `derive_agg_role` block (already in scope).
* Builds a single cumulative `BackendStageConfig` by concatenating
  every cache entry's `aggregations` + `readouts` before emitting the
  streaming-config JSON, so the data plane's swap installs every
  role's aggregations atomically.
* Merges per-(metric, role) cache entries by metric name before
  passing to `emit_backend_storage_routing` so a metric with both
  DDSketch + Sum correctly routes both shape families to the ASAP
  tier (the routing classifier reads `cfg.aggregations` to derive
  shape routing).
* Updates the `backend_routing_cache` doc comment to reflect the
  (metric, role) keying and the cumulative-across-all-roles emit
  semantic.

Constraints honored: no `data_plane/` changes (swap semantics are
correct), no signature changes to `emit_backend_streaming_config_json`
or `emit_backend_storage_routing` (merge happens at the call site),
fire-and-forget POST contract preserved.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds `streaming_config_cumulative_push_covers_all_planned_metrics` —
the streaming-config sibling of the existing
`storage_routing_cumulative_push_covers_all_5_sketched_metrics`.

Replays the demo's per-metric plan-POST sequence (5 sketched contract
metrics) against a mock backend, captures every streaming-config body,
and asserts the LAST body (the one the data plane's atomic swap
installs) carries aggregations for ALL 5 metrics — proving the
per-(metric, role) cache + cumulative concatenation in main.rs's
typed Backend-stage block preserves every prior plan-POST's row
across subsequent swaps.

Pre-fix this would fail with "missing aggregations for metric X" —
only the last POST's metric survived the swap.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 34cb569 into main May 18, 2026
zzylol added a commit that referenced this pull request May 18, 2026
…t; retire generate_streaming_config_yaml (Option B) (#290)

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 deleted the b2-cumulative-streaming-config-emit branch July 17, 2026 20:04
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