Skip to content

feat(controller): additive QuerySpec types from design.md (QueryShape/DataShape/AccuracyTarget/QueryLanguage) - #273

Merged
zzylol merged 1 commit into
mainfrom
feat/controller-align-queryspec-to-design-additive
May 6, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/controller-align-queryspec-to-design-additive

Conversation

@zzylol

@zzylol zzylol commented May 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Introduces controller/src/types_v2.rs with the typed schema fragments from controller/docs/design.md §6 core::workload: QueryLanguage, AccuracyTarget, QueryShape, DataShape, QueryId, BindingName, QueryExprPlaceholder, WorkloadPlan.
  • Folds the new fields (id, language, accuracy, dollars, deployment_model, shape, data) into analyzer::QuerySpec as #[serde(default)] additions so existing JSON callers (the POST /api/v1/plan HTTP endpoint, workloads.yaml pre-population, all in-tree test fixtures) keep working byte-for-byte without supplying them.
  • Analyzer::analyze enforces the L1 cross-product rejections from the design.md §6 table (Streaming × Batch, Streaming × Mutable) and resolves typed accuracy over legacy accuracy_sla: f64 with the typed form taking precedence; legacy callers translate via AccuracyTarget::from_legacy_accuracy_sla.
  • replan.rs, planner/, opamp/ are intentionally untouched — the new fields are present but not yet load-bearing in cost / binding decisions. That's a separate downstream PR once the L4 rule engine and stage allocator can pivot on them.
  • Adds an "Implementation status" note next to the QuerySpec block in controller/docs/design.md documenting where the implementation now sits relative to the design intent.

Test plan

  • cargo build --release -p controller clean (133 warnings — same as origin/main, all pre-existing).
  • cargo clippy --release -p controller --bin controller reports identical 162 warnings before and after this change (no new clippy hits from types_v2.rs, analyzer.rs, or main.rs deltas).
  • cargo test --release -p controller blocked by 9 pre-existing test-fixture compile errors in controller/src/config/agent.rs, config/asapquery_backend.rs, config/precompute.rs, main.rs test mod (missing data_sink field on AgentCollectorConfig literals — AgentCollectorConfig gained the field in feat(e2e): all-five-sketch runtime path + harness — controller, processor, fake-exporter, P1–P9 #204 but the test fixtures were never updated). These reproduce on origin/main with cargo test --no-run; out of scope per the brief's "Touch ONLY" constraint. New analyzer tests are wired and should pass once that fixture gap is closed in a separate PR.
  • Existing JSON API smoke verified by the new json_back_compat_omitting_new_fields test — a QuerySpec JSON without any of the new fields parses cleanly via serde_json::from_str and analyzes to the same QueryWorkload as today.

New types introduced

  • QueryLanguage (PromQL / Sql / DataFusion / ElasticDsl)
  • AccuracyTarget (Exact / Epsilon(f64) / EpsilonDelta { eps, delta })
  • QueryShape (OneShot / Streaming / Periodic { every: Duration })
  • DataShape (Batch / AppendOnlyStream / Mutable / Mixed)
  • QueryId(String), BindingName(String)#[serde(transparent)] strings
  • WorkloadPlan { bindings, roots } + QueryExprPlaceholder — container only, CSE pass deferred until L3 algebra grows LetBinding / Ref

QuerySpec field additions

Field Type Default
id Option<QueryId> None
language Option<QueryLanguage> None
accuracy Option<AccuracyTarget> None (legacy accuracy_sla translated)
dollars Option<f64> None
deployment_model Option<String> None
shape QueryShape OneShot
data DataShape AppendOnlyStream

Analyzer behavior changes

  • Accuracy precedence: typed spec.accuracy: Some(AccuracyTarget) wins over legacy accuracy_sla: f64. Translation is Exact ↔ 1.0, Epsilon(eps) ↔ 1.0 - eps, EpsilonDelta { eps, .. } ↔ 1.0 - eps. The resolved value flows into QueryWorkload.accuracy_sla so existing planner / cost-model code sees the right number either way.
  • L1 cross-product rejection: (QueryShape::Streaming, DataShape::Batch) and (QueryShape::Streaming, DataShape::Mutable) return a clear anyhow::Error with a pointer back to controller/docs/design.md §6. Everything else is accepted.

Test additions

7 new tests in analyzer::tests:

  • typed_accuracy_overrides_legacy_accuracy_sla
  • typed_accuracy_exact_clamps_to_one
  • l1_rejects_streaming_over_batch
  • l1_rejects_streaming_over_mutable
  • l1_accepts_streaming_over_append_only_stream
  • json_back_compat_omitting_new_fields
  • json_forward_compat_supplying_new_fields

7 new tests in types_v2::tests:

  • query_language_serde_roundtrip
  • accuracy_target_serde_roundtrip
  • accuracy_target_from_legacy
  • query_shape_serde_roundtrip_and_default
  • data_shape_serde_roundtrip_and_default
  • query_id_transparent_string_serde
  • workload_plan_default_is_empty

Open question

AccuracyTarget::Exact clamps the resolved accuracy_sla to 1.0 so legacy planner code keeps a sensible value, but exact paths in the cost model don't have a real pre-existing notion of "exact required" outside the WorkloadCharacteristics::exact_required flag (set today only by stateful PromQL parses like sum_over_time). The downstream PR that switches the planner to consume AccuracyTarget directly should also decide whether Exact should imply exact_required = true so sketch-binding rules are skipped. That's a behavior-change call, not safely additive — flagging here.

Doc-tightening

The mirrored controller/docs/design.md §6 says "pub id: QueryId" (non-optional). The current implementation makes id: Option<QueryId> for back-compat — existing JSON callers don't supply one. When the design lands as authoritative, either the controller derives a deterministic id server-side from (metric_name, accuracy_sla) (no API churn) or the design relaxes to Option<QueryId>. Worth resolving before the typed schema becomes load-bearing in the planner.

🤖 Generated with Claude Code

…/DataShape/AccuracyTarget/QueryLanguage)

