feat(planner): asap-planner --planner saved writes a sketch-bench eval plan without re-solving by milindsrivastava1997 · Pull Request #828 · ProjectASAP/ASAPQuery

milindsrivastava1997 · 2026-10-12T01:19:55Z

Before this PR

The only way to get streaming and inference configs from an rqe-optimizer plan was --planner milp, which solves the plan itself. The #777 eval's plans were solved in sketch-bench (autosketch_vs_asap, saved in sketch-bench#203), and re-solving gives a different plan: a different solver objective, accuracy source and arrival rates. experiment_run_e2e.py also always runs the planner.

After this PR

B1 + B2: asap-planner --planner saved

asap-planner --planner saved --streaming_engine precompute --data-ingestion-interval-ms 1000 \
  --plan <sketch-bench>/rqe-optimizer/results/autosketch-vs-asap-hydra/plans/synthetic-templatesclassic-shared1.plans.json.gz \
  --workload-table <sketch-bench>/rqe-optimizer/results/autosketch-vs-asap-hydra/inputs/tables/synthetic-templatesclassic-shared1.json \
  --method asap --weights cpu [--sla-ms 1000] --output_dir DIR

The table's e2e block (sketch-bench#208) becomes a ControllerConfig: one query group per interval, with metrics hints from its labels. workload_from_config (the facts-free half of build_milp_workload) turns it into Raqes.
Each leaf query maps to its plan RQE through e2e.queries[].rqes, and the RQE to its chosen deployment, sketch, params, window and slide. The result goes through deployments_to_planner_output, the existing converter with MilpSolution replaced by (deployments, Raqe → deployment). plan_to_planner_output is now a thin wrapper around it, so --planner milp output is unchanged.
Without --sla-ms it picks the cheapest plan, and with it the sla plan at that SLA. A .gz plan file is read directly (flate2).
It refuses:
- roll-ups (deployment_grouping)
- an RQE the plan doesn't serve
- a plan deployment whose RQEs would need different sketches
- one query served by two deployments (the existing check, which also covers the same PromQL at two intervals on different deployments)
- any shape the engine can't serve (the existing pattern check)

B3: controller.prebuilt_output_dir in experiment_run_e2e.py skips controller_service.start and syncs that directory's streaming_config.yaml and inference_config.yaml to the controller output dir. It checks that both files exist at startup.

Departures from the #824 plan:

B1 passes deployments instead of building a MilpSolution, because PlanCost can't be built honestly from a saved plan.
B2 takes --sla-ms instead of --kind, because the file has one SLA plan per SLA.

Blocked: 12 classic queries (#827)

On the real classic base table (ASAP cheapest at CPU weights), 12 of the 54 queries are shapes ASAPQuery can't serve: sum by (job) (rate(...)) and topk(32, sum by (label_0) (...)) over rate and sum_over_time. So --planner saved refuses the table:

Error: cannot optimize leaf 'topk(32, sum by (label_0) (sum_over_time(data[15m])))' ...: no supported query pattern matched

With those 12 removed from a copy of the table, it writes 5 aggregations. Their windows, slides and params match the plan: dd 0.02 1s; dd 0.05 300s/60s sliding; exact-increase 60s; exact-sum 1s and 60s. The other 3 plan deployments serve the removed queries. #827 tracks engine support, and the e2e run waits on it.

Validation

cargo test -p asap_planner: all pass, including 5 new tests in saved_plan.rs. They cover:
- a ratio whose legs share one sliding DDSketch
- --sla-ms selection, plus no matching plan
- roll-up refusal
- one deployment needing two sketches (this test caught leaves that merge into one item, such as p90 and p50, not being checked)
- an RQE missing from the plan
cargo clippy --all-targets -D warnings, cargo fmt, black, flake8: clean.
The CLI ran on sketch-bench's committed plan file: the refusal above, the 42-query conversion, and --sla-ms 1000.
B3: the copy-and-sync path was run on its own with a local provider. A full e2e run waits on feat(planner): serve the #777 eval's sum-of-rate and nested topk queries #827 and B4.

Part of #824 (B1, B2, B3).

🤖 Generated with Claude Code

…l plan without re-solving Reads a plan from sketch-bench's autosketch_vs_asap plan file and the eval table's `e2e` block (PromQL queries, the plan RQEs each reads, metric labels), and writes streaming and inference YAML through the same converter as --planner milp. experiment_run_e2e.py gains controller.prebuilt_output_dir to deploy those configs instead of running the planner. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

milindsrivastava1997

Code review: 10 findings, posted inline.

milindsrivastava1997 · 2026-10-12T20:16:36Z

+                pick.deployment_grouping.is_none(),
+                "RQE {rqe:?} is served by a roll-up, which the engine can't read yet"
+            );
+            let deployment = Deployment {


Plan pick never checked against the RAQE's window/slide. The synthesized Deployment takes capability, metric, filter and grouping from the RAQE, but sketch, params, window and slide from the plan. Nothing checks pick.window_secs/slide_secs against the RAQE's lookback_ms/interval_ms. If --plan doesn't match --workload-table (wrong table, reused RQE ids like q0/q1, mis-ordered e2e.queries[].rqes), a 5m quantile_over_time leaf can bind to a 1s-window or sum-sketch pick. The planner then writes the wrong config and exits 0.

Fixing: each pick is now refused unless its window divides the RAQE's lookback (the rule sketch-bench uses for merged_shards) and its slide fits the repeat interval.

milindsrivastava1997 · 2026-10-12T20:16:36Z

-                CONTROLLER_LOCAL_OUTPUT_DIR,
-                node_offset=args.node_offset,
-            )
+            if args.controller_prebuilt_output_dir:


