Skip to content

feat: keep per-series arithmetic warm via Planner compile - #801

Open
zzylol wants to merge 6 commits into
refactor/current-series-planner-readoutsfrom
feat/per-series-arithmetic-warm
Open

zzylol wants to merge 6 commits into
refactor/current-series-planner-readoutsfrom
feat/per-series-arithmetic-warm

Conversation

@zzylol

@zzylol zzylol commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #799.

Why

#798 forwarded query-side computation that Planner could not compile to the exact engine. Per-series arithmetic over stored readouts, such as avg_over_time(x[5m]) (sum/count), rate(a) / rate(b) and rate(x) * 2, stopped being served from stored state. #798 also rewrote the issue 701/702 tests that asserted a warm avg_over_time, and replaced the finite-division overflow test. Planner PR #489 (integration rev e7c64ab) now compiles per-series Binary when rows carry $promql_series_identity. It also fixes the without aggregation panic (SummaryFamilySchemaMismatch).

What

  • Repin every ASAPPlanner dependency to e7c64ab20a18592c462357c9fff954892b0d4a08. Cargo.lock changes only the six source lines.
  • Selection. If a selected query-time computation fails compile_query_computation with Planner's missing-identity error (row binary requires grouped rows or rows with a series identity; Planner has no typed variant, so the message is matched), the workload is selected again over roots typed with promql_rows::with_series_identity. The typed workload is added as one more candidate forest. Deployment pricing chooses among all forests, as it does for the native forests. There is no preferred-request trial compile.
    • Every query is retyped so that states shared across queries (for example sum_over_time(x) next to avg_over_time(x)) keep one semantic definition. A shared state has the same deployed fingerprint typed or untyped (sum_over_time(data[5m]): 17087339741902330309 both ways). A root that cannot be retyped keeps its canonical states, so the typed forest is rejected if such a root shares a state fingerprint with a retyped root.
    • Rejections are recorded in the selection trace (stage: planner.series_identity_reselection): a failed typed selection, a window-preparation failure, no computation that compiles after retyping, or a shared state. Trace entries from the typed selection carry "reselection": "series_identity".
  • Adapter. native_values::physical fills $promql_series_identity from each readout series' labels. When the output carries the identity, it decodes result labels from that column. This path does not check for duplicate labelsets after __name__ is dropped, as execute_batches does. The Planner fix for duplicate labelsets covers it; this PR does not change it.
  • Tests.
    • The issue 701/702 warm avg_over_time assertions and temporal_average_overflow_falls_back_after_state_is_warm are restored from before refactor: run query-side computation as Planner physical DAGs #798. The overflow test now looks for the checked-division flag anywhere in the fragment, because Planner uses series_binary instead of VectorBinary. Its quotes now price candidates that read state for every query below the rest, instead of pricing the first feasible candidate cheapest, so the result does not depend on forest order.
    • New in control plane:
      • per-series arithmetic has a warm Planner-fragment candidate;
      • the typed workload is only a candidate forest, and its trace entries are tagged;
      • mixed workloads ([avg_over_time(data[5m]), sum by (job)(rate(data[5m]))], [rate(data[5m])*2, sum(rate(data[5m]))], [avg_over_time, sum_over_time]) keep an all-warm candidate;
      • a shared state's fingerprint is the same typed and untyped;
      • a typed forest whose unretyped root shares a state is rejected;
      • a compile failure other than the identity error does not trigger reselection;
      • without aggregations plan on the workload-cost fixture.
    • New in the adapter: per-series ratios and scalar arithmetic match series, with readouts that carry __name__, and the result drops it.
  • The developer doc (physical-compiler.md) describes the reselection.

Before this PR

avg_over_time(issue701_data[5m]) → ExactFallback (row binary requires grouped rows or rows with a series identity). The 701/702 process tests then got a forwarded response instead of a warm one. sum without (pod) (m) planning panicked with SummaryFamilySchemaMismatch.

After this PR

avg_over_time(issue701_data[5m]) → one Planner PhysicalFragment (SeriesLabels ×2 + series binary) over the sum and count readouts, answered warm. The overflow workload is warm at value 0. At 1e308 the stored plain Sum overflows, the checked division errors, and the query falls back to the exact answer 1e308, as before #798. sum without (pod) (m) and quantile without (pod) (0.5, m) plan without panicking.

For [avg_over_time(data[5m]), sum by (job)(rate(data[5m]))], the request has the canonical workload plus three forests (typed first, then two native). Enumeration gives five candidates, and one serves both queries from four states. Removing the trial compile did not change the enumerated candidate set in the probed workloads, only the order.

Validation

  • Before the repin and routing, the new and restored tests failed: 3 control-plane unit tests, and 3 of the 4 issue 701/702 process tests. The adapter tests fail without the identity binding (SeriesLabels: EOF while parsing). typed_reselection_is_only_a_candidate_forest fails on the pre-review head, where the typed workload replaced the primary request. The other review tests cover behavior that did not regress, or code added in the review.
  • cargo fmt --all -- --check, cargo clippy --workspace --all-targets --locked -- -D warnings
  • cargo test --workspace --locked --lib: 1700 passed; cargo test -p control_plane --locked --tests: 459 passed; 0 failed
  • cargo test -p data_plane --locked --test asapquery_compatibility_process_e2e -- --test-threads=1: 26 passed
  • Review: an agent that did not write this PR reviewed it independently. A separate agent applied the fixes and wrote the review tests.

🤖 Generated with Claude Code

zzylol and others added 4 commits September 30, 2026 09:08
Restores the issue 701/702 warm avg_over_time assertions and the
finite-division overflow test that #798 rewrote, and adds control-plane
acceptance tests. These fail at the current Planner pin.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Picks up per-series Binary compilation over stored readouts, the
without-aggregation state column fix, and Prometheus-compensated sums.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Per-series Planner fragments read $promql_series_identity. Fill it from
each readout series' labels and decode result labels from it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When a selected query-time computation does not compile over canonical
readouts, select the workload again over roots typed with the series
identity. Prefer it when it deploys; otherwise keep it as a candidate
forest. The per-series test now checks the snapshot path, and the
overflow test finds the checked-division flag anywhere in the fragment.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zzylol
zzylol force-pushed the refactor/current-series-planner-readouts branch from bac6b16 to 9dfc40c Compare September 30, 2026 09:19
@zzylol
zzylol force-pushed the feat/per-series-arithmetic-warm branch from ac9d15a to 56a245d Compare September 30, 2026 09:19
zzylol and others added 2 commits September 30, 2026 09:46
Remove the trial compile that made the identity-typed workload the primary
request: every forest reaches the same enumeration and pricing, and the
trial compiled a request that differed from the deployed one. Reselect only
on Planner's missing-identity error, reject a typed forest whose unretyped
root shares a state with a retyped root, and record rejections and tagged
typed-selection entries in the selection trace. The overflow process test
now prices stateful candidates cheapest instead of relying on forest order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <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