Introduces controller/src/types_v2.rs with the typed schema fragments
from controller/docs/design.md §6 core::workload — QueryLanguage,
AccuracyTarget, QueryShape, DataShape, QueryId, BindingName,
QueryExprPlaceholder, WorkloadPlan — and folds the per-spec fields
(id, language, accuracy, dollars, deployment_model, shape, data) into
analyzer::QuerySpec as #[serde(default)] additions so the existing
JSON API surface (POST /api/v1/plan, workloads.yaml pre-population,
in-tree test fixtures) keeps working byte-for-byte.

Analyzer::analyze gains the L1 cross-product rejections from the
design.md §6 table (Streaming x Batch and Streaming x Mutable both
return a clear error with a doc pointer) and resolves typed `accuracy`
over legacy `accuracy_sla` with the typed form taking precedence.
The fields are not yet load-bearing in replan.rs / planner/ — that's
a separate downstream PR; the new fields land here so the planner has
a target to dock against without another schema rev.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit be44752 into main May 6, 2026
zzylol added a commit that referenced this pull request May 6, 2026
…pr + Schema flow) (#275)

Per design.md §6 "core::intent_algebra — Layer 3", introduce the L3
intent IR the planner pivots on, in a new `controller/src/intent_algebra/`
module. Phase A (#273) shipped the typed QuerySpec types; this PR ships
the algebra + schema flow they describe a workload over.

What lands:

- `AggIntent` — vocabulary of "what to compute, not how": Count, Sum,
  Min, Max, Avg, Quantile{q, accuracy}, TopK{k, accuracy},
  Cardinality{accuracy}, Frequency{accuracy}, Rate{window},
  Increase{window}. No sketch types here — sketch binding is L4.
- `QueryExpr` — the L3 algebra DAG, single-rooted per query: Scan,
  Window, Aggregate, LetBinding, Ref. Variant subset chosen for the DC
  + PromQL deployment scope per the orchestrator spec; Filter/Project/
  Partition/Distinct/Merge/Join/SetOp/Sort/Limit/Subquery/WindowFunc/
  BinaryOp are deferred so each lands with a planner consumer rather
  than as dead code (additive growth).
- `Schema` — typed schema flowing on every L3 edge with `unique_keys`
  populated per design.md §6 schema-flow table. `unique_keys` is the
  load-bearing CSE-legality field (design.md §6 line ~1284 + the
  batched-queries example); `cse_substitution_legal_only_with_unique_keys`
  pins this invariant as a unit test.
- `lower_parsed_query(parsed, accuracy) → QueryExpr` — single-query
  lowering from the existing `query_parser::ParsedQuery`. Workload-
  level CSE that produces fan-in (multi-root with LetBinding/Ref) is
  the follow-up's job.

Per-variant schema propagation rules match the design.md table: Scan
emits the source schema with unique_keys from its catalog; Window
propagates row identity (carries unique_keys verbatim, requires
time_index on input); Aggregate{by, ..} emits unique_keys = [by] and
strips the time axis; LetBinding/Ref propagate the bound expr's schema.

Wire-up state. Nothing in `analyzer::Analyzer` or `planner/` consumes
these types yet — that's the follow-up Phase C PR. Phase B exposes the
IR so that wiring becomes a focused change rather than co-emission of
new types + new consumers.

Tests added (in module): agg_intent_serde_roundtrip,
output_column_names_are_intent_keyed, quantile_output_is_float64,
sum_preserves_input_dtype, schema_serde_roundtrip, schema_new_*,
schema_with_time_index_*, add_unique_key_dedupes,
query_expr_simple_aggregate, query_expr_let_binding_ref,
query_expr_unresolved_ref_errors, query_expr_window_requires_time_index,
query_expr_aggregate_invalid_by_column, query_expr_serde_roundtrip,
cse_substitution_legal_only_with_unique_keys, lower_promql_basic,
lower_promql_with_group_by, lower_promql_cardinality,
lower_empty_metric_errors.

design.md §6 grows an "Implementation status" subsection naming the
Phase B subset and the deferred phases C/D/E/F.

Note on cargo test status. The controller's `cargo test` target was
already broken at origin/main (post-#273): nine `data_sink` field-init
errors in `src/config/agent.rs`, `src/config/asapquery_backend.rs`,
`src/config/precompute.rs`, and `src/main.rs` test code. This is
pre-existing breakage from the `data_sink` field added to
`AgentCollectorConfig` in #204; intent_algebra introduces no new test
errors. `cargo build` and `cargo clippy --bins` are both clean for
intent_algebra.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 6, 2026
…nfig constructors (#276)

PR #204 added the `data_sink: AgentDataSink` field to
`AgentCollectorConfig` (production sites updated) but missed nine
test-fixture / test-code constructors, leaving
`cargo test --release -p controller` blocked at compile-time. This
broke the baseline for downstream PRs (#273, #274, #275) which could
not run their newly added unit tests.

Mechanical fill-in only, no semantic change to existing tests:
- 7 sites use `AgentDataSink::default()` (Otlp-to-backend) — the
  canonical default that PR #204 introduced for new pipelines.
- 2 sites (`config::agent::tests::ddsketch_cfg` and
  `main::api_tests::generated_agent_yaml_contains_opamp_extension`)
  pin `AgentDataSink::PrometheusScrape { endpoint: "0.0.0.0:8889" }`
  because their pre-existing assertions check for the legacy
  `prometheus` exporter on :8889. Pinning the sink keeps the test
  semantics intact rather than rewriting the asserts.

After the fix:
- `cargo build --release -p controller` clean.
- `cargo test --release -p controller --no-run` clean (was the
  blocker).
- `cargo test --release -p controller` runs 395 tests; 389 pass,
  6 pre-existing failures unrelated to `data_sink`:
    * 2 in `analyzer::tests` — float-precision asserts.
    * 4 in `opamp::tests` / `api_tests` — protobuf framing
      ("invalid tag value: 0") on `ServerToAgent` decode.
  These are tracked separately and out of scope for this PR.
- Newly-shipped tests now run end-to-end:
  `query_language` (8), `language_logical_plan` (6),
  `types_v2` (7), `algebra` (intent_algebra family, 6+).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 7, 2026
…314)

The controller's L1→L5 pipeline (PRs #273-279) plans sketch placement,
and Phase B (PR #297) added `emit_backend_config_json` for the backend's
StreamingConfig. The per-metric `BackendStorageRouting` table that the
backend's HTTP query handler consults on every PromQL query, however,
was still hand-authored YAML at `deploy/configs/backend-storage-routing.yaml`
— two sources of truth, drift between the controller's stage-split
decisions and the routing config, and manual edits required when the
workload changes.

Phase α makes the controller THE planner: it emits a
`BackendStorageRouting` JSON document as part of every plan emit, the
backend hot-loads it on push, and routing flows via OpAMP push instead
of YAML edits.

Concretely:

* `config::stage_config::emit_backend_storage_routing(metric_plans)` —
  for each `(metric, &BackendStageConfig)` pair the controller has
  planned this cycle, emit a `metrics:` row with the per-shape engine
  routing list. Classification rules are sourced from the L4
  `sketch_algebra` outputs landing at the backend (DDSketch / KLL →
  warm tier for `quantile`; HLL → warm tier for `count`; Count-Sketch
  → warm tier for `topk`; CMS → warm tier for `count` / `point_count`).
  Archive-eligible shapes — `histogram_quantile`, `delta`, `deriv`,
  `absent`, `rate_post_hoc`, plus `topk` / `count` when no sketch
  claims them — are emitted on a `thanos_archive` target with an
  explicit `applies_to_query_shape` filter. Warm-tier slot stays the
  default (no filter) so unanticipated shapes route to warm rather
  than failing through to the archive's first-target fallback.

* `BackendClient::post_storage_routing_json` — sibling of
  `post_streaming_config_json`. Rewrites the configured streaming-config
  endpoint URL's path component from `/api/v1/streaming-config` to
  `/api/v1/storage_routing` so operators only configure one
  `CONTROLLER_BACKEND_ENDPOINT` and both pushes land at the same backend
  host.

* `main::handle_plan` — when `USE_TYPED_STAGE_SPLIT=1` and the typed L5
  emitter produced a `BackendStageConfig`, also call
  `emit_backend_storage_routing` and POST it via the shared
  `BackendClient`. Same fire-and-forget contract as the existing
  `streaming-config` push: errors logged at WARN, the next replan cycle
  retries.

Tests: 11 new tests (7 emitter unit / snapshot tests, 2 URL-derivation
tests, 2 mock-backend integration tests). Snapshot test in
`storage_routing_three_metric_snapshot_stable` pins the exact JSON
shape for a DDSketch + HLL + Count-Sketch plan so accidental schema
drift surfaces immediately. Pre-existing 10 controller failures
unchanged (493 pass, was 482+).

Phase α is gated behind `USE_TYPED_STAGE_SPLIT=1` (the existing typed
L5 path). Operators can still hand-author
`deploy/configs/backend-storage-routing.yaml` for dev / standalone
deployments — the backend falls back to the static YAML when no JSON
has been pushed yet (Part B, separate PR).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol deleted the feat/controller-align-queryspec-to-design-additive branch May 9, 2026 18:00
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