Skip to content

feat(query): maintain shared current-series quantile and TopK state - #700

Merged
zzylol merged 1 commit into
mainfrom
feat/shared-series-quantiles
Sep 14, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/shared-series-quantiles

Conversation

@zzylol

@zzylol zzylol commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Rebuilt on top of main (ca944a6a) as the current-series feature alone.

Scope change from the original PR

The window / full-window half of this PR is dropped: #712 landed an equivalent
while this branch was open, and main's version is kept wherever the two overlapped
(covers_range, full_window_slide_ms derivation, the compatibility fixtures).
Rebasing the old branch as-is would have silently reverted #712.

What remains is the part main does not have: CurrentSeries is absent from main
entirely. 43 files, down from 49.

What this adds

Multiple current-value quantiles and TopK limits over the same metric share one
bounded maintained population instead of each planning its own state. Lowering
follows the Planner-selected IR rather than reconstructing intent from query text,
so a catalog rename cannot change the physical plan.

  • SeriesPopulation / SeriesReadout contract and LogicalOperator::CurrentSeries
  • Backend CurrentSeriesStore: bounded membership, lookback expiry, shared ordered
    state feeding quantile / TopK / sum / count / avg readouts
  • Planner bumped to b8b5d705 for typed ExactKind::Min, with the Min readout
    wired through catalog resolution and the exact-agg merge path
  • asap_current_series_populations / _cache_builds_total on /metrics

Review fixes carried in

A review of the original branch produced five findings; four are fixed here.

  • Budget overflow no longer latches permanently. It records when the budget
    blew and re-arms once the lookback window has moved past it. Previously one
    transient cardinality spike stranded the population on Prometheus until an
    operator forced a replan.
  • Quantile readout guards an empty value set instead of underflowing
    values.len() - 1 — that panic would have poisoned the mutex every remote-write
    batch waits on.
  • CompileError::Query reports query_id, not the query text.
  • Full-window sketch reads skip a layout-provably-empty sid instead of
    discarding the sids already accumulated and dropping the query to the exact path.

The fifth finding (backend install gate missing a full_window_slide_ms check)
was withdrawn on inspection: QueryPlan::validate_against_catalog already
rejects that shape, unconditionally, before the pane/origin loop runs.

Deliberately not included

The original branch also reversed count() over HLL from register-merge to
declined, deleting the ASAPController#163/#165 rationale comments without
mentioning it in any commit message. That is a behavioral change unrelated to
current-series — PromQL count() arguably should count series rather than
report merged cardinality, but it deserves its own PR and its own argument.
Main's semantics are kept here.

Verification

cargo fmt --check, clippy --workspace --all-targets -D warnings, and
cargo test --workspace -- --test-threads=12139 passed, 0 failed, 4 ignored.

🤖 Generated with Claude Code

@zzylol zzylol changed the title Support shared current-series quantiles and TopK maintenance Preserve Planner aggregate semantics through maintenance and serving Sep 12, 2026
zzylol added a commit that referenced this pull request Sep 13, 2026
Rebuilds #700 on top of main (ca944a6) as the current-series feature alone.
The window/full-window half of the original PR is dropped: #712 landed an
equivalent, and main's version is kept wherever the two overlapped.

Multiple current-value quantiles and TopK limits over the same metric now
share one bounded maintained population instead of each planning its own
state. Lowering follows the Planner-selected IR rather than reconstructing
intent from query text, so a catalog rename cannot change the physical plan.

- `SeriesPopulation` / `SeriesReadout` contract and `LogicalOperator::CurrentSeries`
- Backend `CurrentSeriesStore`: bounded members, lookback expiry, shared
  ordered state feeding quantile / TopK / sum / count / avg readouts
- Planner revision bumped to b8b5d705 for typed `ExactKind::Min`, with the
  Min readout wired through catalog resolution and the exact-agg merge path
- `asap_current_series_populations` / `_cache_builds_total` on /metrics

Review fixes carried in (from a review of the original PR):
- Budget overflow no longer latches permanently. It records when the budget
  blew and re-arms once the lookback window has moved past it, instead of
  stranding the plan on Prometheus until an operator forces a replan.
- Quantile readout guards an empty value set instead of underflowing
  `values.len() - 1` while holding the lock the remote-write path waits on.
- `CompileError::Query` reports `query_id`, not the query text.
- Full-window sketch reads skip a layout-provably-empty sid instead of
  discarding the sids already accumulated and dropping to the exact path.

Deliberately not included: the original PR also reversed `count()` over HLL
from register-merge to declined. That is a behavioral change unrelated to
current-series and belongs in its own PR with its own argument; main's
semantics are kept here.

Verified on this tree: cargo fmt --check, clippy --workspace --all-targets
-D warnings, and cargo test --workspace -- --test-threads=1
(2139 passed, 0 failed, 4 ignored).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@zzylol
zzylol force-pushed the feat/shared-series-quantiles branch from ea31d1b to 98d19a3 Compare September 13, 2026 22:18
@zzylol zzylol changed the title Preserve Planner aggregate semantics through maintenance and serving feat(query): maintain shared current-series quantile and TopK state Sep 13, 2026
Rebased onto main (84109cb). Carries only the current-series feature; the
window/full-window half of the original #700 was dropped when #712 landed an
equivalent, and main's version is kept wherever the two overlapped.

Multiple current-value quantiles and TopK limits over the same metric share one
bounded maintained population instead of each planning its own state. Lowering
follows the Planner-selected IR rather than reconstructing intent from query
text, so a catalog rename cannot change the physical plan.

- `SeriesPopulation` / `SeriesReadout` contract and the `CurrentSeries` residual
  operator
- Backend `CurrentSeriesStore`: bounded membership, lookback expiry, shared
  ordered state feeding quantile / TopK / sum / count / avg readouts
- Planner revision bumped to b8b5d705 for typed `ExactKind::Min`, wired through
  catalog resolution, the exact-agg merge path, and the backend wire sub-type
- `asap_current_series_populations` / `_cache_builds_total` on /metrics

Adopts main's current naming throughout (`PhysicalCompilationRequest`,
`PhysicalDeploymentContext`, `ResidualQueryOperator`, `query_plan::residual`,
`selected_plan_root`, `summary_store`, `allow_mixed_summary_and_exact_execution`)
rather than the deprecated compatibility aliases. Serde wire names and JSON
fixture keys are unchanged from main.

Review fixes carried in:
- Budget overflow no longer latches permanently. It records when the budget blew
  and re-arms once the lookback window has moved past it, instead of stranding
  the plan on Prometheus until an operator forces a replan.
- Quantile readout guards an empty value set instead of underflowing
  `values.len() - 1` while holding the lock the remote-write path waits on.
- Full-window sketch reads skip a layout-provably-empty sid instead of
  discarding the sids already accumulated and dropping to the exact path.

Verified on this tree: cargo fmt --check, clippy --workspace --all-targets
-D warnings, and cargo test --workspace -- --test-threads=1
(1750 passed, 0 failed, 14 ignored).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@zzylol
zzylol force-pushed the feat/shared-series-quantiles branch from 98d19a3 to e10c3fc Compare September 14, 2026 01:58
@zzylol
zzylol merged commit 8212857 into main Sep 14, 2026
1 check passed
@zzylol
zzylol deleted the feat/shared-series-quantiles branch September 14, 2026 03:11
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