Skip to content

fix(executor): DDSketch interpolated quantiles and top-k membership on sketchlib planner-pin/0.4.0-plus - #803

Merged
zzylol merged 5 commits into
mainfrom
fix/sketchlib-0.4.0-plus-followups
Oct 21, 2026
Merged

zzylol merged 5 commits into
mainfrom
fix/sketchlib-0.4.0-plus-followups

Conversation

@zzylol

@zzylol zzylol commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Why

#783 moved the executor to sketchlib main. Two kernel answers then drifted from what PromQL and the old kernels returned:

  • DDSketch quantiles. The kernel read sketchlib's nearest rank ⌈q·n⌉. PromQL quantile_over_time and SQL percentile_cont instead interpolate between adjacent order statistics. On the coverage run, 26 of the 90 quantile queries missed their exact answers by more than the 5% target (max 0.0517).
  • Top-k membership. sketchlib's heap rescores only the key an update touches, so another resident sharing a cell keeps a stale count. A new key could then be admitted against that stale minimum and evict a resident whose current estimate is higher. The old kernel rescored every candidate after every update.

The fixes need sketchlib PRs that are still open, so this PR pins a branch that carries them.

What

How

  • The DDSketch kernel's estimate is inner.quantile_interpolated(q)?. This turns Ok(None) into the empty-population error, and sketchlib's error for an out-of-range q passes through.
  • WeightedFrequency::update_key uses try_update_weighted on the inner Count-Min or Count Sketch, then rescore_heap(), then heap_mut().update(key, estimate). Plain sketches (capacity 0) use try_update_weighted directly.
  • The Planner's DDSketch rule (ddsketch_relative_error_alpha_v1) states alpha relative error on the value read and assumes no rank convention. The interpolated read is within alpha when both neighbours share a sign or one of them is zero. Since fix(executor): a DDSketch takes zero and negative samples #802 a DDSketch also takes negative samples, and an undeclared gauge's quantile keeps its relative bound (through max, for example). When the two neighbours have strictly opposite signs, the error is bounded only in absolute terms, alpha · ((1 − f) |lower| + f |upper|). For example, the median of -100, -100, 99, 99 reads -0.748 against -0.5. The nearest-rank read was further off there (-100), so this PR narrows the gap rather than opening it. A DDSketch quantile's relative bound fails when the interpolated neighbours straddle zero #807 tracks the guarantee. The bound is unchanged.

Key code interfaces added / modified

Cargo.toml

# `planner-pin/0.4.0-plus`: release 0.4.0 (eb04e7f) plus #245, #246, #247, #248.
asap_sketchlib = { git = "https://github.com/ProjectASAP/asap_sketchlib", rev = "63e565506426020c505ef0b84e0c91f450d09e7c" } # was rev = "eb04e7f…"

crates/executor/src/summary_kernels/weighted_frequency.rs

/// Keyed top-k (or, at capacity 0, plain) Float64 frequency state. The type stays.
pub struct WeightedFrequency {
    sketch: Sketch,          // unchanged
    width: usize,            // unchanged
    depth: usize,            // unchanged
    capacity: usize,         // unchanged
    total_magnitude: f64,    // removed: sketchlib's try_update_weighted keeps counters finite
}
impl WeightedFrequency {
    /// Adds `weight` to an identity tuple; an error, state unchanged, for a refused
    /// weight or identity, or an update carrying a counter past f64::MAX.
    pub fn update(&mut self, values: &[Value], weight: f64) -> Result<(), Error>; // unchanged signature; was: a total weight past f64::MAX
}

There are no other interface changes. The DDSketch kernel keeps its signatures; only the value it returns changes.

Usage. The coverage harness plans each random-workload quantile query (for example seed-862 workload-01 q03-quantile) with --no-raw --failure-probability 0.3. Plan selection picks a per-series DDSketch. At each check step the executor's SummaryEstimate calls DDSketchAccumulator::estimate(Quantile { q }), which now reads quantile_interpolated. The harness compares the answer with the exact PromQL quantile: the max relative error is 0.0500 here, against 0.0517 before.

Before this PR

  • DDSketch quantiles used nearest rank. For example, [20, 40] at q = 0.9 read 40, while PromQL gives 38. 26 of 90 coverage quantile queries were above 5% (max 0.0517).
  • Top-k heaps admitted keys against stale resident counts, so membership could differ from the old kernel.
  • refactor!: summary kernels on asap_sketchlib main (de15d66) #783 refused a Count Sketch update once the summed weight magnitude passed f64::MAX, even when the weights cancelled.

