Skip to content

chore: pin asap_sketchlib to main 0.4.0 and port the executor to its APIs - #790

Closed
zzylol wants to merge 1 commit into
mainfrom
chore/sketchlib-main-pin
Closed

zzylol wants to merge 1 commit into
mainfrom
chore/sketchlib-main-pin

Conversation

@zzylol

@zzylol zzylol commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Closes #785

Why

The workspace pinned asap_sketchlib to the private branch planner-pin/sketch-library-fixes (3f44cf1c). That branch is an old main plus open PRs, so the Planner could not take a sketchlib fix without staying on a private branch. Sketchlib main 0.4.0 (eb04e7f) now holds those fixes (#242). It also makes ASAPv1 the only serialization (#241), renames types (#239, #240), and adds f64 Count-Min and Count Sketch with candidate heaps and TupleKey keys (#232, #237). On main, KLL also allocates its item buffer lazily, which is the main CPU and memory win for quantile queries.

What

  • Cargo.toml / Cargo.lock pin asap_sketchlib 0.4.0 at eb04e7f. prost, bincode, serde-big-array and xxhash-rust leave the lock file.
  • The executor uses main's types and names everywhere. The executor encodes no state itself, so there is no proto, portable or MessagePack path left to remove. The removed portable wrappers (CountMinSketch, KllSketch, DdSketch, HllSketch, HydraKllSketch) are replaced by main's native sketches.
  • Weighted frequency. The executor's WeightedFrequency (CMS / Count Sketch with heap, and plain Count Sketch at heap capacity 0) is now an adapter over main's CountMinSketchWithHeap<Vector2D<f64>, RegularPath> / CountSketchWithHeap<…>. Items are keyed by their canonical TupleKey bytes. Sketchlib's WeightedFrequency kernel is gone.
  • Memory charges use the library's memory_bytes where it exists: KLL, KMV, plain Count-Min, and the heap sketches. The KLL 4,608 + 32·k formula is removed. The shared-state charging rule is unchanged: stored states a query reads are not charged again.
  • HydraKllSketchAccumulator and HydraKllAccumulatorUpdater are deleted. Nothing built them, and their library type is gone. Hydra-KLL is out of scope.

How

  • Weight checks. The weight rule (finite, and non-negative for Count-Min) stays in the executor until sketchlib QueryExpr: merge BinaryOp into Compare/Arithmetic (Instance 2 of #220) #245 (check_weight, try_update_weighted) is released. The executor also keeps every counter finite: it tracks the summed magnitude of every weight added or merged in. Since no counter can exceed that sum, an update or merge that would push the sum past f64::MAX is refused with the state unchanged. The Planner's CountMinWeightsNotProvenNonNegative rule keeps its meaning, because Count-Min still rejects negative inputs at validation.
  • Top-k depends on fresh rescoring. Main's heap rescores only the key an update touches, so other residents' counts go stale. The old kernel rescored every candidate after every update. The executor's top-k rows (scores and which keys survive) depend on that, so the adapter rescores every resident after each update (TopKHeap::update_owned, which never evicts) before it offers the updated key. This costs O(heap_size · depth) per update, the same as the old kernel. When QueryExpr: merge BinaryOp into Compare/Arithmetic (Instance 2 of #220) #245's rescore_heap is released it can replace this loop. Merges use main's merge, which re-estimates both heaps' candidates.
  • DDSketch quantile. Main's DdSketch::get_value_at_quantile takes the nearest rank ⌈q·n⌉, clamped to [min, max]. The executor answered with the lower order statistic at rank ⌊q·(n−1)⌋, the bucket representative γ^i(1+α). To keep answers equal, DDSketchAccumulator::quantile reads that rank from the stores main exposes (positive_store_counts, negative_store_counts, zero_count). quantile_interpolated, which the two integration tests of the checked-ratio rule used, moves next to it. The bucket mapping (⌊ln v / ln γ⌋) is the same in both versions.
  • HLL. Main fixes the precision in the register type. The executor builds every precision in 4..=18, so it defines the missing register types with impl_hll_registers! and wraps them in one enum.

Key code interfaces added / modified

crates/executor/src/summary_kernels/weighted_frequency.rs

/// The counter matrix of a WeightedFrequency state (stays; was re-exported from sketchlib).
pub enum FrequencyAlgorithm {
    /// Count-Min: nonnegative weights, minimum over rows.   // unchanged
    Cms,
    /// Count Sketch: signed weights, median over odd rows.  // unchanged
    CountSketch,
}
/// Keyed f64 frequency state over sketchlib's heap sketches (stays; internals replaced).
pub struct WeightedFrequency { /* private */ }
impl WeightedFrequency {
    /// Shapes it builds: sketchlib's, plus an odd Count Sketch depth.
    pub fn check_shape(algorithm: FrequencyAlgorithm, width: usize, depth: usize) -> Result<(), Error>; // new (was sketchlib's)
    /// The counters, depth rows of width, so tests can compare states cell by cell.
    pub fn counters(&self) -> &[f64]; // new
}

crates/executor/src/summary_kernels/dd_sketch.rs

impl DDSketchAccumulator {
    /// Adds one sample; non-finite or out-of-range values are ignored.
    pub fn update(&mut self, value: f64); // new
    /// Lower order statistic at ⌊q(n−1)⌋, the executor's quantile answer.
    pub fn quantile(&self, q: f64) -> Option<f64>; // new (was the portable DdSketch's)
    /// PromQL-style interpolation between adjacent order statistics.
    pub fn quantile_interpolated(&self, q: f64) -> Option<f64>; // new (was the portable DdSketch's)
    /// Bucket representative of the sample at a 0-based rank.
    pub fn value_at_rank(&self, rank: u64) -> Option<f64>; // new
}

crates/executor/src/summary_kernels/hll_sketch.rs

/// The precisions the executor builds.
pub const HLL_PRECISIONS: RangeInclusive<u8> = 4..=18; // new
pub struct HllSketchAccumulator { /* private */ } // `inner: HllSketch` was public
impl HllSketchAccumulator {
    pub fn new(precision: u8) -> Result<Self, KernelError>; // was new(HllVariant, u32) -> Self
    pub fn precision(&self) -> u8;               // new
    pub fn insert_sample(&mut self, value: f64); // new
    pub fn insert_bytes(&mut self, item: &[u8]); // new
    pub fn clear(&mut self);                     // new
}

crates/executor/src/summary_kernels/count_min_sketch.rs, datasketches_kll.rs, univmon.rs, hydra_kll.rs (removed)

/// f64 Count-Min, FastPath, items keyed by their text.
pub type CountMinSketchF64 = asap_sketchlib::CountMinSketch<Vector2D<f64>, FastPath>; // new
pub struct CountMinSketchAccumulator { pub inner: CountMinSketchF64 } // was portable CountMinSketch
impl CountMinSketchAccumulator { pub fn update(&mut self, item: &str, weight: f64); } // new
pub struct DatasketchesKLLAccumulator { pub inner: asap_sketchlib::Kll } // was KllSketch
/// UnivMon shapes the executor builds: sketchlib's, rows ≤ MATRIX_MAX_ROWS.
pub fn check_shape(heap_size: usize, rows: usize, cols: usize, layers: usize) -> Result<(), Error>; // new
pub struct HydraKllSketchAccumulator; // removed (never built)
pub struct HydraKllAccumulatorUpdater; // removed (never built)
pub(crate) fn kll_memory_bytes(k: u16) -> usize; // removed

Usage. A PromQL topk by (service) (2, sum_over_time(m[5m])) with AccuracyTarget::max_error(0.01).failure_probability(0.01) reaches this PR when plan selection gets a at a CountSketchWithHeap { width: 7500, depth: 85, heap_size: 50 } raw-sample summary. precompute_raw_samples::raw_sample_summaries_compile_and_match_their_kernels compiles that DAG and executes it. The keyed build calls WeightedFrequency::new(CountSketch, 7500, 85, 50) and then update(&[Utf8(item)], weight) per sample. keyed_evaluation reads rows(2), and the test checks that every item's score equals its exact total.

Before this PR

  • The workspace built only against the private pin 3f44cf1c.
  • The executor used sketchlib's WeightedFrequency kernel and the legacy portable wrappers.
  • KLL memory was charged from a fixed formula (4,608 + 32·k, about 11 kB at k = 200) whatever the stream.

After this PR

  • The workspace builds against sketchlib main 0.4.0, with the same answers on every executor parity test, the StateColumnUpdater parity test, runtime_install, and every planner-layering example.
  • KLL states charge what they hold. A k = 200 KLL with one sample charges about 0.7 kB, and about 5.6 kB once its buffer is full.

Changed or relaxed expectations

  • stored_pane_memory_limit.rs: each KLL state now charges its lazily grown buffer, about 0.7 kB with the old 1 sample per series. The panes now take 1,000 samples per series, so each KLL fills its buffer (about 5.6 kB). The limits, and what each test proves, are unchanged; the size comments are updated.
  • deployment.rs, state_column_updater.rs, row_reference: state fingerprints and cell comparisons read WeightedFrequency::counters() and rows() instead of the kernel's serde form, which is gone.
  • promql_numeric_regressions.rs: the CMS/CS weight check uses main's f64 CountMinSketch / CountSketch. Both DDSketch interpolation tests use DDSketchAccumulator::quantile_interpolated.
  • univmon.rs test: init_univmon / fast_insert / free become new / update_weighted_terminal / clear.

Answer changes and deviations

  • HLL: main's classic estimate() returns usize, so the executor's distinct count is now rounded down to an integer (by less than 1). The register updates and the estimator formula are the same. No test changed.
  • Heap ties: the old kernel broke score ties by bincode key bytes, both in output order and in eviction. Now output ties are ordered by TupleKey bytes, and a new key that ties the heap minimum is not admitted (main's heap admits strictly larger counts). This differs only for exactly tied scores; no test changed.
  • Point estimates hash TupleKey bytes with main's per-row seeds instead of bincode bytes with xxh64(2·row). The same answers hold where keys do not collide, and the same bounds hold where they do.
  • No rows ≤ MATRIX_MAX_ROWS check on weighted frequency. Move the asap_sketchlib pin to a released sketchlib main #785 asked for it, but plan selection sizes Count Sketch heaps from the binomial median tail: topk … max_error(0.01).failure_probability(0.01) gets a at depth: 85, and native_count_sketch_dimensions_are_not_packed_wire_dimensions builds depth 55. With the check, the executor refused those planner-chosen plans (precompute_raw_samples lost its CountSketchWithHeap fixture). Past 20 rows, main derives each row's seed as SEED_LIST[i % 20]·(i/20+1) + i/20, and deep_heap_sketch_rows_hash_independently (25 rows) still shows independent rows. The cost is that a state past 20 rows has no ASAPv1 encoding, which the executor does not use. Needs a decision: keep this, or cap the Planner's sizing at 20 rows.
  • UnivMon keeps the old 20-row cap explicitly, in both the executor (univmon::check_shape) and the Planner (hash_independent). Main's in-memory UnivMon::check_shape dropped it, but its ASAPv1 encoder still refuses more than 20 rows, so the accept set is unchanged.

Follow-ups

  • Accuracy rules vs main's estimators (logical-asap-optimization/src/accuracy/estimators/*.rs): this PR only re-points the citations. univmon.rs now cites docs/api/api_univmon.md and estimate_l2 / estimate_cardinality, and MAX_LAYERS reads UNIVMON_MAX_LAYERS (19, unchanged). Main's estimator bodies match the pinned ones line for line except for renames and one change: estimate_g_sum now returns early on an empty sketch only when count_every_key is false. A full check of each bound against main's docs, and of the dependency on sketchlib feat: rewrite uniqExact to COUNT(DISTINCT ...) #221 (UnivMon reworked to follow the paper), is left for a follow-up.
  • plan-selection still sizes KLL by its maximum item buffer (kll_items); its docs say "allocates up front", which is now "grows up to". Cost model 1 is unchanged on purpose.
  • DDSketch: either adopt main's get_value_at_quantile (an answer change), or add a rank read upstream and drop value_at_rank.
  • HydraCms: main has Hydra::query_group_total, so a group total can be enabled (the capability check and comments still say Hydra lacks it).
  • Replace the per-update rescoring and weight checks with sketchlib QueryExpr: merge BinaryOp into Compare/Arithmetic (Instance 2 of #220) #245 once it is released.

Verification

  • cargo test --workspace --locked at 2dff6b98a2c294b9b8b33c106784604449b38376: 1,766 passed, 0 failed, 34 ignored (150 test binaries), including the executor parity tests, state_column_updater, stored_pane_memory_limit, runtime_install and planner_layering_example1–4. cargo check -p asap-planner --tests --locked passes.
  • Per the current policy, fmt, clippy and GitHub CI were not waited on.

…APIs

Pin eb04e7f (release 0.4.0) instead of planner-pin/sketch-library-fixes.
WeightedFrequency wraps main's f64 CountMinSketchWithHeap /
CountSketchWithHeap keyed by TupleKey bytes, with executor-side weight
checks and per-update heap rescoring until sketchlib #245 is released.
KLL, KMV, CMS and heap memory charges use the library's memory_bytes.
Remove the never-built HydraKLL accumulator.

Closes #785
@zzylol

zzylol commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Closing as a duplicate. The same pin move to asap_sketchlib 0.4.0 is being finished in #783, and the parts of #785 it lacked were added there as new commits.

@zzylol zzylol closed this Oct 21, 2026
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.

Move the asap_sketchlib pin to a released sketchlib main

1 participant