feat: CountSketch + KLL + DDSketch + HLL decoders for modified-OTLP hot path (PR C) - #8
Merged
Merged
Conversation
…tSketch)
First of the per-sketch-type PR C follow-ups. Extends the Phase 1
modified-OTLP sketch hot path (PR A + PR B + PR D) to handle the
`CountSketch` variant end-to-end, alongside the existing CountMin
path.
What lands
----------
**1. sketch-core: new `count_sketch` module**
`asap-common/sketch-core/src/count_sketch.rs`
- Minimal `CountSketch` struct with a `Vec<Vec<f64>>` signed-counter
matrix — the same shape as the modified OTLP
`asap_sketchlib::proto::sketchlib::CountSketchState` wire format.
- `new`, `from_legacy_matrix`, `sketch`, `merge`, `merge_refs`,
`serialize_msgpack`, `deserialize_msgpack` — the element-wise merge
surface the precompute engine worker needs to call
`AggregateCore::merge_with` on two stored sketches.
- Error types use `Box<dyn Error + Send + Sync>` to match the trait
signatures downstream.
- 6 unit tests: empty, from_legacy_matrix, element-wise merge,
dimension-mismatch rejection, multi-input merge_refs,
msgpack round-trip.
**2. query_engine: new `CountSketchAccumulator`**
`asap-query-engine/src/precompute_operators/count_sketch_accumulator.rs`
- Wraps `sketch_core::count_sketch::CountSketch`.
- Implements `AggregateCore` (clone_boxed_core, type_name, as_any,
merge_with, get_accumulator_type, get_keys, query_statistic) and
`SerializableToSink` (serialize_to_json, serialize_to_bytes).
- `from_sketchlib_proto_bytes(buf)` — decoder for the modified OTLP
`CountSketchDataPoint.sketch` bytes. Decodes
`asap_sketchlib::proto::sketchlib::CountSketchState` via prost,
picks the `counts_int` or `counts_float` field based on the
`counter_type` enum, validates dims, reshapes row-major into a
`Vec<Vec<f64>>`, and constructs via `CountSketch::from_legacy_matrix`.
Mirrors PR B's `CountMinSketchAccumulator::from_sketchlib_proto_bytes`
but on the signed-counter `CountSketchState`.
- `query_statistic` returns an error today — the median-of-estimators
heavy-hitter query path and `TopKState` integration are deferred
to a follow-up. The matrix round-trip already works end-to-end
without that richer query surface, and queries against stored
CountSketch data fall through to the §5.2 fallback in the
meantime.
- Re-exported from `precompute_operators/mod.rs` alongside the other
accumulator types.
- 6 unit tests: int64 round-trip, float64 round-trip, dim mismatch,
zero-dim rejection, AggregateCore merge matches element-wise add,
merge_with rejects wrong type.
**3. AggregationType: new `CountSketch` variant**
`asap-common/dependencies/rs/promql_utilities/src/query_logics/enums.rs`
- Adds `CountSketch` to the `AggregationType` enum (sits next to
`CountMinSketch` / `CountMinSketchWithHeap`).
- Updates `as_str`, `is_keyed`, `is_multi_population_value_type`, and
`from_str` (canonical name + legacy aliases "CountSketchAccumulator",
"CS", "cs", "count_sketch").
**4. Dispatcher wiring in `otel.rs`**
`asap-query-engine/src/drivers/ingest/otel.rs`
- Replaces the `SketchKind::CountSketch` `Err("PR C")` arm in
`decode_modified_otlp_sketch_bytes` with a real
`CountSketchAccumulator::from_sketchlib_proto_bytes(bytes)` call.
- The other three sketch types (`Kll`, `DdSketch`, `Hll`) still
return `Err` — tracked in the remaining per-sketch PR C
follow-ups.
**5. E2E integration test**
`asap-query-engine/tests/e2e_modified_otlp_sketch_path.rs`
- New second test `e2e_count_sketch_modified_otlp_path` alongside
the existing CountMin test from PR D. Spawns its own
`PrecomputeEngine` + `OtlpReceiver::with_ingest_state`, POSTs a
protobuf-encoded `ExportMetricsServiceRequest` with
`Metric.data = CountSketch{…}` carrying a 2x4 **signed** matrix
(row 0 = [1,-2,3,-4], row 1 = [-5,6,-7,8]) encoded as
`CountSketchState`, advances the watermark via a second request,
waits for flush, drains the sink, downcasts to
`CountSketchAccumulator`, asserts the stored matrix is
bit-identical to what went in.
- Exercises every public surface: new `AggregationType::CountSketch`
variant, new `CountSketchAccumulator` decoder, new dispatcher
arm, plus the existing precompute engine sketch-pane merge +
window close + sink emission path.
- Uses non-overlapping ports (19510–19512 vs CountMin's
19500–19502) so both tests can run back-to-back.
Validation
----------
- `cargo check -p query_engine_rust --all-targets`: clean
- `cargo clippy -p query_engine_rust --all-targets -- -D warnings`: clean
- `cargo clippy -p sketch-core --all-targets -- -D warnings`: clean
- `cargo fmt --check`: clean
- `cargo test -p sketch-core count_sketch::`: 6 passed
- `cargo test -p query_engine_rust --lib`: **445 passed** (up from
439 after PR D — 6 new unit tests from the new accumulator module)
- `cargo test -p query_engine_rust --test e2e_modified_otlp_sketch_path`:
`test e2e_count_min_sketch_modified_otlp_path ... ok`
`test e2e_count_sketch_modified_otlp_path ... ok`
`test result: ok. 2 passed; 0 failed`
What's still PR C territory
---------------------------
- **PR C-HLL** — route sketchlib `HyperLogLogState` into either a
new `HllAccumulator` or the existing `SetAggregator`.
- **PR C-KLL** — convert sketchlib `KllState` proto into
`DatasketchesKLLAccumulator::inner`. Needs either a new
constructor on the upstream `asap_sketchlib::KLL` type or a
level-by-level reconstruction helper in `sketch-core/kll.rs`.
The largest of the four because the sketchlib KLL uses msgpack
serde with private state.
- **PR C-DDSketch** — create a new `DDSketchAccumulator` wrapping
sketchlib's `DdSketchState`.
- **PR C-delta** — delta transmission (`*_ENCODING_PROTO_DELTA`).
Per-series baseline tracking in the worker + delta-merge
codepath on each accumulator.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fills in the three remaining per-sketch-type decoders for the modified-OTLP Metric.data hot path in PR #8: * KLL: DatasketchesKLLAccumulator::from_sketchlib_proto_bytes decodes KllState via lossy statistical reconstruction (replays items through update() on a fresh sketch with the same k, since sketch-core's KLL backend types keep their level structure private; quantile estimates remain within KLL's own rank-error bound). * DDSketch: new sketch-core DdSketch type (bucket counts + alpha + aggregates, merge by store-index alignment) and DDSketchAccumulator wrapping it with full AggregateCore + SerializableToSink impls and a from_sketchlib_proto_bytes decoder for DdSketchState. * HLL: new sketch-core HllSketch type (registers + variant + precision + HIP accumulators, merge by register-wise max) and HllSketchAccumulator wrapping it with a from_sketchlib_proto_bytes decoder for HyperLogLogState. Wires all three into the drivers/ingest/otel.rs dispatcher (SketchKind::Kll, SketchKind::DdSketch, SketchKind::Hll) so COUNT-sketch-family and quantile-family sketches arriving via the modified-OTLP Metric.data variants now reach concrete accumulators instead of falling through to the §5.2 fallback. Adds AggregationType::DDSketch variant (HLL and DatasketchesKLL already existed). Query semantics for the new accumulators (cardinality estimation for HLL, quantile estimation on the log-bucket representation for DDSketch) are intentionally deferred to a follow-up — the matrix/register/bucket round-trip through merge + store already works end-to-end, and query_statistic returns a placeholder error that falls through to §5.2. Extends tests/e2e_modified_otlp_sketch_path.rs with three new end-to-end tests (e2e_kll_sketch_modified_otlp_path, e2e_dd_sketch_modified_otlp_path, e2e_hll_sketch_modified_otlp_path) that POST real ExportMetricsServiceRequest payloads through the OTLP HTTP receiver, route them through the PrecomputeEngine, close a window, and assert the captured accumulator's inner state matches what was sent. Plus 6+6 unit tests for the new sketch-core types and 6+4 unit tests for the new accumulators, including round-trip verification for each counter type / variant and rejection cases for invalid dimensions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 11, 2026
… + typed Predicate (#136) Additive lift of all 10 A-classified legacy variants into the canonical `intent_algebra::query_expr::QueryExpr`, plus the minimal typed `Predicate` IR for `Filter.pred` covering the 4 used-and-cleanly-shaped ScalarExpr variants. ## What landed ### Canonical `QueryExpr` grew from 5 → 15 variants Pre-existing (PR #130 / canonical): `Scan`, `Window`, `Aggregate`, `LetBinding`, `Ref`. New (this PR): `Filter`, `Project`, `Partition`, `Distinct`, `Merge`, `Join`, `SetOp`, `Sort`, `Limit`, `BinaryOp`. Naming convention: single-input variants use `child:` (matches existing canonical `Window`/`Aggregate`/`LetBinding`). Multi-input variants follow design.md §6 shapes exactly: `Merge { children }`, `Join { kind, pred, left, right }`, `SetOp { kind, all, left, right }`, `BinaryOp { op, lhs, rhs, vector_match }`. ### Supporting types lifted alongside `ColumnRef`, `PartitionKeys`, `BinaryOpKind`, `JoinKind`, `SetOpKind`, `SortKey`, `VectorMatch`/`VectorMatchKind`/`VectorGrouping`/`GroupSide`, `LiteralValue`, `ProjectItem`. All re-exported from `intent_algebra::mod.rs`. ### New typed `Predicate` ```rust pub enum Predicate { Column(ColumnRef), Literal(LiteralValue), BinaryOp { op: BinaryOpKind, lhs: Box<Predicate>, rhs: Box<Predicate> }, IsNull { expr: Box<Predicate>, negated: bool }, } ``` Plus `Predicate::from_legacy_scalar(&legacy::ScalarExpr) -> Result<Predicate, QueryExprError>`. Translates the 4 supported variants (Column / Literal / BinaryOp / IsNull); returns `QueryExprError::UnsupportedLegacyScalar(name)` for the 4 E-deferred (FunctionCall / ScalarSubquery / InList / Between). Per user decision, those stay in legacy until a real consumer demands the typed shape. Translation handles `LiteralValue::Duration → Int(nanos)` fold (canonical LiteralValue is deliberately narrower). ### Consumer migration deferred to subsequent batches **Zero consumer-site redirects in this PR** — and that's the right call. Every legacy-A-variant consumer (`query_parser/`, `physical/ {stage_split,planner,allocator}.rs`, `optimizer/engine.rs`, `legacy_lower.rs`) simultaneously matches A-variants AND C-variants (`Source`, `Aggregate`, `Window`, `SketchAgg`, `WindowedAgg`, `TopK`, `HistogramQuantile`, `PromQLSubquery`) AND constructs `ScalarExpr` predicates with E-deferred variants. Migrating them mid-pipeline would require lifting C-variants and E-variants in the same PR — explicit out-of-scope per the batch plan. The consumers migrate one C-batch at a time: - Batch 3a (PR #11): `AggFunc → AggIntent` - Batch 3b (PR #10): `TopK` split - Batch 3c (PR #8): `WindowedAgg` un-fusion - Batch 3d (PR #9): `SketchAgg → Aggregate@L3` - Batch 3e (PR #12): PromQL parser `histogram_quantile → Quantile` - Batch 13: retire `legacy_expr.rs` + `legacy_lower.rs` ### Catch-all arms added in 6 canonical-side consumers Files: `optimizer/cost/mod.rs`, `physical/colored_dag/{allocator,emitter}.rs`, `sketch_algebra/lower.rs`, `warm_tier_analysis.rs`. These previously matched canonical `QueryExpr` exhaustively over the 5 pre-existing variants; now they need conservative handling for the 10 new ones (Edge stage / Logical wrap / child-walk / zero-cost) with TODO markers for the follow-up reshape batches. ### Serde tag note Initially gave `ColumnRef`/`LiteralValue`/`PartitionKeys`/`Predicate` internally-tagged `#[serde(tag = "kind", rename_all = "snake_case")]` attrs, but serde rejects internally-tagged newtype variants containing primitives (e.g. `Named(String)`). Switched to externally-tagged `#[serde(rename_all = "snake_case")]`. Caught by the round-trip test. ## Build + test - `cargo build --release -p controller` — clean - `cargo build --release -p query_engine_rust` — clean - `cargo test -p controller --lib` — **666 passed** (was 655 after Batch 1; +11 from new typed-Predicate + schema tests) - `cargo test -p query_engine_rust --lib -- engines::warm_tier` — 13/13 ## Diff: 8 files, +794 / -8 Co-authored-by: zz_y <zz_y@node0.zz-y-304941.softmeasure-pg0.clemson.cloudlab.us> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
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.
PR C — completes the per-sketch-type decoder roll-out for the Phase 1 modified-OTLP sketch hot path (PR #5 + #6 + #7). With this PR, all five modified-OTLP
Metric.datasketch variants — CountMin, CountSketch, KLL, DDSketch, HLL — reach concrete accumulators end-to-end instead of falling through to the §5.2 fallback.Scope
CountMinSketchAccumulator(PR #6)count_min::CountMinSketchCountSketchAccumulatorcount_sketch::CountSketchDatasketchesKLLAccumulatorkll::KllSketchDDSketchAccumulatordd_sketch::DdSketchHllSketchAccumulatorhll_sketch::HllSketchWhat's new
sketch-core types
sketch_core::count_sketch::CountSketch— element-wise-mergeable signed-counter matrix.sketch_core::dd_sketch::DdSketch— log-bucketed quantile sketch; merge aligns bucket arrays on absolutestore_offsetand sums counts element-wise, combining{count, sum, min, max}aggregates.sketch_core::hll_sketch::HllSketch— register array + variant + precision + HIP accumulators; merge is register-wise max with HIP-component addition whenvariant == Hip.QE accumulators (
precompute_operators/)CountSketchAccumulator,DDSketchAccumulator,HllSketchAccumulator— each implements the fullAggregateCore+SerializableToSinksurface and afrom_sketchlib_proto_bytesconstructor that decodes the correspondingasap_sketchlib::proto::sketchlib::*Stateproto emitted by DataCollector's per-sketch processors.DatasketchesKLLAccumulator::from_sketchlib_proto_bytes— decodesKllStatevia lossy statistical reconstruction (replaysitems[]throughupdate()on a fresh sketch with the samek, since sketch-core's KLL backend types keep their level structure private). Quantile estimates remain approximately equivalent to the source's — within KLL's own rank-error bound, which the source already inherited — so queries hitting the reconstructed sketch return answers the user would already have accepted from the source. Bit-identical reconstruction is tracked as a sketchlib upstream follow-up.Dispatcher (
drivers/ingest/otel.rs)decode_modified_otlp_sketch_bytesnow dispatches all fiveSketchKindvariants to their concrete decoders. Previously onlyCountMinwas wired (PR feat: route modified-OTLP sketch metrics through precompute engine (PR B) #6);CountSketch,Kll,DdSketch, andHllused to return anot yet implementederror and fall through to §5.2.Enums
AggregationType::CountSketch(was already added in the earlier CountSketch commit).AggregationType::DDSketch— new variant, wired throughDisplay/FromStr.Deferred
Query semantics for the new accumulators — cardinality estimation for HLL, quantile estimation on the log-bucket representation for DDSketch, median-of-estimators heavy-hitter tracking for CountSketch — are intentionally deferred to a follow-up.
query_statisticon these accumulators returns a placeholdernot yet implementederror that the caller falls through to the §5.2 fallback with, so queries still return correct answers while the matrix/register/bucket merge + store round-trip works end-to-end today.Tests
Unit tests
sketch-core::count_sketch— 6 passedsketch-core::dd_sketch— 6 passed (aligned/overlapping/disjoint merge, alpha mismatch, msgpack round-trip)sketch-core::hll_sketch— 6 passed (register-wise max, variant/precision mismatch,merge_refs, msgpack round-trip)CountSketchAccumulator— 6 passed (int64 / float64 / dim mismatch / zero dims / merge / wrong-type rejection)DatasketchesKLLAccumulator::from_sketchlib_proto_bytes— 3 passed (reconstructed quantile, small-k rejection, inconsistent levels rejection)DDSketchAccumulator— 4 passed (round-trip, invalid-alpha rejection, bucket alignment merge, wrong-type rejection)HllSketchAccumulator— 6 passed (regular/HIP round-trip, register-length mismatch, zero-precision rejection, register max merge, wrong-type rejection)E2E tests (
tests/e2e_modified_otlp_sketch_path.rs)All five tests POST a real
ExportMetricsServiceRequestthrough the OTLP HTTP receiver, route it through a runningPrecomputeEngine, close a window, and assert the captured accumulator's inner state matches the payload:e2e_count_min_sketch_modified_otlp_path(from PR test: e2e test for modified-OTLP CountMin sketch hot path (PR D) #7)e2e_count_sketch_modified_otlp_pathe2e_kll_sketch_modified_otlp_path— verifies reconstructed median of [1..=100] is within KLL's rank-error bound of 50e2e_dd_sketch_modified_otlp_path— verifies bucket counts, offset, count, sum, alpha all round-tripe2e_hll_sketch_modified_otlp_path— verifies register bytes + precision round-tripValidation
cargo check --all-targets: cleancargo clippy --all-targets -- -D warnings: cleancargo fmt --check: cleancargo test -p sketch-core --lib: 54 passedcargo test -p query_engine_rust --lib: 458 passedcargo test -p query_engine_rust --test e2e_modified_otlp_sketch_path: 5 passed (all variants hot end-to-end, ~6s serial)🤖 Generated with Claude Code