Skip to content

Publish storage routing from the physical plan - #713

Merged
zzylol merged 2 commits into
mainfrom
refactor/703-consolidate-on-asapplanner
Sep 13, 2026
Merged

zzylol merged 2 commits into
mainfrom
refactor/703-consolidate-on-asapplanner

Conversation

@zzylol

@zzylol zzylol commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Prerequisite for #703 item 18 (consolidating onto the ASAPPlanner path). Publishing the physical plan without routing is a live gap, independent of the consolidation.

Problem

compile_and_publish_physical_plan passes None for storage routing:

.post_catalog_plan_typed(&publication, None, &adaptation_evidence)

So a deployment planned through POST /api/v1/physical-plan/compile-and-publish installs no routing table, the backend falls through to its default engine, and archive-shape queries miss — histogram_quantile, delta, deriv, absent, rate_post_hoc, plus topk/count when no matching sketch was planned.

Only the legacy POST /api/v1/plan path emitted routing, through emit_backend_storage_routing in emit/backend_push.rs.

Fix

build_routing_entry never needed a BackendStageConfig — it immediately projected one down to the per-metric sketch algorithms and classified from those. It now takes &[SketchAlgorithm] directly, and storage_routing_document is the shared entry point:

  • the two BackendStageConfig emitters project onto it (legacy behaviour unchanged)
  • PhysicalCompiler accumulates algorithms per metric while compiling aggregations and builds the document in compile(), exposed as PhysicalPlan::storage_routing

Both publication paths now run one classifier over the same planning decisions, so routing cannot drift from what was actually materialized. It also means the classifier survives the later removal of BackendStageConfig.

Testing

  • New compiled_plan_routes_every_materialized_metric asserts the compiled plan routes every materialized metric and keeps a warm target on each entry.
  • cargo +1.98.0 test -p control_plane --lib — 738 passed, 0 failed.
  • cargo +1.98.0 check -p control_plane --all-targets clean.

Notes for #703 item 18

Two claims in the issue no longer hold on main, found while scoping this:

  • collector_ids is already optional. PhysicalDeploymentTarget::BackendLocalRemoteWrite requires it empty, DistributedCollectors requires it non-empty (main.rs:836-840).
  • CDM monitors are dead on both paths, not missing from the physical one. MonitorIntent threads through post_typed_backend_for_rolepush_documents_coupledpush_cumulative_entries and is dropped there as _monitors (emit/backend_push.rs:399). It should be deleted with the legacy path, not ported.

After this, storage routing is the last backend-facing behaviour the legacy path had and the physical path did not.

🤖 Generated with Claude Code

The physical-plan publication path passed `None` for storage routing, so
a deployment planned through `compile-and-publish` installed no routing
table. The backend then falls through to its default engine and
archive-shape queries (`histogram_quantile`, `delta`, `deriv`, `absent`,
`rate_post_hoc`, and `topk`/`count` without a matching sketch) miss.
Only the legacy `POST /api/v1/plan` path emitted routing, via
`emit_backend_storage_routing`.

Derive routing from the compiled plan instead. `build_routing_entry`
only ever needed the per-metric sketch algorithms, so it now takes those
directly and `storage_routing_document` is the shared entry point. The
`BackendStageConfig` emitters project onto it and the physical compiler
accumulates algorithms while compiling aggregations, so both publication
paths run one classifier over the same decisions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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