Prebuilt configs aren't checked against the experiment. The client sends queries from controller_client_config, not the table's e2e block, and data_ingestion_interval_ms, streaming_engine and punting are ignored here. If the planner ran with --data-ingestion-interval-ms 1000 but Prometheus scrapes at 15s, or the query groups differ, the engine gets queries or windows the config doesn't cover. The run then finishes silently with fallback or wrong results.

Deferring to B4 (#824). B4 builds the client config and scrape interval from the same table e2e block the planner reads, which closes most of this. A full cross-check in the runner isn't worth it on its own.

milindsrivastava1997 · 2026-10-12T20:16:36Z

+                    f"{args.controller_prebuilt_output_dir}"
+                )
+                shutil.copytree(
+                    args.controller_prebuilt_output_dir,


copytree(..., dirs_exist_ok=True): same-path error and stale merge. If prebuilt_output_dir points at <experiment_outputs>/<same name>/controller_output, copy2 raises SameFileError and copytree raises shutil.Error. With different paths, stale files from an earlier run with the same name (e.g. controller.log, other YAML) are kept and rsynced to the remote.

Fixing: a source equal to the destination is refused, and controller_output is cleared before the copy.

milindsrivastava1997 · 2026-10-12T20:16:36Z

+    let mut rqe_of: HashMap<(String, u64), &str> = HashMap::new();
+    for q in &e2e.queries {
+        let leaves = decompose_to_leaves(&q.promql);
+        ensure!(


avg queries hit the leaf-count error, not the AvgQuery refusal. reject_unwritable_queries only runs inside deployments_to_planner_output, after this check. avg_over_time(data[5m]) with one RQE is split into sum and count leaves, so the user sees "has 2 leaves but names 1 RQEs" instead of the intended AvgQuery error.

Fixing: reject_unwritable_queries now runs before the leaf loop.

milindsrivastava1997 · 2026-10-12T20:16:36Z

+            selector.sla_ms,
+        );
+    };
+    let chosen: HashMap<&str, &Chosen> = plan.chosen.iter().map(|c| (c.rqe.as_str(), c)).collect();


A duplicate rqe in chosen silently keeps the last entry. collect() into a HashMap drops earlier picks for the same RQE without warning, so the plan is only partly reproduced. Consider refusing duplicates.

Fixing: duplicate RQEs in chosen are refused. (The committed classic plan files have none.)

milindsrivastava1997 · 2026-10-12T20:16:36Z

+            p.method == selector.method
+                && p.weights == selector.weights
+                && p.kind == kind
+                && p.sla_ms == selector.sla_ms


Exact f64 equality on sla_ms; cheapest requires a null sla_ms. If sketch-bench writes a non-null sla_ms on cheapest plans, or a value like 999.9999 after unit conversion, --sla-ms 1000 (or no --sla-ms) gets "0 plans match".

Won't fix. sketch-bench leaves sla_ms out of cheapest plans and writes the SLA values as integers (100, 300, 1000, 3000, 10000), so a parsed --sla-ms 1000 matches exactly. If it ever drifts, the error is loud ("0 plans match"), not silent.

milindsrivastava1997 · 2026-10-12T20:16:36Z

-        let deployment = &solution.deployments[planned.deployment].deployment;
+    for (raqe, &index) in workload.raqes.iter().zip(served_by) {
+        let deployment = &deployments[index];
        if raqe.grouping_labels != deployment.grouping_labels {


This roll-up guard can't fire on the saved path. saved_plan copies grouping_labels from the RAQE into the synthesized Deployment, so roll-up detection there depends only on Chosen.deployment_grouping.is_none(). If sketch-bench changes how it records roll-ups (e.g. always emitting deployment_grouping, or emitting null), neither check catches it and configs are written at the RAQE grouping. Building the Deployment's grouping from the plan's deployment_grouping when present would let this shared check do the work.

Fixing: the Deployment's grouping now comes from deployment_grouping when it's present, and the saved-path check is gone, so the shared guard catches roll-ups and Hydra grids.

milindsrivastava1997 · 2026-10-12T20:16:36Z

+    anyhow::ensure!(
+        matches!(args.query_language, QueryLanguage::promql)
+            && args.prometheus_url.is_none()
+            && !args.enable_punting


Flag validation is duplicated. run_saved repeats run_milp's PromQL, hints-only and flag-exclusion checks, plus the MILP-only-flags ensure from main's legacy branch. A new flag now has to be added in three places. A shared helper (e.g. ensure_promql_hint_only(args, name)) plus hoisting the MILP-only ensure to planner != Milp would cover all of them.

Fixing: the shared checks now live in one helper.

milindsrivastava1997 · 2026-10-12T20:16:36Z

+    let mut position: HashMap<usize, usize> = HashMap::new();
+    let mut deployments: Vec<Deployment> = Vec::new();
+    let mut served_by = Vec::with_capacity(workload.raqes.len());
+    for (raqe, &item) in workload.raqes.iter().zip(&workload.raqe_items) {


nit (efficiency): Resolution runs once per RAQE, so a query repeated across N dashboards rebuilds N identical Deployments, each with a cost_entry JSON round-trip. Resolving once per workload.items index and expanding through raqe_items gives the same result with less work.

Won't fix: the workloads have 54 queries, so the repeated work is negligible.

milindsrivastava1997 · 2026-10-12T20:16:36Z

        # Controller configuration
        self.controller_punting = cfg.controller.punting
        self.controller_auto_discover_labels = cfg.controller.auto_discover_labels
+        self.controller_prebuilt_output_dir = cfg.controller.get("prebuilt_output_dir")


Scope: this PR bundles the Rust --planner saved feature with an unrelated asap-tools e2e-runner change (prebuilt_output_dir), and the runner change has no test. AGENTS.md: "Keep PRs small and single-purpose." Consider splitting it out.

Keeping it bundled: B1 to B3 ship together on purpose (#824), since B3 is only useful with --planner saved. Adding a test for the prebuilt_output_dir checks.

This was referenced Oct 12, 2026

Tracker: run #777 eval plans end to end from saved plans #824

Open

fix(query-engine): stub bench_ingest_sender in the Dockerfile dependency layer #832

Merged

milindsrivastava1997 force-pushed the 824-planner-saved branch from 36b62ce to f2950c1 Compare October 12, 2026 17:29

milindsrivastava1997 force-pushed the 824-planner-saved branch from f2950c1 to be66ac5 Compare October 12, 2026 17:31

milindsrivastava1997 commented Oct 12, 2026

View reviewed changes

Conversation

milindsrivastava1997 commented Oct 12, 2026

Before this PR

After this PR

Blocked: 12 classic queries (#827)

Validation

Uh oh!

milindsrivastava1997 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant