Skip to content

feat: wire SeriesIdResolver + SketchIndex into OTLP receive + query (Phase 4+5+6 finish) - #122

Merged
zzylol merged 1 commit into
mainfrom
feat/phase4-5-6-wire-in
May 10, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/phase4-5-6-wire-in

Conversation

@zzylol

@zzylol zzylol commented May 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Lights up the centralized sid resolver and the new SketchIndex on the live ingest+query path. Previous PRs in this chain:

This PR is the wire-in. Together with #120 + #121 + ProjectASAP/ASAPCollector#372, it completes the centralized-sid-namespace refactor laid out in docs/design-controller-into-backend.md.

What landed

Phase 4 finish

  • OTLP receive sid-resolution gate in route_modified_otlp_sketches_to_precompute. For each sketch DataPoint:
    • (sid=0, attrs present) → resolver mints fresh sid via compute-or-mint cache.
    • (sid != 0, attrs present) → trust attrs; backend-resolved value wins. Mismatch → push old sid to unknown_series_ids for sender-side eviction.
    • (sid != 0, attrs empty)resolver.is_known(sid) lookup; miss → push to unknown_series_ids, drop DP (sender will resend with attrs on next push).
  • route_modified_otlp_sketches_to_precompute now returns Vec<u64> of unknown sids; both MetricsServiceImpl::export and handle_otlp_http stamp it into ExportMetricsServiceResponse.unknown_series_ids (the field already existed on the wire from Phase 2 as a Vec::new() placeholder).
  • New ResolveSeriesIDs gRPC service in metrics_service.proto: bulk pre-resolution endpoint for senders that have an attribute-only batch ready before the next Export. Reuses the existing SeriesAssignment shape; idempotent end-to-end.

Phase 5 finish

  • IngestState carries Arc<SeriesIdResolver> + Arc<SketchIndex>. Both allocated once in main.rs and threaded through PrecomputeEngine::new so the OTLP receiver, the precompute engine, and SimpleEngine all share the same instances.
  • OTLP ingest writes: after sid resolution, on first-seen sid the receiver materializes SketchInstanceMetadata (capability inferred from sketch kind: DDSketch/KLLQuantileApprox, HLLCardinalityApprox, CountSketch/CountMinFrequencyTopk; sketch_config from the parent-container fields lifted in Phase 2; accuracy via AccuracyBound::from_config) and registers it. Per-DP: append_sample(sid, label_values, (start_ms, end_ms), SketchSampleState { bytes, encoding }).
  • ModifiedOtlpSketchDp extended with series_id, start_time_unix_nano, and container_config: SketchConfig. The five typed-DP match arms thread these from the OTLP wire shape.
  • Helpers: sketch_kind_handle_for(&dp) and encoding_to_handle(i32) -> SketchEncoding.
  • Query path warm-tier read in SimpleEngine::execute: parses PromQL → extracts metric name + label-matcher KEY set → calls SketchIndex::instances_matching → classifies each candidate sid:
    • All Hit → falls through to handle_query (the per-Capability sketch reducer over query_range is a follow-up; today the all-Hit case still flows the legacy SimpleMapStore path).
    • Any Ghost or Unknown → returns EngineError::CapabilityMiss(SketchWarmTier, …) to trigger archive failover.
    • Empty match → likewise CapabilityMiss.
  • New SketchIndex::instances_matching(metric, required_keys) helper (~15 lines): filters instances where metric_name matches and required_keys is a subset of the instance's group_by_keys (over-approximation; archive failover catches false positives).
  • Legacy WorkerMessage::AccumulatorInput (aggregation_id-keyed) write site annotated // DEPRECATED: — kept as fallback during validation, removed after warm-tier carries traffic.

Phase 6 finish

  • EngineRouter::execute already retries on CapabilityMiss against the next engine in priority order, so Ghost/Unknown automatically fall through to the archive engine (ThanosForward is already wired into the router today). No router change neededCapabilityMiss(SketchWarmTier, ...) IS the failover trigger. Verified by reading routing/engine_router.rs::execute (lines 218–249).

Deferred (TODOs at call sites)

  • Per-Capability sketch reducer over SketchIndex.query_range for the all-Hit case. Today still flows through the legacy SimpleMapStore path. Follow-up PR will swap that for a real warm-tier evaluator (DDSketch quantile fold, HLL cardinality estimate, CMS / CS frequency lookup).
  • Phase 6 hybrid stitch (warm covers [t0..t1'], archive covers [t1'..t1], concatenate). Requires QueryResult to carry timestamp coverage metadata, which it doesn't today. Deferred — flagged as a stretch goal in the original plan.
  • Legacy WorkerMessage::AccumulatorInput aggregation_id-keyed write path: marked // DEPRECATED: only. Removal is a separate cleanup PR after warm-tier validation in production.

Diff stats

asap-query-engine/src/drivers/ingest/otel.rs              | 569 +++++++++++++++---
asap-query-engine/src/engines/simple/engine.rs            | 277 +++++++++
asap-query-engine/src/main.rs                             |  32 +-
asap-query-engine/src/precompute_engine/engine.rs         |   4 +
asap-query-engine/src/precompute_engine/ingest_handler.rs |  16 +
asap-query-engine/src/stores/sketch_index.rs              |  25 +
crates/.../metrics_service.proto                          |  28 +
7 files changed, 884 insertions(+), 67 deletions(-)

Test plan

  • cargo build --release -p query_engine_rust clean (warnings only)
  • 7 net-new tests pass:
    • drivers::ingest::otel::sid_resolution_tests::* (3 tests: fresh-mint, unknown-sid-empty-attrs, sid-attrs-mismatch)
    • engines::simple::engine::warm_tier_classify_tests::* (3 tests: Ghost → CapabilityMiss, Unknown → CapabilityMiss, no-instance → CapabilityMiss)
    • 1 expansion in series_resolver tests
  • Baseline-checked: 32 pre-existing failures (datafusion::*, persistence_integration_tests::*, schema_timeline_dispatch_tests::*) are red on main independently of this branch — verified via git stash + run on main. This PR's changes are net +7 passing, -1 failing.
  • Follow-up: per-Capability sketch reducer for the all-Hit case
  • Follow-up: hybrid stitch with QueryResult coverage metadata
  • End-to-end smoke after Refactor feat: DataCollector controller client — dynamic query config #2 + this PR + companion ASAPCollector#372: tcpdump shows series_id traffic on subsequent Exports, unknown_series_ids repopulates after backend restart without persistence

🤖 Generated with Claude Code

…ath (Phase 4 + 5 + 6 finish)

Lights up the centralized sid resolver and the new SketchIndex on the
live ingest+query path. Companion to refactor #2 (#121) which built the
SketchIndex storage substrate.

## Phase 4 finish

- OTLP receive (`route_modified_otlp_sketches_to_precompute`): per-DP
  sid resolution gate. Three cases:
    (sid=0, attrs present)  → resolver mints fresh sid via
                              compute-or-mint cache
    (sid != 0, attrs present) → trust attrs; backend-resolved value
                                wins. Mismatch → push old sid to
                                `unknown_series_ids` for sender-side
                                eviction.
    (sid != 0, attrs empty) → resolver.is_known(sid) lookup; miss →
                              push to `unknown_series_ids`, drop DP.
- `route_modified_otlp_sketches_to_precompute` now returns
  `Vec<u64>` of unknown sids; both `MetricsServiceImpl::export` and
  `handle_otlp_http` stamp it into `ExportMetricsServiceResponse.
  unknown_series_ids` (the field already existed on the wire from
  Phase 2, was Vec::new() placeholder).
- New `ResolveSeriesIDs` gRPC service in `metrics_service.proto`:
  bulk pre-resolution endpoint for senders that have an attribute-only
  batch ready before the next Export. Reuses the existing
  `SeriesAssignment` shape; idempotent end-to-end.

## Phase 5 finish

