Repository navigation
fix: one finest-grain stored summary serves several groupings (quantiles at the estimate level, sums at the state level) - #804
Merged
Conversation
A per-series summary is keyed by the series identity, but each query's PromQL scan projects only the labels its own `by` needs, so `avg by (region) (quantile_over_time(0.99, data[100s]))` and `avg by (region, service, job) (quantile_over_time(0.9, data[100s]))` got different per-series DDSketch definitions, and so did the per-series Sum panes of a fine and a coarse `sum by (...) (sum_over_time(...))`. - The summary-capability rule now groups per-series builds whose scans of one metric (no matchers, same windows and shifts) project different labels, and also per-series exact accumulators read only through finalization. The shared build is the member over the scan with every member's labels; each other member's reader (estimate or finalize) reads it and a Project keeps that reader's columns by name. - A projection of columns keeps a quantile estimate's sample sign, so the relative bound still carries through avg over the shared sketch. - CapabilitySharingCandidate.family is the first member's FieldDataType. Tests: three quantile queries with different groupings store one DDSketch definition, and a fine and a coarse sum one Sum definition, each answering as its query-time reference; the stored-output tests of #781 and #795 follow the shared plans.
…anes The rollup rule's Pass 2 candidates were never composed with the window-composition rule, so a rolled-up plan read its finer build's whole window from raw samples, and a deployment that keeps no raw samples rejected every rollup. After each rollup Pass 2 candidate, its window-composition Pass 2 candidates are listed too: the finer build as panes, the coarse computation rolled up from their merged state. Test: for a fine and a coarse sum over one window, some Stage 1 candidate both rolls up and builds panes (it failed before). The reuse-rule execution test leaves out paned candidates, whose panes it does not feed.
… beside a grouping The reuse execution test expands pane chains and feeds each raw TimeRange the samples in its own window, so rolled-up candidates built over tumbling panes are executed instead of skipped. A new runtime test plans a top-k of per-series sums and per-series DDSketch quantiles beside a grouping of the same values and checks one stored state and answers equal to the query-time reference. Docs of the capability-sharing rule name the label-superset case; an unused import is moved to the tests.
zzylol
force-pushed
the
fix/per-series-summary-shares-identity
branch
from
October 21, 2026 09:16
45a0049 to
7782883
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
ASAPQuery keeps one finest-grain summary per series and answers every grouping of a metric from it: one sketch per series for the quantiles, one per-series sum for a fine and a coarse sum. The joint plan built one copy per query instead. Each query's PromQL scan projects only the labels its own
byneeds, so per-series summaries holding the same states got different definitions:avg by (region) (quantile_over_time(0.99, data[100s]))andavg by (region, service, job) (quantile_over_time(0.9, data[100s]))stored two per-series DDSketch definitions, about 1.6 GB each at ASAPQuery's scale.sum by (region, service, job) (sum_over_time(data[W]))andsum by (region) (sum_over_time(data[W]))stored two per-series Sums.What
In both cases, one finest-grain stored summary now serves several groupings.
avg by G. Sketches are not merged.How
same_rows), the builds no filter, and both inputs a$promql_series_identitycolumn. A per-series state is keyed by the series identity, which determines every label, so these builds hold the same states. Besides sketches, the rule now covers per-series exact accumulators read only throughFinalizeExactAccumulator.Projectthat keeps that reader's own columns by name, so the operators above it are unchanged.Project. A projection of columns now keeps the sample sign of the values below it, so DDSketch's relative bound still carries throughavgover a declared non-negative gauge. Only bare column references pass, so no computed value inherits the sign.Key code interfaces added / modified
crates/logical-asap-optimization/src/asap_aware_cse.rsUsage (the tests
quantile_queries_with_different_groupings_share_one_per_series_sketchandfine_and_coarse_sums_store_one_sum):Before this PR
After this PR
Verification
Tests written first.
planner/tests/runtime_install.rs:quantile_queries_with_different_groupings_share_one_per_series_sketchfailed onmainwith 3 DDSketch definitions.fine_and_coarse_sums_store_one_sumfailed onmainwith 2 Sum definitions.query_time_reference(2, 6 and 3 answers for the quantile queries; 6 and 2 for the sums).logical-asap-optimizationa_rollup_is_also_built_over_tumbling_panes: some Stage 1 candidate of the sum pair both rolls up and builds panes. It failed without the rollup change.Added in review.
per_series_answers_beside_a_grouping_share_one_state_and_answer_as_the_reference:topk(2, sum_over_time(...))besidesum by (region), and a per-seriesquantile_over_timebesideavg by (region). Each stores one per-series state, and every answer, including the top-k series' labels, equals the query-time reference.reuse_rulesexecution test now expands pane chains and feeds each rawTimeRangethe samples in its own window, so the paned rollup candidate is executed and compared, not skipped.Existing tests updated to the shared plans.
sum by (job)besidesum by (job, instance)is now two group sums built from one stored per-series sum (renamedgroup_sums_from_one_stored_per_series_sum_are_listed_and_built). The cross-series maximum now shares its per-series maxima with the per-series query.main's per-query check, not by this PR: an average of KLL quantiles has no relative bound, so the query planned alone fails withNoAccuracyGuarantee, and the exhaustive search rejects the KLL candidates for the same reason.Commands.
cargo test --workspace --locked, after rebasing ontomain: all test binaries passed. Under the current speed-first policy,clippywas not run.Limitations and follow-up.
by (...)reduction at ingestion (DEFAULT_GROUP_COUNT), even over 6 series, so the rollup's finer build prices higher. Follow-up: cap the assumed group count at the declared series count.by (region)andby (job)alone, still get separate builds; sharing them would need a scan with the union of the labels. Scans with label matchers are not grouped.