feat(planner): (metric, role)-keyed WorkloadStore + PlanStore (B2 full) - #283
Merged
Merged
Conversation
…ti-role metrics (B2 full restructure) Closes the B2 thread (Sum-as-separate-aggregation alongside sketch). PR #282's bundle deferred this with note "needs (metric, role)-keyed restructure"; the B2-planning-agent's analysis concluded the restructure is the architecturally clean solution despite its size (vs the side-table alternative). What changed: * New AggRole enum (Quantile / Sum / Count / Topk / Other) in control_plane/src/workload.rs + derive_agg_role() classifier that resolves from (a) sketch_family_override (DDSketch/KLL → Quantile, HLL → Count, CountSketch → Topk, CMS → Other) then (b) outermost PromQL token of query_string. * WorkloadStore keyed by (metric: String, role: AggRole) — append-not-collapse on duplicate metric inserts. New `get_all_for_metric` + `keys()` accessors for emit-path walks. * PlanStore keyed by (metric, role) — one CollectionPlan per pair; rollback/diff/expired now per-pair. `metrics()` dedups across roles to preserve the metrics-exposer wire shape. * agent_to_metric → agent_to_metrics: Vec<(metric, role)> in replan.rs so a single agent can serve multiple (metric, role) plans. register_agent / unregister_agent updated; push_config_to_agent picks the first pair (the 5-sketch routing-connector edge YAML carries every metric's pipeline anyway, so one push covers them all). * Replanner::replan_metric stays as a wrapper that loops over every role registered for the metric; new replan_metric_role(metric, role) for targeted single-role replans. replan_expired loops over expired (metric, role) pairs; handle_violation re-plans every pair the agent serves. * main.rs pre-pop loop now derives the role per WorkloadEntry via derive_agg_role() before set — the three http_requests_total entries in mvp-workload.yaml (sum / sum+ rate / count) now persist as TWO distinct keys (http_requests_total, Sum) + (http_requests_total, Count) instead of collapsing onto one with only the last entry's plan surviving. * handle_plan derives the role from the QuerySpec's query_string + optional sketch_type override and threads it into store.set / workload_store.set / register_agent. * HTTP endpoints (/api/v1/plan/:metric, /diff, /rollback, /config) preserved at metric granularity; responses extended with a per-role `roles: [...]` array (additive, no breaking URL change). Rollback returns BAD_REQUEST when no role has a previous plan (matches pre-B2 single-role behaviour). * Emit helpers (collect_metric_to_family, collect_metric_to_grouping_labels) walk the new (metric, role) keyed store and take the FIRST sketch-binding role per metric for the routing-connector OTTL conditions — Sum-shaped roles correctly decline bind_workload_typed and flow through the metrics/raw_passthrough pipeline. * Bootstrap typed path (emit_bootstrap_typed) walks every role's workload until one binds, instead of the prior single-role lookup. * metrics_exposer's refresh_plan_ids iterates per-pair so each (metric, role) emits its own asap_active_plan_id row under the metric label. Role folded into the plan_id hash so distinct-role plans produce distinct ids. Test plan: * cargo test -p control_plane --lib: 740 pass (was 719; +21 new tests covering AggRole classification, WorkloadStore multi-role coexistence, PlanStore role-keyed rollback/diff/expired, Replanner replan_metric_role + multi-role agent registration + violation handling, and the synthetic http_requests_total 3-entry regression test). * cargo test -p control_plane --bins: 27 pass (unchanged). * cargo test -p data_plane --lib: 712 pass (no data_plane changes — backend's ingest_precompute_for_agg_config already handles AggKind::ExactAgg(Sum)). Smoke test (NOT run by this commit): The mvp-smoke-test/run_smoke.sh end-to-end check requires docker compose + the full ASAP stack + a backend-streaming.yaml that includes a Sum-shaped aggregationType entry for http_requests_total. The static backend-streaming.yaml in deploy/configs/ today only carries DDSketch entries — that's a deployment-config follow-up, NOT a controller change. Without the static YAML update the data_plane has no Sum capability for http_requests_total at startup and the `sum by (zone)` PromQL still surfaces capability-miss until the dynamic streaming-config POST path (Phase 5 controller → backend hot-reload) plumbs through to the live backend. Ambiguous-case decisions (documented inline): * count_over_time(...) classifies to Count (cardinality semantics). count(over_time(sum_over_time(...))) shape would be Sum via the outer aggregation. * rate / increase classify to Sum (both bind to ExactAgg(Sum) on the data plane). * CountMinSketch override classifies to Other (frequency estimator without an inherent topk shape — keep Topk pure for sketch-with-heap families). Closes B2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
zzylol
added a commit
that referenced
this pull request
May 18, 2026
…2 follow-up) (#287) * fix(emit): cumulative streaming-config across (metric, role) pairs (B2 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> * test(emit): regression test for cumulative streaming-config push 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> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the B2 deferral from PR #282 — the
WorkloadStoreandPlanStoreare now keyed by(metric: String, role: AggRole)instead of justmetric: String. A single metric can carry multiple aggregation roles, each with its own plan, agent assignment, and HTTP plan endpoint.The bug this fixes:
mvp-workload.yamlregistershttp_requests_totalTHREE times (entries 2/3/4:sum by (zone),sum by (zone) (rate(...)),count(...)). Pre-B2 the store key was the metric name alone, soset(metric, …)collapsed all three onto one slot — only the LAST entry survived, andsum by (zone) (http_requests_total)PromQL failed at the data plane withExactAgg(Sum) capability not satisfied.What changed
AggRoleenum (Quantile/Sum/Count/Topk/Other) +derive_agg_role()classifier incontrol_plane/src/workload.rs. Resolution order: (1)sketch_family_override(DDSketch/KLL → Quantile, HLL → Count, CountSketch → Topk, CMS → Other) then (2) outermost PromQL token inquery_string.WorkloadStorekeyed by(String, AggRole)withget_all_for_metric+keys()accessors for emit-path walks.PlanStorekeyed by(String, AggRole)—rollback/diff/expiredall per-pair;metrics()dedups across roles (preserves metrics-exposer wire shape).Replanner:agent_to_metric: HashMap<String, String>→agent_to_metrics: HashMap<String, Vec<(String, AggRole)>>so one agent can serve multiple pairs.replan_metric(metric)keeps signature — now a wrapper that loops every role.replan_metric_role(metric, role)for targeted single-role replans.replan_expiredloops over expired pairs;handle_violationre-plans every pair the agent serves.main.rspre-pop loop derives the role per entry viaderive_agg_role()— multi-shape metrics now persist as multiple(metric, role)keys.handle_planderives the role from the request'squery_string+ optionalsketch_typeoverride./api/v1/plan/:metric,/diff,/rollback,/config) preserved at metric granularity; responses extended with a per-roleroles: [...]array (additive, no breaking URL change)./rollbackreturnsBAD_REQUESTwhen no role has a previous plan (matches pre-B2 single-role behaviour).bind_workload_typedand flow throughmetrics/raw_passthrough.metrics_exposerrefresh_plan_idsiterates per-pair; role folded into the plan_id hash so distinct-role plans produce distinct ids.Ambiguous-case decisions
count_over_time(...)→Count(cardinality semantics).rate/increase/irate→Sum(both bind toExactAgg(Sum)on the data plane).CountMinSketchoverride →Other(frequency estimator without an inherent topk shape — keepsTopkpure for sketch-with-heap families).Test plan
cargo test -p control_plane --lib: 740 pass (was 719; +21 new tests coveringAggRoleclassification, multi-role store coexistence, per-role rollback/diff/expired, multi-role replan, the http_requests_total 3-entry pre-pop loop regression, and the agent-serves-multiple-roles violation handling)cargo test -p control_plane --bins: 27 pass (unchanged)cargo test -p data_plane --lib: 712 pass (no data_plane edits — backend'singest_precompute_for_agg_configalready handlesAggKind::ExactAgg(Sum))/mydata/mvp-smoke-test/run_smoke.sh): NOT run by this PR. Reason: the live deployment's staticdeploy/configs/backend-streaming.yamlonly carries DDSketch entries forhttp_requests_total— there is no Sum-shapedaggregationTyperow for the data plane to satisfysum by (zone). Until either the static YAML is updated OR the dynamic/api/v1/streaming-configPOST path is wired up to populate Sum entries from the controller, the smoke test'ssumzonequery will still surface capability-miss at runtime. The controller change in this PR is necessary but not sufficient for the end-to-end fix; the YAML / dynamic-push follow-up is a separate deployment-config PR.Links
sum by (zone) (http_requests_total)returns{"error":"No result for query"}because the agent never sketches Sum forhttp_requests_total.Closes B2.
🤖 Generated with Claude Code