fix(data_plane): make recovered disk-resident warm-sketch series queryable after restart - #332
Merged
Merged
Conversation
…yable After a data-plane restart the durable warm-sketch tier recovered the parts manifest + part cache (#329/#330) but registered NO sids in the in-memory SketchStore `instances` map -- registration only ever happens on the live ingest path when a fresh DataPoint arrives. With an empty registry, `instances_matching` enumerated nothing for the recovered metrics (engine returned "No result" before reading any window) and `query_range`/`query_exact_agg_range`'s disk-union early-returned on the missing `sid_group_by_keys`. The on-disk part format carries only label VALUES + sketch_type_name -- not the metric name, group-by KEYS, or structured `AggKind` the query path needs. Fix: persist a compact per-sid metadata sidecar (`sid_metadata.json`) that the flusher upserts whenever it makes a part durable, and replay it on recovery to re-register each disk-resident sid as a queryable instance. `capability`/`accuracy` are re-derived from the persisted `agg_kind` exactly as the ingest path derives them. Idempotent: a sid a live DataPoint already re-registered is not clobbered. Persistence-OFF behavior is unchanged (the sidecar only exists under the flusher). #330's restart tests passed despite this bug because they call `idx2.register(...)` on the fresh store before querying ("here we re-register to model that") -- masking the disk-only path. The two new tests do a GENUINE fresh reopen with NO register() for both the KLL quantile and Sum exact-agg shapes; both fail on origin/main and pass with this fix. 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.
Problem
PRs #329 (durable tier) + #330 (flush/time-seal/recovery-load/exact-agg disk read) landed, and a live test confirmed flush + recovery load parts (
persistence recovery: live=234). But after a data-plane restart,asap_queryreturned "No result" cluster-wide and the in-memory SketchStore collapsed from ~9000 to ~4 instances.Root cause
Recovery (
persistence/recovery.rs::recover+SketchStore::start_persistence) loads the parts manifest + part-cache but does not register the disk-resident series as sids. Registration into theinstancesmap only ever happens on the live ingest path (drivers/ingest/otel.rs) when a fresh DataPoint arrives; the SeriesIdResolver WAL recovers(metric, attrs_fp, agg_kind) -> sidbut does not push identities into the SketchStore.So after a true restart the
instancesregistry is empty for the recovered metrics:instances_matching(index/mod.rs) keys offinstances-> enumerates nothing -> engine returns "No result" before reading any window.union_disk_parts_into/union_disk_exact_agg_intoearly-return onsid_group_by_keys(sid)(alsoinstances-backed).The on-disk part format (
part.rs) carries only label VALUES +sketch_type_name-- not themetric_name, group-by KEYS, or structuredAggKindthe query path needs to re-register a queryable sid.Fix
Add a compact per-sid metadata sidecar (
sid_metadata.json, newpersistence/metadata.rs):AggKind) right before it makes a part durable, via a newEpochSource::instance_metadata_for_persisthook.start_persistencereplays the sidecar and re-registers each disk-resident sid as a queryable instance (register_recovered_disk_series).capability/accuracyare re-derived from the persistedagg_kindexactly as the ingest path derives them (so no serde needed on the control-planeCapability/SketchKindHandleenums).Properties:
persistence_disabled_keeps_327_retention_behaviorstill passes.Why #330's tests missed it
restart_recovery_makes_flushed_data_queryableandlive_aged_unsealed_panes_flush_and_survive_restartboth callidx2.register(...)on the fresh store before querying (the test even comments "here we re-register to model that"). That re-population masks the disk-only path. The two new tests do a genuine fresh reopen with noregister().Reproducing tests (fail on origin/main, pass here)
recovered_sketch_series_queryable_after_fresh_reopen_without_register-- KLL quantile: ingest -> seal -> flush -> evict (disk-only), fresh reopen, assertinstances_matchingfinds the sid andquery_rangereturns the evicted window.recovered_exact_agg_series_queryable_after_fresh_reopen_without_register-- Sum exact-agg by (zone): same shape viaquery_exact_agg_range+exact_agg_coverage_bounds.Both assert at the
instances_matchingstep (registry=0on origin/main).Test plan
cargo test -p data_plane-- 763 lib tests pass (+7 new: 2 integration + 5 sidecar unit), all 38 persistence tests pass, fix(data_plane): canonicalize sketch metric name at OTLP ingest #323-fix(data_plane): make durable sketch tier flush, read exact-agg from disk, report real memory #330 regressions green.cargo clippy -p data_plane-- no new warnings in touched files (pre-existingapprox_constantat forward.rs:850 ignored).registry=0, "No result").🤖 Generated with Claude Code