feat(summary_executor): add coverage tracking and ExactAgg support - #417
Merged
Merged
Conversation
Two additions to data_plane's SummaryExecutor, per the "one sid = one
aggregation, read out directly" design direction (no exact-vs-approximate
special-casing, no outer-agg-fold):
Coverage: `SummaryValue::{Points,TopK}` now carry a trailing
`coverage: Option<(u64, u64)>` -- this group's own (min_window_end_ms,
max_window_end_ms) observed, computed the same way
ASAPTierResult.coverage actually is today (folded from raw window-END
timestamps, including a carry-in base's window-end even though it never
surfaces as an output point -- see sketch_reducer.rs's own
evaluate_core/evaluate_cardinality_global). This is data_plane-local;
nothing outside this file depends on it yet.
ExactAgg: find_candidates/fetch_state/merge_states now also recognize
AggKind::ExactAgg sids for SummaryKind::{Sum, Increase} (SidHandle/
GroupState become enums with a parallel ExactAgg variant wrapping
Arc<dyn AggregateCore> per window, reusing query_exact_agg_range's
existing decode/merge machinery rather than reimplementing it).
SummaryKind::MinMax/Count/Rate are deliberately NOT matched: MinMax's
direction doesn't survive onto AggKind::ExactAgg's metadata, and no
AggregationType resolves to Count/Rate today -- matching them would
force a later caller to guess. readout()/SketchQuery still never see
ExactAgg states (asap_plan::bind never wraps an ExactAccumulator in a
SummaryEstimate, so execute() stops at ExecOutcome::State first); the
new GroupState::exact_value(&self, key) is the entry point a future
live-serving caller will use to read that State's contents directly.
Both are pure additions to this file -- no change to what's actually
wired into live serving (engine.rs still calls SketchReducer directly).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Jul 27, 2026
…t plan Rebased onto current main (was 27 commits behind, predating #416/#417/#418) and updated the doc's content to match: - Status note: SummaryExecutor now covers the Frequency family and ExactAgg(Sum/Increase) candidate matching + coverage tracking, not just quantile/cardinality -- these landed since the doc was last synced. - "Today"/gap list: gap 2 (merge only existed for ExactAgg) and gap 3 (nothing checked param agreement) are now resolved on the new path, marked accordingly rather than left as open problems. - SidHandle/GroupState table entries: updated to describe the actual enum shape (Sketch/ExactAgg variants), not the original sketch-only struct design. - New "Architecture reference" section citing ASAPController design.md's "Serving-time execution" section directly -- the planning-vs-serving split this doc's Rollout section builds on. - "Rollout" section: replaces "still open" with an actual plan. Corrects a real error in the previous version -- it named `implement_promql_for_asap_tier` as "the seam" for tree construction, but that function uses the naive DefaultCostModel and has a documented, tracked gap where it can't realize the Frequency intent at all. The correct seam is `sketch_algebra::lower::bind_query_expr` (ControlPlaneCostModel, what main.rs's real production pipeline uses). Also documents why rate()/topk-over-rate/outer-agg-fold must be excluded from the shadow comparison entirely (not just deprioritized) and why shadow mode -- not a cutover -- is this phase's actual scope. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
Per the "one sid = one aggregation, read out directly" direction (no exact-vs-approximate special-casing, no outer-agg-fold stacking):
SummaryValue::{Points,TopK}now carry a trailingcoverage: Option<(u64, u64)>— this group's own(min_window_end_ms, max_window_end_ms)observed, computed the same wayASAPTierResult.coverageactually is today (folded from raw window-END timestamps — confirmed the legacy code, despite its doc saying "min_window_start_ms", never actually tracks a window start; seesketch_reducer.rs'sevaluate_core/evaluate_cardinality_global). Includes a carry-in base's window-end even though it never surfaces as an output point. Data_plane-local; nothing outside this file consumes it yet.find_candidates/fetch_state/merge_statesnow also recognizeAggKind::ExactAggsids forSummaryKind::{Sum, Increase}.SidHandle/GroupStatebecome enums with a parallelExactAggvariant wrappingArc<dyn AggregateCore>per window, reusingquery_exact_agg_range's existing decode/merge machinery rather than reimplementing it.SummaryKind::MinMax/Count/Rateare deliberately not matched —MinMax's direction doesn't survive ontoAggKind::ExactAgg's metadata, and noAggregationTyperesolves toCount/Ratetoday, so matching them would force a later caller to guess.readout()/SketchQuerystill never seeExactAggstates (execute()stops atExecOutcome::Statefirst for these); the newGroupState::exact_value(&self, key)is the entry point a future live-serving caller uses to read thatState's contents directly.Both are pure additions — no change to what's wired into live serving (
engine.rsstill callsSketchReducerdirectly).Test plan
cargo build --workspace— cleancargo test -p data_plane --lib— 917 passed (0 failed); 21/21 insummary_executorincluding 6 new tests (2 coverage, 1 genuine carry-in-splice coverage test, 3 ExactAgg)cargo test -p data_plane --test e2e_controller_plans_and_backend_serves— 9/11 pass; the 2 known pre-existing failures reproduce identically on origin/main, unrelated to this changecargo clippy -p data_plane --lib— 75 warnings, identical count to baseline, none in the touched filerustfmton the touched file🤖 Generated with Claude Code