- `IngestState` now carries `Arc<SeriesIdResolver>` +
  `Arc<SketchIndex>`. Both allocated once in `main.rs` and threaded
  through `PrecomputeEngine::new` so the OTLP receiver, the precompute
  engine, and `SimpleEngine` all share the same instances.
- After sid resolution, on first-seen sid the OTLP path materializes
  `SketchInstanceMetadata` (capability inferred from sketch kind:
  DDSketch/KLL → QuantileApprox, HLL → CardinalityApprox,
  CountSketch/CountMin → FrequencyTopk; sketch_config from the
  parent-container fields lifted in Phase 2; accuracy via
  `AccuracyBound::from_config`) and registers it. Per-DP:
  `append_sample(sid, label_values, (start_ms, end_ms), state)`.
- `ModifiedOtlpSketchDp` extended with `series_id`,
  `start_time_unix_nano`, and `container_config: SketchConfig`. The
  five typed-DP match arms thread these from the OTLP wire shape.
- New helpers: `sketch_kind_handle_for(&dp)` and
  `encoding_to_handle(i32) -> SketchEncoding`.
- Legacy `WorkerMessage::AccumulatorInput` write site annotated
  `// DEPRECATED:` — kept as fallback during validation, removed
  after warm-tier carries traffic in production.

- `SimpleEngine` got an optional `sketch_index: Option<Arc<SketchIndex>>`
  field + `with_sketch_index` builder. `QueryEngine::execute` now
  pre-classifies the query's sid set:
    1. Parse PromQL → extract metric name + label-matcher KEY set.
    2. `index.instances_matching(metric, required_keys)` → candidate sids.
    3. For each: `index.classify(sid)`:
        - all `Hit` → fall through to handle_query (legacy path; the
          per-Capability sketch reducer over `query_range` is a
          follow-up — see TODO in the engine).
        - any `Ghost` or `Unknown` → return
          `EngineError::CapabilityMiss(SketchWarmTier, ...)`.
        - empty match → likewise CapabilityMiss.
- New `SketchIndex::instances_matching(metric, required_keys)` helper:
  filters instances where metric_name matches and required_keys is a
  subset of the instance's `group_by_keys` (over-approximation;
  archive failover catches the rest).

## Phase 6 finish

- The existing `EngineRouter::execute` already retries on
  `CapabilityMiss` against the next engine in priority order, so the
  Ghost/Unknown branch falls through to the archive engine
  (Thanos forward) automatically. No router change needed —
  `CapabilityMiss(SketchWarmTier, ...)` IS the trigger.

## Deferred (TODOs at call sites)

- Per-Capability sketch reducer over `SketchIndex.query_range` for
  the all-Hit case (today still flows through the legacy
  `SimpleMapStore` path). Follow-up PR.
- Phase 6 hybrid stitch (warm covers `[t0..t1']`, archive covers
  `[t1'..t1]`, concatenate). Requires `QueryResult` to carry
  timestamp coverage metadata, which it doesn't today. Deferred.
- Legacy `WorkerMessage::AccumulatorInput` aggregation_id-keyed write
  path: marked `// DEPRECATED:` only. Removal is a separate cleanup
  PR after warm-tier validation.

## Build + test

- `cargo build --release -p query_engine_rust` clean (warnings only).
- 7 net-new tests pass:
    drivers::ingest::otel::sid_resolution_tests::* (3 tests)
    engines::simple::engine::warm_tier_classify_tests::* (3 tests)
    plus 1 small expansion in series_resolver tests
- 902/902 tests pass for code touched by this PR. The 32 pre-existing
  failures (datafusion / persistence / schema_timeline_dispatch) were
  red on main before this branch existed and are unrelated to
  warm-tier wiring.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 3169c39 into main May 10, 2026
@zzylol
zzylol deleted the feat/phase4-5-6-wire-in branch May 10, 2026 18:22
zzylol added a commit that referenced this pull request May 10, 2026
The datafusion-backed query path (engines/physical, engines/logical, the
datafusion_summary_library crate, the parallel datafusion-LogicalPlan
adapter, all DF-specific tests, and the controller-side query_language
adapter) is redundant: the controller crate already owns logical and
physical plan layers (controller/src/algebra/{lower,physical,plan}.rs +
intent_algebra/lower.rs + language_logical_plan/). Keeping the parallel
DF stack alive cost an extra translation hop, ~5800 lines of code, and
a test surface that papered over the divergence.

Companion structural cleanup: relocate the new sid-keyed SketchIndex
files (added in #121 / #122) into stores/sketch_db/, the canonical
home for storage. They were floating at stores/ top level after
#121.

## What was deleted

- crates/datafusion_summary_library/ (8 files — sketch-aware datafusion
  ExecutionPlan operators)
- asap-query-engine/src/engines/physical/ (7 files — datafusion-LogicalPlan
  → ExecutionPlan adapters)
- asap-query-engine/src/engines/logical/ (2 files — plan_builder for
  the DF logical layer)
- asap-query-engine/src/tests/datafusion/ (11 test files exercising the
  deleted engines)
- controller/src/query_language/datafusion/ (small adapter)
- datafusion = "43" + datafusion_summary_library deps from
  asap-query-engine/Cargo.toml + workspace Cargo.toml.

## What was moved

- asap-query-engine/src/stores/sketch_index.rs
    → asap-query-engine/src/stores/sketch_db/sketch_index.rs
- asap-query-engine/src/stores/epoch_columnar.rs
    → asap-query-engine/src/stores/sketch_db/epoch_columnar.rs
- All cross-file imports rewritten to the new sketch_db:: paths.

## In-source surgery

- engines/simple/engine.rs: deleted the DF-importing methods
  (SessionContext, physical_plan::collect, record_batch_to_result_map,
  engines::logical::plan_builder::build_*). Phase 5 SketchIndex
  warm-tier hook (with_sketch_index + classify branch in execute())
  preserved verbatim, only its imports were rewritten.

- stores/sketch_db/simple_map_store/per_key.rs: query_disk_parts and
  EpochSource::snapshot_sealed_epoch now panic with
  "datafusion-dependent path removed; ingest/persistence still under
  refactor". Both fire only on persistence-enabled runs (agreed-upon
  breakage). Marked // TODO: replace with non-datafusion path.

- 8 controller files: deleted DF-referencing variants/match arms/trait
  impls.

## Verification

- cargo build --release -p query_engine_rust: clean (3 dead-code warnings)
- cargo build --release -p controller: clean (5 unused-import warnings on
  pre-existing items)
- cargo test --release -p query_engine_rust --lib: 792 passed, 2 failed
  (schema_timeline_dispatch_tests::*, both pre-existing flakes; 8
  persistence tests skipped — they fire the TODO panics)
- grep for "datafusion" in the tree: 6 lines remain, all in TODO
  comments and panic messages. No imports, deps, or types.

## Follow-ups

- Replace persistence TODO panics with a non-datafusion warm-tier
  serializer (likely the asap_sketchlib proto codec).
- The per-Capability sketch reducer (warm-tier query evaluator) is in
  flight in a parallel branch — replaces the all-Hit CapabilityMiss
  with real sketch evaluation from SketchIndex.query_range.

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>
zzylol added a commit that referenced this pull request May 10, 2026
Replaces the all-Hit `CapabilityMiss` fall-through in the warm-tier
classify hook (added in #122) with real sketch evaluation off
SketchIndex.query_range. Queries answerable from sketches now skip
the archive forward entirely.

## What landed

### `engines/warm_tier/sketch_reducer.rs` (559 lines)

```rust
pub struct SketchReducer<'a> {
    pub index: &'a SketchIndex,
}

pub enum WarmTierError {
    UnsupportedFunction(String),
    UnsupportedCapability { function: String, capability: Capability },
    DeserializeFailure { sid: u64, encoding: SketchEncoding, reason: String },
    NoData { metric_name: String },
}

pub struct WarmTierResult {
    pub series: Vec<(BTreeMap<String,String>, Vec<(i64, f64)>)>,
}

impl<'a> SketchReducer<'a> {
    pub fn evaluate(
        &self, sids: &[u64], function_name: &str, function_args: &[f64],
        t0_ms: u64, t1_ms: u64,
    ) -> Result<WarmTierResult, WarmTierError>;
}
```

Per-Capability dispatch:

| Capability | Function | Sketch lib calls |
|---|---|---|
| `QuantileApprox(DDSketch)` | `quantile_over_time`, `histogram_quantile` | `DdSketch::from_raw` + `DdSketch::quantile(q)` |
| `QuantileApprox(Kll)` | `quantile_over_time`, `histogram_quantile` | `KllSketch::new(k)` + replay items + `quantile(q)` |
| `CardinalityApprox` | `count_distinct_over_time`, `cardinality_estimate` | `HllSketch::from_raw` + `estimate()` |
| `FrequencyTopk(CountMin/CountSketch)` | `topk`, `topk_over_time` | TODO — surfaced as `UnsupportedCapability` for now |

Decoders cover `ProtoFull` (always) and `MsgpackFull` (DD/KLL/HLL).
Delta encodings (`ProtoDelta` / `MsgpackDelta`) surface as
`DeserializeFailure` because applying a delta requires the prior base
snapshot, which `query_range` doesn't stitch.

### `engines/warm_tier/promql_extract.rs` (159 lines)

`extract_promql_call(query) -> Option<PromqlCall>` walks the
`promql_parser` AST and returns the outermost call's function name
+ leading numeric args. Supported shapes: `Call(func, args)`,
`Aggregate(op, param, expr)`, unwrapping `Paren` and `Subquery`.
Bare `VectorSelector` / `MatrixSelector` → empty function name
(treated as CapabilityMiss). Binary ops / nested calls beyond
outermost → None (CapabilityMiss).

### `engines/simple/engine.rs` hook

The Phase 5 warm-tier classify branch (added in #122) now does:

```rust
if all_hit {
    let reducer = SketchReducer::new(idx);
    match reducer.evaluate(&candidates, &fn_name, &fn_args, t0_ms, t1_ms) {
        Ok(result) => return Ok(warm_tier_result_to_query_result(result)),
        Err(WarmTierError::UnsupportedFunction(_)
            | WarmTierError::UnsupportedCapability { .. }
            | WarmTierError::DeserializeFailure { .. }
            | WarmTierError::NoData { .. }) => {
            return Err(EngineError::CapabilityMiss(SketchWarmTier, ...));
        }
    }
}
```

Mismatch / decode failure / no data → CapabilityMiss → archive failover
(existing EngineRouter behavior).

## Build + test

- `cargo build --release -p query_engine_rust` — clean.
- New tests (14, all pass): DDSketch quantile_over_time within
  ±5% rel-error; KLL exact for k ≤ 50 items; HLL within 5σ envelope
  of true cardinality; capability mismatch → UnsupportedCapability;
  empty/no-data → NoData; unsupported function → UnsupportedFunction;
  garbage proto → DeserializeFailure; multi-series shape; 5
  promql_extract tests covering quantile/histogram/topk/bare/binary.
- PR #122's 3 warm_tier_classify_tests still green.

## Follow-ups

- Per-window merge for `*_over_time` queries within `[t0, t1]`.
- `FrequencyTopk` + `topk(k, foo)` — needs `CmsWithHeap` SketchKindHandle variant.
- Delta encoding stitching — needs base-snapshot lookup.
- Hybrid stitch (warm `[t0..t1']` + archive `[t1'..t1]`) — needs
  `QueryResult` to carry timestamp coverage metadata.
- `KeyByLabelValues` projection currently flattens to value-only
  Vec<String>; revisit if label-key recovery is needed downstream.

## Depends on

This PR will need to rebase onto #123 (datafusion removal + SketchIndex
into sketch_db) when that lands. The rebase changes are mechanical:
import paths `crate::stores::sketch_index::*` →
`crate::stores::sketch_db::sketch_index::*` at two sites in
`sketch_reducer.rs` + one site in `tests.rs`.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant