feat(sketch_db): Phase 5 M2 — derive sketch sid from content hash - #150
Merged
Merged
Conversation
The backend now computes `series_id` for a sketch DataPoint as xxh64(metric_name, attrs_fingerprint, sketch_kind, sketch_config) instead of minting a monotonic counter value through SeriesIdResolver. Same four inputs → same sid across restarts and across hosts, so the controller no longer needs to emit `aggregationId` for each (metric, agg-type, params) tuple — backend derives it. `SketchIndex.instance(sid).is_some()` replaces `SeriesIdResolver::is_known(sid)` as the "have we seen this sid before?" check for the (sid!=0, no-attrs) wire case; the (sid!=0, attrs) case recomputes the hash and signals stale on disagreement. Raw-sample ResolveSeriesIDs RPC and non-sketch paths keep the existing counter-based resolver (no agent-side coordination change needed yet). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
zzylol
added a commit
that referenced
this pull request
Jul 27, 2026
…eal Cms sketch
Bumps the ASAPController rev pin to feat/extension-realization-hook
(ASAPController#162, not yet merged) to pick up
CostModel::{realize_extension, readout_extension} (ASAPController#150).
ControlPlaneCostModel now implements both for ext_kind == "frequency":
realize_extension parses the accuracy out of payload and returns
Implementation::Sketch{kind: Cms, params: Cms{width,depth}} -- matching
capability_matching::pick_family's own Frequency -> Cms mapping (NOT the
older sketch_catalog::sketch_type_for_op's SketchType::CountSketch,
which is a genuinely different sketch algorithm under a similar name and
predates/disagrees with capability_matching -- kept the newer, tested
mapping). readout_extension builds SketchQuery::PointCount, falling back
to {key: SampleValue, value: None} until the filter value is threaded
through payload (a separate, later change) -- correct as the bare
bucket total in the meantime.
ForcedFamilyCostModel (used by optimizer::rules::bind_workload_typed)
delegates both new methods to its inner ControlPlaneCostModel, so the
`endpoint_request_freq` contract row -- previously a documented, accepted
regression declining to Logical pending #150 -- now commits like every
other row. Flipped the tests that pinned the old decline behavior to
assert the new Cms binding instead.
Also updates emit::stage_config's SketchQuery::PointCount wire JSON for
the new `value` field (ASAPController#162).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Jul 27, 2026
…ding Caught in review: the section header above frequency_extension_binds_cms still described Frequency as declining to bind (pre-#150 behavior) and pointed at the old, renamed test name. Corrected to describe current behavior. 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
1 participant
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
compute_sketch_sid(metric_name, attrs_fingerprint, sketch_kind, sketch_config) -> u64indata_plane::stores::sketch_db::index, a deterministic xxh64 over the four DataPoint inputs the backend has at sketch-ingest time.otel.rsto compute the sid by hash instead of going throughSeriesIdResolver. Same four inputs always yield the same sid across restarts and across hosts; the controller no longer needs to emitaggregationId.SketchIndex.instance(sid).is_some()replacesSeriesIdResolver::is_known(sid)as the "have we seen this sid before?" check for the(sid!=0, no-attrs)wire case.(sid!=0, attrs)recomputes the hash and signals stale on disagreement.(sid=0, attrs)always derives the hash.(sid=0, no-attrs)still drops.ResolveSeriesIDsRPC and non-sketch paths keep the existing counter-based resolver — no agent-side coordination change needed for this PR.Scope this PR does NOT cover (follow-ups)
AggregationConfig.aggregation_idfield removal from YAML wire format.SchemaRegistry/SketchStoreagg_id → sid migration (M2 finish).Arc<dyn AggregateCore>retirement (M3).Test plan
sketch_db::index::testscover determinism + distinguishing each of the 4 hash inputs (metric, attrs values, sketch_kind, container_config).sid_resolution_testsupdated to assert onSketchIndexregistration state instead of resolver state.cargo test -p data_plane --lib— 792 passed; 2 failed (pre-existingschema_timeline_dispatch_testsregressions, unrelated to this change); 6 ignored.cargo check --workspaceclean.🤖 Generated with Claude Code