Skip to content

refactor: compute current-series readouts as Planner physical programs - #799

Open
zzylol wants to merge 2 commits into
refactor/query-side-planner-compilefrom
refactor/current-series-planner-readouts
Open

zzylol wants to merge 2 commits into
refactor/query-side-planner-compilefrom
refactor/current-series-planner-readouts

Conversation

@zzylol

@zzylol zzylol commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Stacked on #798.

Why

Current-series readouts were still computed in storage. current_series.rs kept per-group caches and computed Sum, Count, Average, Quantile and TopK itself. Only TopK ran as a Planner program over the population snapshot. Planner's ReadPopulation compile covers all five readouts (coverage rows 22 and 23).

What

  • Populations are selected over roots typed with the complete series identity (promql_rows::with_series_identity), so compile_current_series_readout compiles every readout. A population is chosen only if its readout compiles.
  • Every current-series entry installs its Planner program (install_population_readout). Storage keeps the population and returns its members. execute_vectors runs the program.
  • Deleted: SeriesReadout and the readout field, the storage readout caches and computation, and the asap_current_series_cache_builds_total metric.
  • Population DAG validation accepts one value per group, with exactly the grouping labels. The adapter reads an Int64 count only when it is the sole numeric column.

Before / After

count by (job) (m)

Installed entry Who counts
Before CurrentSeries { population, readout: Count }, no physical DAG current_series.rs cached group sizes
After CurrentSeries { population } plus a Planner DAG with output [job, count] Planner Aggregate over the snapshot rows

Behaviour differences

  • without grouping (for example quantile without (pod) (0.5, m)) no longer uses a population. Planner cannot yet project without groups from the series identity, so these queries run exactly.
  • Sum and average use Planner's aggregate. It adds values in order, without the compensated summation the storage code used and Prometheus uses. Results can differ in the last bits, and an average whose sum overflows is +Inf rather than finite. This is a Planner follow-up.
  • Reads now copy the members out under the store lock instead of returning cached per-group results.
  • Plans that carry the removed readout field no longer deserialize.

Remaining

  • SeriesPopulation.max_k and quantiles no longer affect storage, but they remain in the population sharing key.
  • Planner panics in logical selection for sum without (pod) (m) and quantile without (pod) (0.5, m) with the workload-cost fixture (SummaryFamilySchemaMismatch). This happens before any backend code runs and is not addressed here.

Validation

  • cargo fmt --all -- --check, cargo clippy --workspace --all-targets --locked -- -D warnings
  • cargo test --workspace --locked --lib, cargo test -p control_plane --locked --tests
  • cargo test -p data_plane --locked --test asapquery_compatibility_process_e2e -- --test-threads=1 (26 passed). This includes current_series_quantiles_topk_share_and_replace_values, which checks quantile values, TopK and sum/count/avg through the process.
  • New or changed tests:
    • Every shared-population entry carries a valid Planner program.
    • A grouped count installs [job, count].
    • without populations are not selected.
    • The count and value columns are decoded correctly.
    • Storage snapshots follow membership, replacement, staleness and lookback.
  • An independent review agent, which did not write the code, found three issues. Deployment failed when a readout did not compile. The value-column choice was loose. Grouped-output validation was loose. All three are fixed. It also found the summation difference above, which is documented rather than fixed.

🤖 Generated with Claude Code

zzylol and others added 2 commits September 30, 2026 09:06
Storage computed current-series Sum, Count, Average, Quantile and TopK
itself from cached per-group arrays; only TopK was a Planner program over
the population snapshot. Populations are now selected over roots typed with
the complete series identity, so Planner compiles every ReadPopulation.
Storage keeps the population and returns its members; the entry's retained
physical program computes the readout. The readout field, the storage
readout caches and the cache-builds metric are removed.

`without` populations are not selected (Planner cannot yet project them
from the series identity); those queries run exactly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review follow-ups: a population is selected only if Planner compiles its
readout, so a failed compile runs the query exactly instead of failing the
deployment. Grouped population outputs must carry exactly the grouping
labels. An Int64 count is the sample only when it is the sole numeric
column. The summation difference from Prometheus is documented.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zzylol
zzylol force-pushed the refactor/query-side-planner-compile branch from 0c9b78d to c1becfb Compare September 30, 2026 09:19
@zzylol
zzylol force-pushed the refactor/current-series-planner-readouts branch from bac6b16 to 9dfc40c Compare September 30, 2026 09:19
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