feat(sid): add policy_fp back-reference to SketchInstanceMetadata + reverse index - #203
Merged
Merged
Conversation
…everse index Closes the O(1) "which sids belong to this policy?" lookup gap that the merged-sid-identity chain left open. PR 6 follow-up #200 populated `SketchInstanceMetadata.capability` with `Some(Capability::ExactAgg(agg_type))`; this PR completes the direct-index story by adding the fingerprint back-reference and a matching `policy_fp → {sids}` index on `SketchStore`. ## What - New field `SketchInstanceMetadata.policy_fp: PolicyFingerprint`. Populated from `output.policy_fp` at registration time in `SketchStore::ingest_precompute_for_agg_config`. Other construction sites (OTel sketch ingest, tests, benches) populate `PolicyFingerprint::UNSET` — they don't carry a source `AggregationConfig` and the reverse index intentionally skips them. - New field `SketchStore.policy_to_sids: RwLock<HashMap<PolicyFingerprint, BTreeSet<u64>>>`. Maintained automatically by `register` / `remove_instance` / `remove_instances_for_agg_config`. UNSET entries are not recorded. - New public methods: - `SketchStore::sids_for_policy(fp) -> Vec<u64>` — O(1) lookup, sorted return order (BTreeSet under the hood). - `SketchStore::policy_count() -> usize` — telemetry. ## Why this matters Before this PR the query path resolved `Candidate → [sids]` by walking `SketchStore::instances_matching(metric, gbk)` and re-deriving the policy fingerprint per sid. With the index landed, the analyzer can go straight from `Candidate { metric, group_by, capability, window } → policy_fp` (via `PolicyRegistry::find_matching`, future PR) and `policy_fp → [sids]` (this PR) in two O(1) hops. ## What this PR explicitly does NOT change - OTel sketch ingest (`drivers/ingest/otel.rs`) still registers with `policy_fp: UNSET`. Sketches arrive with their shape (kind + config) embedded in the OTLP DP, not a policy reference. Deriving the fp at ingest by content-matching against `PolicyRegistry` is a natural follow-up but out of scope here. - `PolicyRegistry::find_matching` (analyzer-side candidate → policy resolution) doesn't exist yet. The plumbing landed (registry + reverse index); the matching function is the missing piece for the full O(1) query lookup. ## Test plan - [x] 6 new unit tests on the reverse index: - `sids_for_policy_returns_empty_for_unset_or_missing` - `register_indexes_one_sid_under_its_policy` - `register_groups_multiple_sids_under_one_policy` - `register_separates_distinct_policies` - `unset_policy_sids_are_not_in_reverse_index` - `remove_instance_drops_reverse_index_entry` - [x] `cargo check --workspace` clean - [x] `cargo test --workspace --lib --bins` green (one pre-existing `avg_finds_sum_and_count` HashMap-iteration flake unchanged; passes on retry) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
zzylol
added a commit
that referenced
this pull request
May 14, 2026
…p lookup (#204) Closes the analyzer-side half of the merged-sid-identity query-path index. PR #203 added `SketchStore::sids_for_policy(fp) → [sid]`; this PR adds `find_matching_policies(registry, candidate) → [fp]` so the query engine can do `Candidate → [fp] → [sid]` in two O(1)-amortized hops instead of walking the sid metadata map. ## What - New `control_plane::warm_tier_analysis::policy_capability(cfg)` — maps `AggregationConfig` to the warm-tier `Capability` its sids serve. Inverse of `capability_for(&AggIntent)`: where the latter says "this intent wants *this* capability", this says "this stored policy *provides* this capability". - New `control_plane::warm_tier_analysis::find_matching_policies(registry, candidate)` — walks the policy registry, returns every fingerprint whose policy satisfies the candidate. Predicate (all must hold): 1. `policy.metric == candidate.metric_name` 2. `candidate.group_by_keys ⊆ policy.grouping_labels` (extra policy keys are fine; query can re-aggregate down) 3. `policy_capability(policy)` exists and is satisfied by `candidate.required_capability` (uses the existing `Capability::is_satisfied_by` semantics) 4. `policy.window_size ≤ candidate.range_seconds` (finer windows answer coarser queries via merge; reverse isn't true). `range_seconds == 0` (instant-vector) bypasses this check. 5. `policy.spatial_filter_normalized.is_empty()` — only unfiltered policies for now (candidate doesn't carry a filter shape; filtered match is a future enhancement) - New dep: `control_plane → asap_types` (`PolicyRegistry`, `PolicyFingerprint`, `AggregationConfig`). Added to `Cargo.toml`. ## Mapping table — `policy_capability(cfg)` | `AggregationType` | `Capability` | |---|---| | `Sum` | `ExactAgg(Sum)` | | `Increase` | `ExactAgg(Increase)` | | `MinMax` | `ExactAgg(MinMax)` | | `DDSketch` | `QuantileApprox(DDSketch)` | | `DatasketchesKLL` | `QuantileApprox(Kll)` | | `HLL` | `CardinalityApprox` | | `CountMinSketch` | `FrequencyEstimate(CountMin)` | | `CountSketch` | `FrequencyEstimate(CountSketch)` | | `CountMinSketchWithHeap` | `FrequencyTopk(CmsWithHeap)` | | multi-pop variants (`Multiple*`, `HydraKLL`), legacy wrappers, set-aggregators | `None` (skipped) | The multi-pop variants stay `None` until the keyed-ExactAgg follow-up lands an L4 binder for them. Adding a `Capability::ExactAgg(MultipleSum)` arm here today would surface candidates the planner can't route. ## End state of the query-path index ``` PromQL → analyzer → WarmTierCandidate │ ▼ find_matching_policies(registry, &candidate) │ ← O(N_policies); typically ~thousands, not millions ▼ Vec<PolicyFingerprint> │ ▼ SketchStore::sids_for_policy(fp) │ ← O(1) per fp; PR #203 ▼ Vec<sid> │ ▼ SketchStore::range_query + SketchReducer::evaluate ``` ## What's not in scope - Candidate-side spatial-filter shape. Today filtered policies are unconditionally skipped; the candidate has no filter to compare against. When the analyzer surfaces a filter, this predicate gains a `candidate.spatial_filter_normalized == policy.spatial_filter_normalized` arm. - Wire-up to the query engine. `ASAPQueryEngine.execute` still walks `instances_matching(metric, gbk)` today; this PR is the lookup primitive, not the integration. A follow-up swaps the call. ## Test plan - [x] 14 new unit tests covering each match / mismatch dimension - [x] `cargo check --workspace` clean - [x] `cargo test -p control_plane --lib warm_tier_analysis::tests::matching` — 15 passed - [x] `cargo test --workspace --lib --bins` green except for the pre-existing `avg_finds_sum_and_count` HashMap-iteration flake 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
zzylol
added a commit
that referenced
this pull request
May 14, 2026
…205) Wires up the OTel sketch-ingest path to populate `SketchInstanceMetadata.policy_fp` instead of leaving it `UNSET`. Sketch-backed sids now participate in the `policy_fp → {sids}` reverse index (PR #203), so the analyzer's `find_matching_policies` (PR #204) returns fingerprints whose sids are O(1)-reachable. ## What - New `control_plane::warm_tier_analysis::find_policy_by_content( registry, metric, group_by_keys, agg_type, expected_params)` — finds the policy whose contents match a freshly-ingested sketch's shape. Returns `Some(fp)` on a unique match, `None` on zero or multiple matches (ambiguous → stay UNSET). - Two data-plane helpers in `data_plane/src/drivers/ingest/otel.rs`: - `aggregation_type_for_sketch_handle(SketchKindHandle) → Option<AggregationType>` - `sketch_config_to_params(&SketchConfig) → HashMap<String, Value>` Both lock in the data-plane → control-plane wire-shape mapping so drift surfaces as test failures, not silent lookup misses. - New `derive_sketch_policy_fp(ingest_state, metric, kind, cfg, group_by_keys)` threads the content match. Called from the OTel sketch ingest registration site (`route_modified_otlp_sketches_to_precompute`). Returns `PolicyFingerprint::UNSET` when no policy matches — sids stay reachable via the legacy `instances_matching(metric, gbk)` walk. ## Matching shape Match requires all of: 1. `policy.metric == metric` 2. `policy.aggregation_type == agg_type` (mapped from `SketchKindHandle`) 3. `policy.grouping_labels` (as a set) == `group_by_keys` 4. For every key in `expected_params`, `policy.parameters` has the same `serde_json::Value` (extra policy params tolerated) 5. `policy.spatial_filter_normalized.is_empty()` — OTLP sketches don't carry a filter context Ambiguous match (multiple policies → same shape) returns `None` intentionally. Such policies would have collided on sid identity anyway — surfacing as UNSET is the honest signal of a control-plane bug. ## Param-key vocabulary | `SketchConfig` | params key(s) | |---|---| | `DDSketch { relative_accuracy }` | `relative_accuracy` | | `Kll { k }` | `k` | | `Hll { precision }` | `precision` | | `CountSketch { rows, cols }` | `rows`, `cols` | | `CountMin { rows, cols }` | `rows`, `cols` | Names must stay in sync with `AggregationConfig::from_yaml_data` in `asap_types/src/aggregation_config.rs`. The new test `sketch_config_to_params_uses_canonical_keys` locks them. ## What's still standing - OTel sketch-ingest doesn't surface a spatial-filter context, so filtered policies remain unreachable from this path. When the agent's processor emits a filter shape in the OTLP DP, plumb it through to the lookup. - Keyed-group ExactAgg (`MultipleSum` / `MultipleIncrease`) policies don't appear in `policy_capability` either; if/when those land an L4 binder, both this lookup and `find_matching_policies` need matching arms. ## Test plan - [x] 2 new unit tests in `otel::policy_fp_lookup_tests` covering the `SketchKindHandle → AggregationType` mapping and the `SketchConfig → params` rendering - [x] `cargo check --workspace` clean - [x] `cargo test --workspace --lib --bins` green (pre-existing `avg_finds_sum_and_count` HashMap-iteration flake unchanged) 🤖 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
…ryEngine.execute (#206) Switches the query engine's candidate → sid resolution to the content-addressed fast path. PRs #203, #204, #205 landed the primitives; this PR integrates them. ## Before vs. after Before, per candidate: 1. `idx.instances_matching(metric, gbk)` walks the `RwLock<HashMap<u64, SketchInstanceMetadata>>` and returns every sid whose metric + group-by-keys match. 2. Per-sid: classify, check capability satisfaction, push to `hit_sids`. After, per candidate: 1. Snapshot the streaming config; build the `PolicyRegistry`. 2. `find_matching_policies(registry, candidate)` — walks the small policy registry (≤ thousands of entries) with the full match predicate (metric + group_by + capability + window + filter). Returns `Vec<PolicyFingerprint>`. 3. For each fp: `idx.sids_for_policy(fp)` — O(1) hash lookup over the reverse index added in PR #203. 4. Same per-sid classify + capability check as before (defensive; fast-path-discovered sids already satisfy the candidate by construction, but UNSET sids reached via the fallback don't). ## Slow-path fallback retained When the fast path yields zero sids — because either: - no policy in the registry matches the candidate (control plane hasn't published one yet), OR - the candidate's sids were registered with `PolicyFingerprint::UNSET` (legacy paths that didn't carry an `AggregationConfig` at ingest, test fixtures, raw mode) — the engine falls back to the metadata walk `instances_matching(metric, gbk)`. The per-sid capability filter below catches mismatches the fast path would have rejected at policy-match time. The fallback can be deleted in a follow-up once every code path populates `policy_fp` and existing on-disk records have aged out. ## Snapshot semantics The streaming-config snapshot is pinned once per query (not per candidate). Hot-reload swaps the underlying `Arc<StreamingConfig>` mid-query are isolated by the snapshot: the query sees the policy set that was active at query start. Same isolation the legacy `streaming_config_snapshot()` call already gave the engine elsewhere. ## Test plan - [x] `cargo check --workspace` clean - [x] `cargo test --workspace --lib --bins` green (`asap_types::capability_matching::tests::avg_finds_sum_and_count` HashMap-iteration flake unchanged) - Existing query-engine tests cover the slow-path fallback (their fixtures register sids with UNSET fp); future tests on the fast path land alongside an end-to-end test that builds a streaming config with matching policies and verifies the fp-keyed lookup is used. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 14, 2026
…ty-matching flake (#208) Two related cleanups bundled because both touch deterministic sid lookup and they're each small. ## 1. Drop `instances_matching` fallback in ASAPQueryEngine.execute PR #206 wired the content-addressed fast path with a fallback to the legacy `idx.instances_matching(metric, gbk)` walk for sids registered with `PolicyFingerprint::UNSET`. With PRs #203 + #205 populating the fp on every production registration path, the fallback's only consumers are test fixtures and the raw-mode fast-path that the sink already drops. Removing it makes "capability miss" mean exactly one thing — no policy in the registry satisfies the candidate — instead of overloading the miss path between "no policy" and "no sid metadata". ## 2. Make `aggregation_priority` a total order `capability_matching::tests::avg_finds_sum_and_count` had been flaky because `aggregation_priority` returned `Equal` on equal `window_size`, leaving `Vec::sort_by` order dependent on the underlying `HashMap` iteration. The test inserts both a `Sum` and a `CountMinSketch` config for the same metric; `Statistic::Sum` matches both, and when the multi-pop `CountMinSketch` sorted first the downstream key-aggregation lookup (only needed for multi-pop value types) missed and the whole match returned `None`. Sort keys now: 1. Larger `window_size` (coarser windows answer finer-grained queries via re-aggregation). 2. Single-population types beat multi-population (avoids the key-aggregation hunt when both shapes serve the statistic). 3. `aggregation_id()` (the policy fingerprint u64) tie-break — deterministic across runs and hosts. The result for the failing test: `Sum` always wins the Sum-stat candidate, no key-aggregation lookup fires, the function returns `Some(...)` deterministically. ## Engine test update `execute_returns_capability_miss_when_classify_is_ghost` previously asserted the detail string contained "ghost" / "unknown". With the fallback removed, the engine short-circuits at the policy-resolution step (the test fixture's `dd_meta` helper registers with `PolicyFingerprint::UNSET`, so the policy lookup never finds the sid). The `CapabilityMiss` outcome is preserved; the detail-string assertion is dropped since it pinned implementation, not contract. ## Test plan - [x] `cargo check --workspace` clean - [x] `cargo test --workspace --lib --bins` green - [x] `avg_finds_sum_and_count` ran 5× in a row, all pass — flake gone 🤖 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
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
Closes the O(1) "which sids belong to this policy?" lookup gap. PR 6 follow-up #200 populated
SketchInstanceMetadata.capabilitywithSome(Capability::ExactAgg(agg_type)); this PR completes the direct-index story by adding the fingerprint back-reference and a matchingpolicy_fp → {sids}index onSketchStore.What
SketchInstanceMetadata.policy_fp: PolicyFingerprint.SketchStore.policy_to_sidsreverse index, maintained automatically byregister/remove_instance.SketchStore::sids_for_policy(fp) -> Vec<u64>(O(1), sorted) andSketchStore::policy_count() -> usize.Why
Before: query path walked
instances_matching(metric, gbk)and re-derived policy fp per sid. After:Candidate → policy_fp(existingPolicyRegistry) →policy_fp → [sids](this PR), two O(1) hops.Not in scope
policy_fp: UNSET(sketches arrive with shape embedded in OTLP DP, not a policy reference). Deriving the fp at sketch-ingest by content-matching againstPolicyRegistryis a natural follow-up.PolicyRegistry::find_matching(analyzer-sideCandidate → policy_fpresolution) isn't here yet; that's the other half of the full O(1) story.Test plan
cargo check --workspacecleancargo test --workspace --lib --binsgreen🤖 Generated with Claude Code