Skip to content

fix(logical): a quantile of merged panes keeps its samples' sign, so pane DDSketches compose through avg - #795

Merged
zzylol merged 1 commit into
mainfrom
fix/sign-through-pane-merge
Oct 21, 2026
Merged

zzylol merged 1 commit into
mainfrom
fix/sign-through-pane-merge

Conversation

@zzylol

@zzylol zzylol commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Why

Without retained raw data, the MVP benchmark's p99 query cannot be planned:

avg by (region, service, job) (quantile_over_time(0.99, data[100s]))

(every 1 s, max_error 0.05, failure probability 0.3, data a 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 with accuracy_target_missed, "Relative bound None", because the samples' sign never reached avg.

#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

  • A quantile estimate (φ in [0, 1]) read from SummaryMerge over a PaneChain of 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 through avg, so KLL candidates are unchanged.
  • With the declaration, P5-m1 gets DDSketch's relative bound composed through avg and is selected. Without the declaration, it is still rejected.
  • Unchanged: the φ ∈ [0, 1] guard, and every other stop. A filter, a value projection, abs, delta/deriv and an @ anchor still end the sign.

How

In estimates_samples_of, the quantile arm resolves its summary input through merged_chain (a SummaryMerge of exactly one PaneChain, or the chain itself). It then matches either a SummaryAgg or a PaneChain that has no item, weight SampleValue and 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_of in crates/logical-asap-optimization/src/per_computation_candidates.rs.

Usage (end to end, from crates/planner/tests/runtime_install.rs, run at this head):

// What the user writes: the benchmark's p99 query and the declared gauge.
let workload: PlanningWorkload = p99_avg_workload(/* values_non_negative */ true);
// The deployment keeps no raw samples.
let capabilities = DeploymentCapabilities { raw_data_retained: false, ..Default::default() };
// One Planner call. Stage 1 composes P5-m1 (DDSketch panes merged at query time).
// Its avg now sees values_non_negative[data] on the merged quantile, and Stage 3
// binds that statistic to 1 from the declaration.
let plan = plan_installable(UserInput::new(
    &workload,
    FrontendInput::Promql { histograms: None },
    PlanningModels::builtin().with_deployment_capabilities(&capabilities),
)).await?;
assert_eq!(plan.selection.unwrap().selected_candidate_id, "P5-m1");
// The root's guarantee once the sign and the 1,000 series are bound:
// Relative, error 0.05, failure probability 0 (DDSketch is deterministic).

stage_pipeline on the benchmark workload (q03, with values_non_negative: true) and its deployment_capabilities.json:

Candidate Before After
P1 raw_data_not_retained raw_data_not_retained
P2, P3, P3-m1 no_accuracy_guarantee no_accuracy_guarantee
P4 raw_data_not_retained raw_data_not_retained
P5 accuracy_target_missed (Relative bound None) raw_data_not_retained (now passes accuracy)
P5-m1 accuracy_target_missed (Relative bound None) selected: accepted relative (DDSketch), 21.22 cost/s: DDSketch in 100 panes of 1000 ms for 1000 groups

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/sum when 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 avg and sum. 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 over SummaryMerge[PaneChain] of m names values_non_negative[m] in its fold stats, and φ = -1 and 2 carry no sign. On main: None vs Some("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 equal query_time_reference's and are within 5% of the exact averages of the per-series p99s. On main: "no valid candidate", with P5-m1 accuracy_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

  • The rule follows only a SummaryMerge of one PaneChain, the shape the window-composition rule produces. A merge of several independently built states is not covered.
  • The harness workload q03-quantile.json does not declare values_non_negative yet. It needs "values_non_negative": true on data to plan.

@zzylol zzylol added accuracy Accuracy requirements, guarantees, and propagation bug Something isn't working promql PromQL front-end lowering (L1→L2) labels Oct 21, 2026
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
zzylol force-pushed the fix/sign-through-pane-merge branch from 4234f9f to 1ffc38f Compare October 21, 2026 02:07
@zzylol
zzylol merged commit 825504e into main Oct 21, 2026
1 of 3 checks passed
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]>
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.
zzylol added a commit that referenced this pull request Oct 21, 2026
…'s numbering

#795's tests were written against the exhaustive search's candidate
ids (P5-m1); with #792's default search the same plan is P3-m1, and a
failed selection lists the first rejection of each cause.
zzylol added a commit that referenced this pull request Oct 21, 2026
…'s numbering

#795's tests were written against the exhaustive search's candidate
ids (P5-m1); with #792's default search the same plan is P3-m1, and a
failed selection lists the first rejection of each cause.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accuracy Accuracy requirements, guarantees, and propagation bug Something isn't working promql PromQL front-end lowering (L1→L2)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant