feat(planner): add asap-planner --planner legacy|milp by milindsrivastava1997 · Pull Request #808 · ProjectASAP/ASAPQuery

milindsrivastava1997 · 2026-10-07T19:08:36Z

A5′ of #790. asap-planner --planner milp plans with sketch-bench's rqe-optimizer MILP and writes streaming_config.yaml / inference_config.yaml. The default is legacy, which is unchanged.

Changes

Shared library entry point: optimizer::plan_milp(config, &MilpInputs) loads the facts and cost table, builds the objective and solves. Both asap-planner and asap-optimizer-cli call it. parse_weight moved there from the CLI.
PlannerOutput::write_to_dir: serializes both YAMLs before writing either. Both MILP paths and the PromQL, SQL and Elastic controllers' generate_to_dir use it, so the output file names live in one place.
New asap-planner flags:
- --planner legacy|milp
- --workload-facts, --atomic-costs and --saturation-dir (required with milp; the saturation curves came with feat(planner): read MILP accuracy from sketch-bench's saturation curves; heap from the plan #804, and plan_milp passes them to solve_milp as the accuracy source)
- --w-cpu and --w-mem (optional)
- Passing any of the last four without --planner milp is an error.
--planner milp accepts only PromQL with --input_config. These are errors:
- --query-language sql|elastic_*
- --query-log (it has no metrics: hints)
- --prometheus-url (labels come from the hints)
- --enable-punting, --range-duration-ms, --step-ms
windowing, sketch_parameters and aggregate_cleanup are rejected by build_milp_workload (both binaries, always): the MILP picks windows and sketch parameters itself.
reject_unwritable_queries (formerly reject_avg_queries) rejects avg queries (A3b) and per-group step_ms / range_duration_ms, which only size retention that MILP configs don't set yet (planner: MILP plan → InferenceConfig should use ReadBased cleanup instead of NoCleanup #800). It runs only when configs are written, so print-only asap-optimizer-cli runs still plan them.
--planner milp also rejects --data-ingestion-interval-ms 0, --clickhouse-url and --clickhouse-database.
allow_undeployable_families is always false in asap-planner, which always writes configs.

Test plan

New tests: unsupported_config_fields_are_rejected, range_query_overrides_block_only_the_written_configs.
cargo test -p asap_planner (123 tests), clippy with -D warnings, fmt; pre-commit hooks passed.
Ran the real binary with --planner milp on a sum / rate / quantile / topk workload. It wrote 4 aggregations: DatasketchesKLL, MultipleIncrease, MultipleSum and CountMinSketchWithHeap. I checked the rejection errors for missing facts/costs, --step-ms, and MILP flags without --planner milp. Legacy runs unchanged.
Note: the local cost exports (sketch-bench/out*/rqe_atomic_costs.json) are older than the rqe-optimizer version pinned in feat(planner): deploy DDSketch from MILP plans #806. Their dd rows lack mean_relative_value_error, and rqe-optimizer panics on that, so the smoke run used a copy with the dd rows removed. A fresh export is needed.

🤖 Generated with Claude Code

plan_milp loads the facts and costs, builds the objective and solves, so asap-planner can share it with asap-optimizer-cli. PlannerOutput gains write_to_dir. Refs #790 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

--planner milp plans with the rqe-optimizer MILP and writes its configs; legacy stays the default. milp takes --workload-facts and --atomic-costs (required) and --w-cpu/--w-mem, and supports only PromQL with --input_config. It rejects --query-log, --prometheus-url, --enable-punting, --range-duration-ms, --step-ms, and per-group step_ms/range_duration_ms, whose retention MILP configs don't model yet (#800). Refs #790 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Refs #790 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

milindsrivastava1997

Controller::generate_to_dir (asap-planner-rs/src/promql/controller.rs:163) and the SQL/Elastic equivalents still duplicate the write-both-YAMLs logic that PlannerOutput::write_to_dir now provides. Could they call write_to_dir so output file names live in one place?

milindsrivastava1997 · 2026-10-07T19:53:09Z

+        .input_config
+        .as_deref()
+        .ok_or_else(|| anyhow::anyhow!("--planner milp requires --input_config"))?;
+    let scrape_interval_ms = args.data_ingestion_interval_ms.ok_or_else(|| {


--data-ingestion-interval-ms 0 isn't rejected here (asap-optimizer-cli uses range(1..)). A 0 reaches load_workload_facts / extract_aqes, where per-series rates are derived from the scrape interval → inf/NaN or divide-by-zero inside the solver instead of a clear CLI error. Suggest ensure!(scrape_interval_ms > 0) here or range(1..) on the flag.

Also nit: the message says "required for PromQL mode" (copied from the legacy branch); every other check in run_milp names --planner milp.

Fixed in 0c4bcf5: run_milp now rejects 0 with ensure!(scrape_interval_ms > 0). I didn't put range(1..) on the flag because legacy shares it. The missing-interval message now names --planner milp.

milindsrivastava1997 · 2026-10-07T19:53:09Z

+    let scrape_interval_ms = args.data_ingestion_interval_ms.ok_or_else(|| {
+        anyhow::anyhow!("--data-ingestion-interval-ms is required for PromQL mode")
+    })?;
+    let config: ControllerConfig = serde_yaml::from_str(&std::fs::read_to_string(config_path)?)?;


This parses the config with serde_yaml directly, skipping the windowing.validate() that every legacy Controller::from_* constructor runs. E.g. windowing: {type: sliding, window_size_ms: 1000} without slide_interval_ms, or window_size_ms: 0, is rejected by the legacy planner but planned and written under --planner milp.

Addressed in 0c4bcf5 by rejecting windowing under MILP (see the build_milp_workload thread). The MILP picks its own windows, so a validated windowing would still be ignored.

milindsrivastava1997 · 2026-10-07T19:53:09Z

    facts: &WorkloadFacts,
    scrape_interval_ms: u64,
 ) -> Result<MilpWorkload, MilpError> {
+    // They only size retention, which MILP configs don't set yet.


Two things:

step_ms/range_duration_ms are rejected because MILP ignores them, but windowing, sketch_parameters and aggregate_cleanup are equally ignored and pass silently (e.g. sketch_parameters: {DatasketchesKLL: {K: 400}} gets dropped without warning). Worth rejecting all unsupported fields in one place.

Living in build_milp_workload means asap-optimizer-cli print-only runs (no --output-dir, or --allow-undeployable-families) now fail on range-query workloads, though the stated reason (retention sizing in written configs) only applies when configs are written. Compare reject_avg_queries, which is gated on output_dir.is_some().

Both fixed in 0c4bcf5:

build_milp_workload now rejects windowing, sketch_parameters and aggregate_cleanup in one place, in both binaries and whether or not configs are written. I left existing_*_config alone because legacy ignores it too.

The step_ms/range_duration_ms check moved into reject_unwritable_queries (formerly reject_avg_queries), which only runs when configs are written. Print-only asap-optimizer-cli runs plan those workloads again. Test: range_query_overrides_block_only_the_written_configs.

milindsrivastava1997 · 2026-10-07T19:53:09Z

        EngineArg::Precompute => StreamingEngine::Precompute,
    };

+    if args.planner == PlannerArg::Milp {


With --planner milp, --clickhouse-url, --clickhouse-database and --streaming_engine are silently ignored, while --prometheus-url, --enable-punting, --step-ms etc. are explicitly rejected in run_milp. Should these be rejected too for consistency?

Fixed in 0c4bcf5: --clickhouse-url and --clickhouse-database are now rejected under --planner milp. I left --streaming_engine as is: its only value is precompute, which is what MILP emits, so it isn't ignored.

…_to_dir The PromQL, SQL and Elastic controllers each repeated the two-file write; the output file names now live in one place. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

- windowing, sketch_parameters and aggregate_cleanup are rejected by build_milp_workload, since the MILP picks windows and parameters itself. - Per-group step_ms/range_duration_ms move to reject_unwritable_queries (formerly reject_avg_queries), so print-only asap-optimizer-cli runs still plan them. - asap-planner --planner milp rejects a zero scrape interval and the ClickHouse flags. Refs #790 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

milindsrivastava1997 · 2026-10-07T19:59:58Z

Re the generate_to_dir duplication: fixed in ce4483f. PlannerOutput::write_to_dir now returns ControllerError, and the PromQL, SQL and Elastic controllers call it, so the output file names live in one place.

# Conflicts: # asap-planner-rs/src/bin/optimizer_cli.rs # asap-planner-rs/src/optimizer/milp.rs

milindsrivastava1997 and others added 3 commits October 7, 2026 15:07

refactor(planner): move MILP load-and-solve into the library

45734b3

plan_milp loads the facts and costs, builds the objective and solves, so asap-planner can share it with asap-optimizer-cli. PlannerOutput gains write_to_dir. Refs #790 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs(planner): mention value_range in asap-planner --workload-facts help

ac73396

Refs #790 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

milindsrivastava1997 commented Oct 7, 2026

View reviewed changes

milindsrivastava1997 and others added 2 commits October 7, 2026 15:55

refactor(planner): write controller outputs with PlannerOutput::write…

ce4483f

…_to_dir The PromQL, SQL and Elastic controllers each repeated the two-file write; the output file names now live in one place. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Merge remote-tracking branch 'origin/main' into 790-a5p-planner-flag

dcdd2c5

# Conflicts: # asap-planner-rs/src/bin/optimizer_cli.rs # asap-planner-rs/src/optimizer/milp.rs

milindsrivastava1997 merged commit fe74d3c into main Oct 8, 2026
5 of 6 checks passed

milindsrivastava1997 deleted the 790-a5p-planner-flag branch October 8, 2026 01:37

Navigation Menu

Repository navigation

feat(planner): add asap-planner --planner legacy|milp - #808

feat(planner): add asap-planner --planner legacy|milp#808
milindsrivastava1997 merged 6 commits into
mainfrom
790-a5p-planner-flag

milindsrivastava1997 commented Oct 7, 2026 •

edited

Loading

Uh oh!

milindsrivastava1997 left a comment

Uh oh!

milindsrivastava1997 Oct 7, 2026

Uh oh!

milindsrivastava1997 Oct 7, 2026

Uh oh!

milindsrivastava1997 Oct 7, 2026

Uh oh!

milindsrivastava1997 Oct 7, 2026

Uh oh!

milindsrivastava1997 Oct 7, 2026

Uh oh!

milindsrivastava1997 Oct 7, 2026

Uh oh!

milindsrivastava1997 Oct 7, 2026

Uh oh!

milindsrivastava1997 Oct 7, 2026

Uh oh!

milindsrivastava1997 commented Oct 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

milindsrivastava1997 commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Test plan

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!

milindsrivastava1997 commented Oct 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

milindsrivastava1997 commented Oct 7, 2026 •

edited

Loading