feat(ingest): unified registry-allocated sid (PR-1+2+3, precompute follow-up in PR-4) - #190
Merged
Merged
Conversation
…signments (PR-1/3)
Option B from the sid-uniqueness discussion: collapse the duality
between SeriesIdResolver (sequential) and compute_sketch_sid (xxh64
content-addressed) by making the resolver the single mint on the
ingest path. xxh64 only gives probabilistic uniqueness; the wire
shape (agent omits attrs after caching sid → backend disambiguates
by sid) needs uniqueness as a contract, not a property.
Changes:
- `route_modified_otlp_sketches_to_precompute` returns `IngestOutcome
{ unknown_series_ids, series_assignments }` instead of bare
`Vec<u64>`. Both halves of the Phase-4 round trip surface together.
- Attrs-bearing DPs resolve via `series_resolver.resolve(metric, fp)`
instead of `compute_sketch_sid`. Sender's sid disagreeing with the
resolver's binding is signalled via unknown_series_ids; the
canonical assignment is always echoed back so the sender refreshes
its cache.
- gRPC `Export` response now populates `series_assignments`; HTTP
response surfaces the count for observability (OTLP/HTTP spec
keeps the response shape minimal).
- 3 existing tests updated for the new return type + Option B
semantics; 1 new round-trip test
(`second_emit_with_cached_sid_and_no_attrs_hits_same_instance`)
exercises the cache-hit bandwidth-saving path.
Trade-off taken on: sids are no longer stable across independent
backends or backend restarts. PR-2 of this chain adds WAL-backed
resolver persistence; without it, restart-recovery still works via
the existing `unknown_series_ids` eviction primitive (agents observe
stale sid → evict → re-emit with attrs → fresh assignment).
`compute_sketch_sid` is now dead at the ingest layer but stays in
the codebase for now — PR-3 deletes it after PR-2 lands.
cargo build -p data_plane --lib: clean
cargo test -p data_plane --lib drivers::ingest: 14/14 passing
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolver bindings now survive backend restart under
--persistence-enabled. PR-1 made the resolver the authoritative mint;
this PR makes those mints durable.
WAL format v1 (append-only, single-writer):
header: 8 bytes → b"ASAPSRP\x01"
record: u64 sid LE + u32 metric_len LE + metric utf8
+ u32 fp_len LE + fp utf8
Each `resolve()` mint calls `persistence.append(...)` (fsync before
returning) so the caller never observes a sid that isn't on stable
storage. Persistence errors log at WARN and don't propagate — the
resolver stays in-memory-correct; the next restart pays the eviction
cost for the lost mint.
Crash recovery: a torn write at EOF (short read on any record field,
or out-of-range length prefix) is detected at replay; the file is
truncated to the last durable record's offset. Bounds: MAX_METRIC_LEN
16KiB, MAX_FP_LEN 64KiB — well above any realistic input and small
enough that a corrupted file can't OOM the replay loop. No CRC for
now; add one if bit-rot telemetry ever fires.
Trait shape lets tests inject a mock (NoopPersistence) or a failing
backend; production constructor `SeriesIdResolver::open(path)` wires
FilePersistence + replays before returning a warm resolver. `next_sid`
resumes at `max(replayed_sid) + 1`.
Wiring in main.rs: under --persistence-enabled, the WAL lives at
`{persistence_dir}/series_resolver.wal`; otherwise NoopPersistence
preserves the current (in-memory-only) behaviour.
Tests (8 new under series_resolver::persistence_tests):
- empty_log_replays_empty
- append_then_replay_round_trips
- header_mismatch_errors_on_open
- torn_record_truncated_on_replay
- out_of_range_metric_len_treated_as_torn
- resolver_open_replays_existing_log
- resolver_with_noop_persistence_does_not_persist
- append_failure_logs_but_does_not_panic
cargo build -p data_plane (lib + bin): clean
cargo test -p data_plane --lib drivers::ingest: 22/22 passing
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ketch_sid (PR-3/3)
Closes the Interpretation-B identity model on the OTel ingest path.
Sid identity is now `(metric, attrs_fingerprint, agg_kind_canonical)`
— the same 3-tuple `compute_sketch_sid` hashed over, just held as a
registry-allocated u64 instead of a content-addressed one. Two
aggregations over the same series (e.g. DDSketch and Sum on
`http_latency_ms{zone=z0}`) now mint DISTINCT sids, matching the
behaviour that existed before PR-1.
PR-1 had a latent regression: the resolver key was only
`(metric, fp)`, so two sketch kinds over the same series would
collapse to one sid and the second's metadata would silently
overwrite the first at `SketchStore::register`. This PR fixes that
by threading `agg_kind` through to the resolver.
Changes:
- `AggKind::canonical_string()` — stable string form on
`sketch_db::data::AggKind`. Used as the third element of the
resolver's cache key and as the new `agg_kind_canonical` field in
the WAL. Examples: `"sketch:DDSketch:D:0.01"`, `"precompute:Sum:"`.
- `SeriesIdResolver::resolve(metric, fp, agg_kind_canonical)` — added
the third arg. `lookup` similarly. Caller passes the canonical
string (resolver doesn't depend on `AggKind` type, just on a `&str`).
- WAL bumped to v2: header `ASAPSRP\x02`, records gain a 4th
length-prefixed field (`agg_kind_len` u32 LE + bytes). No v1
migration: PR-2 hasn't shipped to production, so v1 files don't
exist in the wild. v1 headers error on open with a clear message.
- OTel modified-OTLP sketch ingest at `otel.rs:879` now builds
`AggKind::Sketch { kind, config }`, calls `canonical_string()`,
passes it to the resolver. The wire-case comment block rewrites to
document the Option-B identity contract.
- `ResolveSeriesIDs` gRPC handler stubbed to return empty
`assignments` + a one-shot WARN log. The proto's `SeriesQuery`
carries only `(metric, fp)`; under the new identity model a
pre-resolve here can't produce the right sid. Drop or extend the
proto in a follow-up.
- `compute_sketch_sid` and its 5 unit tests deleted + the legacy-
parity test deleted. `compute_sid` stays alive: still called by
`SketchStore::ingest_precompute_for_agg_config` for PRECOMPUTE
aggregations. PR-4 migrates that path to the resolver (touching
5 callers: output_sink, eviction, backfill, 2 test sites) and
deletes `compute_sid` + `sketch_kind_tag` + `encode_sketch_config`.
Tests added:
- `distinct_agg_kinds_same_series_distinct_sids` — sid identity
contract under Interpretation B.
- `resolver_open_distinguishes_agg_kinds_on_replay` — WAL v2 records
agg_kind correctly and replay rebuilds the 3-tuple cache.
- All existing resolver + persistence tests updated for the new
signature.
cargo build -p data_plane (lib + bin): clean
cargo test -p data_plane --lib: 763/763 passing
cargo test -p data_plane --lib drivers::ingest: 24/24 passing
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
3 tasks
zzylol
added a commit
that referenced
this pull request
May 13, 2026
…sketch (#193) Adds docs/design-sid-lifecycle.md (559 lines) — design doc for the registry-allocated sid model that landed in #190 + #192. §1-§4 capture shipped behavior: identity contract (sid = registry-allocated u64 for (metric, fp, agg_kind_canonical)), end-to-end architecture diagram, per-DP wire-case table, failure-recovery sequence diagrams (cold start, stale sender sid, restart with/without WAL replay), durability semantics (fsync-per-mint, torn-write detection, WAL v2 format). §5 is a forward-looking sketch for distributed asapquery-backend (sharding on hash(tenant, metric), 8-bit shard_id in top of u64, query coordinator fan-out, HA options, single→sharded migration path). Not implemented; doc says so explicitly. §6 lists 5 open questions for follow-up: ResolveSeriesIDs RPC fate, WAL compaction threshold, per-tenant sid subspace, collector-side routing colocation, cross-shard PromQL semantics. 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
Three-commit chain implementing Option B (backend-allocated registry) for sid mint on the OTel ingest path. Sid identity is
(metric, attrs_fingerprint, agg_kind_canonical)— the same 3-tuplecompute_sketch_sidhashed over, just held as a registry-allocated u64 instead of content-addressed.Why registry (B) over u128 widening (A): xxh64 gives only probabilistic uniqueness; the wire shape requires uniqueness as a contract (agent omits attrs on subsequent emits → receiver must disambiguate
sid → (metric, attrs, agg_kind)). Registry approach gives uniqueness by construction (AtomicU64::fetch_add). Trade-off accepted: sids are not stable across independent backends; cross-restart durability solved by WAL persistence.Commits
84abac7— PR-1: SeriesIdResolver authoritative on ingest path. Replacescompute_sketch_sidcall atotel.rs:880withseries_resolver.resolve. ReturnsIngestOutcome { unknown_series_ids, series_assignments }so the gRPCExportresponse can echo the canonical sid back.3ac90cf— PR-2: WAL-backed persistence.SeriesResolverPersistencetrait +NoopPersistence+FilePersistence. WAL append-only, single-writer, fsync per record.SeriesIdResolver::open(path)replays on construction, resumesnext_sidatmax(replayed)+1. Torn-write detection truncates to last safe offset. Wired inmain.rsunder--persistence-enabled.f72b533— PR-3: identity =(metric, fp, agg_kind); deletecompute_sketch_sid. Fixes a latent PR-1 regression (two sketch kinds over same series collapsed to one sid). WAL bumped to v2 (4-field records).compute_sketch_sid+ 5 unit tests deleted.Design contract pinned
sid = registry-allocated u64 for (metric, attrs_fingerprint, agg_kind_canonical)SeriesIdResolver.series_assignmentsreply, omits attrs on subsequent emits.unknown_series_idsreply → sender evicts + re-emits with attrs → resolver hits cache or mints fresh.WAL format v2
Caps: metric ≤16KiB, fp ≤64KiB, agg_kind ≤4KiB. No CRC for v2; add if telemetry surfaces bit-rot.
ResolveSeriesIDs RPC stubbed
The pre-resolve gRPC handshake at
otel.rs::resolve_series_i_dsreturns emptyassignmentsand logs a one-shot WARN. The proto'sSeriesQuerydoesn't carryagg_kind— a pre-resolve there can't produce the right sid under the new identity model. Agents that ignored the RPC and just sent Export-with-attrs work unchanged. Follow-up: drop the RPC, or extendSeriesQueryto carryagg_kind_canonical.PR-4 (separate follow-up)
compute_sidis not deleted here — still called bySketchStore::ingest_precompute_for_agg_configfor PRECOMPUTE aggregations (5 call sites: output_sink, eviction service, backfill processor, two test sites). PR-4 migrates those to the resolver and deletescompute_sid+sketch_kind_tag+encode_sketch_config. Closes the inconsistency — one mint authority for every sid in the system, matching the stated goal of "single sid across asapcollector and asapquery-backend; precompute is just an extension of the OTel processors at the edge."Test plan
cargo build -p data_plane(lib + bin) — clean (5 pre-existing warnings)cargo test -p data_plane --lib— 763/763 passingcargo test -p data_plane --lib drivers::ingest— 24/24 passing (was 14 pre-chain; +8 persistence + +2 identity tests)otel.rs:867pins the right design contract (Interpretation B — sid =(metric, fp, agg_kind)).series_assignmentsreply is what the exporter caches today;ResolveSeriesIDswas optional and now returns empty (graceful degradation).