After this PR

  • The executor's DDSketch quantile is PromQL's interpolated quantile, within alpha. On coverage, the max error over the 90 quantile queries is 0.0500, with none above 5%.
  • Top-k membership matches rescoring every candidate after every update, up to exact ties.
  • Weight and overflow rules are the library's: a counter carried past f64::MAX is refused, and the state is unchanged.

Verification

  • Tests written first:
    • dd_sketch::quantile_interpolates_between_adjacent_order_statistics: [20, 40] at q = 0.9 is within alpha of 38. It failed on nearest rank and passes now.
    • weighted_frequency::membership_matches_rescoring_every_candidate_after_every_update: a reference model replays colliding Count-Min and Count Sketch streams. Without the rescore it fails at update 15 ([15, 4, 6, 23] vs [15, 1, 4, 6]); with it, it passes.
  • Changed expectations:
    • invalid_updates_leave_the_state_unchanged now expects the refusal from a counter overflow (the same key's cells at f64::MAX twice), not from the summed magnitude.
    • The two integration tests that rebuilt interpolation from nearest-rank reads now call quantile_interpolated.
    • Rebased onto fix(executor): a DDSketch takes zero and negative samples #802: its factory::ddsketch_domain::quantiles_over_zero_and_signed_samples expected nearest-rank values. It now expects PromQL's interpolated ones (0, 0, 3, 5 at q = 0.5 is 1.5; −7, −3, 0, 4 at q = 0.25 is −4). It failed with the old expectations after the rebase.
  • cargo test --workspace --locked after rebasing onto main 6849b1f1 (with test(planner): assert the undeclared-gauge p99 outcome, not a candidate id #801, fix(executor): a DDSketch takes zero and negative samples #802 and fix(planner): Stage 3 rejects plans the executor cannot compile; no PromQL top-k heap over its instant vector #805): 1,824 passed, 0 failed, 36 ignored.
  • Coverage (asapquery_random_workload, the coverage script's flags, all 90 PromQL quantile queries in seed-862, seed-862-x5 and seed-862-long, as is): max relative error 0.04996, none above 5%. The earlier run on 825504e6 had max 0.0517, with 26 above 5%.
  • fmt, clippy and CI were not run, per the current policy.

Follow-up: move the pin to a sketchlib release once #245–#248 merge.

Release 0.4.0 (eb04e7f) plus the open sketchlib PRs #245 (check_weight,
try_update_weighted, rescore_heap), #246 (KLL lazy-buffer follow-ups),
#247 (Count Sketch stack medians) and #248
(DdSketch::quantile_interpolated). Moves to a release once they merge.
…e_cont do

The DDSketch kernel read sketchlib's nearest rank ceil(q n). PromQL
quantile_over_time and SQL percentile_cont interpolate linearly between
the order statistics at ranks floor(q (n - 1)) and ceil(q (n - 1)), so
answers missed the exact ones by more than alpha: [20, 40] at q = 0.9
read 40 against 38. The kernel now reads
DdSketch::quantile_interpolated (sketchlib #248). An empty sketch still
gives no answer, and a q outside [0, 1] (NaN included) is still an
error; the Planner never sends one, since Pass 1 prunes the sketch
candidate (QuantileOutsideUnitInterval) and the exact plan answers +-Inf.

The Planner's DDSketch rule (ddsketch_relative_error_alpha_v1) states
alpha relative error on the value read and assumes no rank convention;
the interpolated read is within alpha when both neighbours share a sign,
which the sign rule already requires before the bound composes. The
bound is unchanged.

The two integration tests that rebuilt interpolation from nearest-rank
reads now call quantile_interpolated.
sketchlib's heap rescores only the key an update touches, so a resident
that shares a cell with it keeps a stale count, and a new key could be
admitted against that stale minimum and evict a resident whose current
estimate is higher. The old kernel rescored every candidate after every
update. Each heap update now writes the counters, calls rescore_heap
(sketchlib #245), and only then offers the updated key, so admission
compares against current estimates; O(heap_size) per update, as before.
rows() already reads every retained key at its current estimate, and a
merge re-estimates both heaps' keys, so the readout needs no extra pass.

A reference-model test (written first, failing at update 15 without the
rescore) replays colliding Count-Min and Count Sketch streams and checks
after every update that the retained keys match rescoring every
candidate, up to exact ties.

Weight checks now use sketchlib's check_weight and try_update_weighted:
the counters stay finite per probed cell, as in the old kernel, instead
of the Planner-side bound on the summed weight magnitude, so a merge
checks the merged counters instead.
@zzylol
zzylol force-pushed the fix/sketchlib-0.4.0-plus-followups branch from bf2d390 to 8eec7ba Compare October 21, 2026 07:43
@zzylol
zzylol merged commit 93b3de6 into main Oct 21, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant