Repository navigation
fix(logical): a quantile of merged panes keeps its samples' sign, so pane DDSketches compose through avg - #795
Merged
Merged
Conversation
A quantile read from merged tumbling panes of one definition (SummaryMerge over a PaneChain) is still one of the samples, or between two of them, just like the same quantile over the raw window. It now carries the metric's sign statistic, so a per-series DDSketch built in panes at ingestion composes its relative bound through avg and sum once the gauge is declared non-negative. The phi in [0, 1] guard and every other stop (a filter, a value projection, abs, delta, deriv, an @ anchor) are unchanged. Tests: the sign rule over merged panes, including phi outside [0, 1]; avg by (region, service, job) (quantile_over_time(0.99, data[100s])) without raw data, planned with plan_installable: the merged-pane DDSketch plan is selected with a relative bound of 0.05 and executed against the query-time reference and the exact answer; without the declaration it is still rejected.
zzylol
force-pushed
the
fix/sign-through-pane-merge
branch
from
October 21, 2026 02:07
4234f9f to
1ffc38f
Compare
GordonYuanyc
added a commit
that referenced
this pull request
Oct 21, 2026
Planner main fails clippy (-D warnings) and one runtime_install test since #787, #789, #795 and the joint search commit: - summary_coverage.rs: a Window alias for the relative time window (type_complexity). - workload_candidates.rs: an array instead of vec! (useless_vec). - query_candidates.rs: OperatorNode::reachable instead of a closure. - stage1_to_stage3.rs: allow too_many_arguments on search_stage2_and_stage3, as lib.rs already does once. - joint_search_equivalence.rs: struct initializer for the settings, and slice::from_ref instead of a clone. - runtime_install.rs: the merged-pane DDSketch candidate of the p99 avg workload is now P3-m1 (was P5-m1); every other assertion of that test passes unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This was referenced Oct 21, 2026
zzylol
pushed a commit
that referenced
this pull request
Oct 21, 2026
refactor: follow asap_sketchlib's UnivMon, Hydra and HLL renames UnivMon fields rows/cols/layers/total_weight/layer_heaps, new/clear/ update_weighted/estimate_*, OwnedDataInput, UNIVMON_MAX_LAYERS; Hydra rows/cols and HydraCounter::CountMinSketch; HLL update_with_hash. refactor!: HLL, KLL, DDSketch, Count-Min and KMV kernels on native asap_sketchlib types The portable HllSketch, KllSketch, DdSketch and CountMinSketch are gone from asap_sketchlib. HLL is a classic HyperLogLogImpl at each precision 4..=18 (register types from impl_hll_registers!); KLL is Kll::with_k; DDSketch the native DdSketch (get_value_at_quantile, get_count); plain Count-Min a regular-path CountMinF64 that adds every weight, zero and negative ones included; KMV the native Kmv. Memory charges come from Kll::memory_bytes, Kmv::memory_bytes and CountMinSketch::memory_bytes. The DDSketch ratio tests interpolate PromQL quantiles from the sketch's order statistics. refactor!: weighted frequency on asap_sketchlib's f64 Count-Min and Count Sketch asap_sketchlib drops WeightedFrequency. The Planner kernel keeps its API over CountMinSketchWithHeap / CountSketchWithHeap (capacity > 0) or a plain CountMinSketch / CountSketch (capacity 0), all Vector2D<f64> on the regular path. An identity tuple is one TupleKey, hashed and retained as bytes. Planner checks what sketchlib does not: finite weights (and nonnegative for Count-Min), finite Float64 identities, an odd Count Sketch depth, and a total weight magnitude within f64. Top-k rows are the heap's keys at their current estimate, descending, ties by key bytes. refactor!: HydraKLL on asap_sketchlib's Hydra with KLL cells (#644) HydraKllSketch is gone from asap_sketchlib. The kernel is a one-column Hydra over HydraCounter::Kll, read by Hydra's median of the cells' KLL quantiles, and charged the cells' Kll::memory_bytes. refactor: box the weighted frequency state held as a template Its heap makes it far larger than the other group-state kinds. test: fixtures that still exercise native KLL memory and DDSketch quantiles Kll allocates its item buffer as items arrive, so each stored-pane state takes 1,100 samples to hold its full buffer (about 11 kB) again; with one sample the merge fit the 4 MiB limit. DdSketch reads its minimum and maximum exactly, so the cross-zero ratio fixture interpolates interior order statistics: [-2, -1, 1.011, 2] instead of [-1, 1.011]. chore!: pin asap_sketchlib to main (de15d66) asap_sketchlib main, with ProjectASAP/asap_sketchlib#242 merged. UnivMon builds any row count its 128-bit key hash covers, so the UnivMon shape tests probe that budget instead of a 20-row limit. Merge remote-tracking branch 'origin/main' into chore/asap-sketchlib-main fix: cap UnivMon at the 20 rows its ASAPv1 state encodes sketchlib main's UnivMon::check_shape accepts any row count within the 128-bit key hash, but its ASAPv1 encoder still refuses more than MATRIX_MAX_ROWS rows. Keep the executor's builds and the Planner's hash_independent at 20 rows, as on the old pin, so every UnivMon state the executor builds can be stored. fix: count the KLL and weighted frequency structs once in memory charges Kll::memory_bytes and the heap sketches' memory_bytes already include their own struct; adding size_of::<Self>() on top counted it twice. chore: pin asap_sketchlib to release 0.4.0 (eb04e7f) eb04e7f is the 0.4.0 release commit on top of de15d66; the library code is the same, only the version, CHANGELOG and README change. Merge remote-tracking branch 'origin/main' into chore/asap-sketchlib-main # Conflicts: # crates/plan-selection/src/lib.rs fix: clippy on main's new code and the renumbered P99 pane candidate Planner main fails clippy (-D warnings) and one runtime_install test since #787, #789, #795 and the joint search commit: - summary_coverage.rs: a Window alias for the relative time window (type_complexity). - workload_candidates.rs: an array instead of vec! (useless_vec). - query_candidates.rs: OperatorNode::reachable instead of a closure. - stage1_to_stage3.rs: allow too_many_arguments on search_stage2_and_stage3, as lib.rs already does once. - joint_search_equivalence.rs: struct initializer for the settings, and slice::from_ref instead of a clone. - runtime_install.rs: the merged-pane DDSketch candidate of the p99 avg workload is now P3-m1 (was P5-m1); every other assertion of that test passes unchanged.
This was referenced Oct 21, 2026
zzylol
added a commit
that referenced
this pull request
Oct 21, 2026
The DDSketch updater refused every sample <= 0, so a DDSketch build over a gauge that ever reports exactly 0 (or a negative value) failed at run time: #795's P5-m1 over a gauge declared non-negative, and every DDSketch selected under ignore_accuracy_guarantees. asap_sketchlib's DdSketch keeps a zero bucket and a negative store, so the updater now accepts zero and any finite sample whose magnitude is within ddsketch_indexable_bounds, positive or negative, and adds it with try_add. NaN, +-Inf, magnitudes above the range, and nonzero magnitudes below it (which the zero bucket would take without the relative error) are still refused. The DDSketch guarantee (relative error alpha per quantile) does not assume positive values; whether it carries through sum and avg still depends on the samples' declared sign, unchanged. Tests: the updater's domain and its quantiles over 0, 0, 3, 5 and over signed samples; through plan_installable and a stand-in runtime, a DDSketch-pane plan over a gauge reporting zero (avg of p99s, declared non-negative) and over signed samples including all-negative series (max of p99s), each equal to query_time_reference and within 0.05 of the exact interpolated values. Two executor tests that locked the old positive-only domain now check the new one.
zzylol
added a commit
that referenced
this pull request
Oct 21, 2026
The DDSketch updater refused every sample <= 0, so a DDSketch build over a gauge that ever reports exactly 0 (or a negative value) failed at run time: #795's P5-m1 over a gauge declared non-negative, and every DDSketch selected under ignore_accuracy_guarantees. asap_sketchlib's DdSketch keeps a zero bucket and a negative store, so the updater now accepts zero and any finite sample whose magnitude is within ddsketch_indexable_bounds, positive or negative, and adds it with try_add. NaN, +-Inf, magnitudes above the range, and nonzero magnitudes below it (which the zero bucket would take without the relative error) are still refused. The DDSketch guarantee (relative error alpha per quantile) does not assume positive values; whether it carries through sum and avg still depends on the samples' declared sign, unchanged. Tests: the updater's domain and its quantiles over 0, 0, 3, 5 and over signed samples; through plan_installable and a stand-in runtime, a DDSketch-pane plan over a gauge reporting zero (avg of p99s, declared non-negative) and over signed samples including all-negative series (max of p99s), each equal to query_time_reference and within 0.05 of the exact interpolated values. Two executor tests that locked the old positive-only domain now check the new one.
zzylol
added a commit
that referenced
this pull request
Oct 21, 2026
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.
zzylol
added a commit
that referenced
this pull request
Oct 21, 2026
…les at the estimate level, sums at the state level) (#804) * fix(logical): one per-series summary serves every grouping of its metric 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. * fix(logical): a rollup Pass 2 candidate is also built over tumbling panes 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. * test: execute paned reuse candidates per pane, and per-series answers 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.
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
Without retained raw data, the MVP benchmark's p99 query cannot be planned:
(every 1 s, max_error 0.05, failure probability 0.3,
dataa gauge declared"values_non_negative": true,raw_data_retained: false). Stage 3 rejects every candidate. The only candidates that read no raw data are the per-series DDSketch panes (P5, and P5-m1 built at ingestion). They were rejected withaccuracy_target_missed, "Relative bound None", because the samples' sign never reachedavg.#780 attaches the sign of a metric's samples to a quantile estimated over the raw window (
estimates_samples_of), but not to a quantile read from merged tumbling panes. A merged DDSketch keeps its alpha, and a quantile of merged panes is still one of the samples (or between two of them), so the sign carries over in the same way.What
SummaryMergeover aPaneChainof one definition now carries the samples' sign statistic, just as the same quantile over the raw window does. The rule does not depend on the family, so it covers KLL panes as well. KLL's rank error still has no rule throughavg, so KLL candidates are unchanged.avgand is selected. Without the declaration, it is still rejected.abs,delta/derivand an@anchor still end the sign.How
In
estimates_samples_of, the quantile arm resolves its summary input throughmerged_chain(aSummaryMergeof exactly onePaneChain, or the chain itself). It then matches either aSummaryAggor aPaneChainthat has no item, weightSampleValueand no filter, and recurses into the chain's input (the input the range selector reads) with the existing strict rule. The guarantee composed for merged panes (merged_panes_guarantee) and the executor's pane expansion are not changed.Key code interfaces added / modified
None. The change is inside the private
estimates_samples_ofincrates/logical-asap-optimization/src/per_computation_candidates.rs.Usage (end to end, from
crates/planner/tests/runtime_install.rs, run at this head):stage_pipelineon the benchmark workload (q03, withvalues_non_negative: true) and itsdeployment_capabilities.json:raw_data_not_retainedraw_data_not_retainedno_accuracy_guaranteeno_accuracy_guaranteeraw_data_not_retainedraw_data_not_retainedaccuracy_target_missed(Relative bound None)raw_data_not_retained(now passes accuracy)accuracy_target_missed(Relative bound None)The same workload without the declaration still has no valid plan: P4, P5 and P5-m1 give
accuracy_target_missed, Relative bound None.Before this PR
Without raw data, a per-series quantile sketch had no relative bound through
avg/sumwhen it was built in panes, even over a declared non-negative gauge. The benchmark query had no valid plan.After this PR
A declared non-negative gauge's quantile sketch, built in panes at ingestion and merged at query time, keeps its relative bound through
avgandsum. The benchmark query plans as P5-m1.Verification
Tests were written first and fail on main (8f5bf8e):
a_quantile_of_merged_panes_shares_its_samples_sign(logical): a quantile overSummaryMerge[PaneChain]ofmnamesvalues_non_negative[m]in its fold stats, and φ = -1 and 2 carry no sign. On main:NonevsSome("m").a_non_negative_gauges_merged_ddsketch_panes_carry_the_relative_bound_through_avg(planner,plan_installable): P5-m1 is selected, with no query-time raw windows, a relative bound ≤ 0.05 and failure probability ≤ 0.3. It is then executed with the executor over 8 series in 4 groups, with 100 one-second panes built in ingestion runs. Its answers equalquery_time_reference's and are within 5% of the exact averages of the per-series p99s. On main: "no valid candidate", with P5-m1accuracy_target_missed.an_undeclared_gauges_merged_ddsketch_panes_get_no_relative_bound_through_avg: without the declaration, planning fails and names P5-m1.cargo test --workspace --locked: 1785 passed, 0 failed.Limitations
SummaryMergeof onePaneChain, the shape the window-composition rule produces. A merge of several independently built states is not covered.q03-quantile.jsondoes not declarevalues_non_negativeyet. It needs"values_non_negative": trueondatato plan.