feat(sid): introduce PolicyFingerprint + PolicyRegistry (dual-keyed) - #196
Merged
Merged
Conversation
PR 3 of the merged-sid-identity chain. Adds the content-addressed policy identity that will eventually replace `aggregation_id`. ## What - New `asap_types::policy_fingerprint::PolicyFingerprint(u64)` newtype. Derived deterministically from `AggregationConfig` content via xxh64 over a canonicalized byte layout. Two control planes producing the same policy independently produce the same fingerprint. - New `asap_types::policy_registry::PolicyRegistry` — content-addressed lookup table `PolicyFingerprint → AggregationConfig`. Built from a list of configs or directly from a `StreamingConfig`. - New `StreamingConfig::policy_registry()` derivation method. Builds the registry on demand from the existing `aggregation_configs` map. Both views share a single source — they can never disagree. ## Identity contract `PolicyFingerprint = h(metric, agg_type, sub_type, parameters, grouping_labels, aggregated_labels, rollup_labels, window_size, slide_interval, window_type, spatial_filter_normalized)` Hash inputs are sorted, JSON-rendered, separator-delimited so map iteration order can't affect the result. Fields explicitly excluded: - `aggregation_id` itself (the thing being replaced). - `original_yaml` (serialization artifact). - `num_aggregates_to_retain` (retention policy, separate concern). - SQL-mode `table_name` / `value_column` (folded into `metric` upstream for time-series mode). ## Dual-keyed PR 3 doesn't touch any caller. `aggregation_configs: HashMap<u64, AggregationConfig>` continues to be the source of truth; the registry is a derived view that callers can opt into. Subsequent PRs migrate callers one at a time: - PR 4: `PrecomputedOutput.aggregation_id` → `policy_fp`; `SketchStoreSink` and friends use `PolicyRegistry::get(fp)` instead of `StreamingConfig::get_aggregation_config(id)`. - PR 5: delete `aggregation_id` from `AggregationConfig` and the YAML schema; control plane stops minting u64 ids. ## Test coverage - Same config → same fingerprint (idempotence). - Different metric / window / spatial_filter / group_by → different fingerprint. - `aggregation_id` and `num_aggregates_to_retain` don't affect the fingerprint (incidental fields). - HashMap iteration order doesn't affect the fingerprint (parameters are sorted before hashing). - Spatial-filter canonicalization (matchers sorted by key) drives the fingerprint — same predicate written in different orders hashes the same. - Registry: round-trip lookup; duplicate-fingerprint collapsing with collision count; empty source → empty registry. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
zzylol
added a commit
that referenced
this pull request
May 14, 2026
PR 6 follow-up. PR 6 (folded into #196) landed `Capability::ExactAgg( AggregationType)` as a dormant variant — wired into `is_satisfied_by` but not produced by `capability_for`, so the analyzer never emitted candidates targeting it. This PR makes the wiring live. ## Two changes 1. **Data plane** (`storage_engines/sketch_db/index/mod.rs`): `ingest_precompute_for_agg_config` now registers `SketchInstanceMetadata.capability = Some(Capability::ExactAgg( agg_cfg.aggregation_type))` for fresh ExactAgg-backed sids. Pre-PR this field was unconditionally `None`, so capability-matching couldn't see warm-tier ExactAgg state. The legacy precompute query path (`SketchStore::query_precomputes_by_agg`) keeps working in parallel — no behaviour change for that read path. 2. **Control plane** (`sketch_algebra/capability.rs::capability_for`): Four intents flipped from `None` → `Some(Capability::ExactAgg(...))`: - `AggIntent::Sum` → `ExactAgg(Sum)` - `AggIntent::Rate { .. }` → `ExactAgg(Increase)` (rate = increase/time) - `AggIntent::Increase { .. }` → `ExactAgg(Increase)` - `AggIntent::Count { Exact }` → `ExactAgg(Sum)` (count_over_time = sum-of-1s) These were previously routed to the archive engine because warm-tier sketches don't cover exact aggregations. With ExactAgg state addressable at the analyzer, the warm-tier path becomes the primary route; archive remains the fallback on capability-miss. ## What stays on archive - `AggIntent::Avg` — Avg = Sum/Count, which requires joining two separate ExactAgg policies at query time. The L4 binder doesn't emit that pattern yet; routing Avg to ExactAgg today would surface capability-misses (no single sid satisfies it). Tracked as a future follow-up. - `AggIntent::Quantile { Exact }`, `Cardinality { Exact }`, `TopK { Exact }`, `Frequency { Exact }` — exact-accuracy variants of approximate-by-default intents. There's no exact-precompute shape for these in the AggregationType vocabulary; they need HashAgg / SortAgg / SortMerge, none of which run at the warm tier. - Archive-only intents (Absent / Present / Delta / Deriv / PredictLinear / HoltWinters / Idelta / Irate / Resets / Changes). PromQL-specific operations with no aggregation semantics worth precomputing at the warm tier. ## Test plan - [x] `cargo check --workspace` clean - [x] `cargo test --workspace --lib --bins` — green (the pre-existing `avg_finds_sum_and_count` HashMap-iteration flake is unchanged; retries succeed) - [x] Updated existing assertion `capability_for_sum_returns_none`, `..count_exact_returns_none`, `..rate_increase_return_none`, and the explicit dormancy test to lock in the flipped behavior. ## What's still owed - L4 binder rules that emit `PhysicalExpr::ExactAgg` nodes — the variant exists on the L4 algebra side, but no rule currently produces it. Until those rules land, the analyzer's flip is observable only through `WarmTierAnalysis::candidates` (the candidate is generated with the right `required_capability`) — the optimizer's plan output doesn't change yet. - Retire `PrecomputedOutput.aggregation_id` (PR 5 left this field for legacy compat; with all production reads on `policy_fp`, it's a one-line deletion). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <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
PR 3 of the merged-sid-identity chain. Adds content-addressed policy identity (
PolicyFingerprint+PolicyRegistry) alongside the existingaggregation_id-keyed map. Zero call-site migration — this is pure infrastructure for later PRs.What's new
PolicyFingerprint(u64)— newtype, content-hashed over the full policy (metric, agg_type, sub_type, parameters, group_by/aggregated/rollup labels, window cadence, spatial_filter_normalized). xxh64 keyed at 0 for portability across hosts.PolicyRegistry—HashMap<PolicyFingerprint, AggregationConfig>lookup table built fromStreamingConfig.StreamingConfig::policy_registry()— derives the registry on demand.Why content-addressed
Dual-keyed, not migration
aggregation_idremains the source of truth in this PR. The registry is a derived view. Callers can opt into either path. PR 4 migratesPrecomputedOutput+ sinks; PR 5 deletesaggregation_id.Test plan
cargo check --workspaceclean.cargo test -p asap_types --lib— 62 passed.🤖 Generated with Claude Code