Skip to content

fix(data_plane): make durable sketch tier flush, read exact-agg from disk, report real memory - #330

Merged
zzylol merged 1 commit into
mainfrom
fix/sketch-durable-live
May 25, 2026
Merged

zzylol merged 1 commit into
mainfrom
fix/sketch-durable-live

Conversation

@zzylol

@zzylol zzylol commented May 25, 2026

Copy link
Copy Markdown
Contributor

Summary

PR #329's durable tier passed unit tests but the first live run
(--persistence-seal-window-count=4 --persistence-hot-window-secs=120)
exposed three bugs the tests missed. On node2, after 13 min of ingest the
sketch_index/parts/ dir was empty, parts_manifest.log was 0 bytes and
the snapshot was a freshly-initialized empty manifest — only
series_resolver.wal grew (1.5M). So data flowed but nothing was ever made
durable
, a docker restart recovered live=0, and post-restart queries
returned "No result".

Bug 1 — flush never fired (most severe). file:persistence/flusher.rs

The flusher only flushes sealed epochs, and sealing only fires on the
count cadence (seal_window_count distinct windows in current_epoch,
epoch_columnar.rs::maybe_rotate_epoch). A slow/stalled series never reaches
the cadence, so its aged windows sit un-sealed in current_epoch forever
and are never flushable. Fix: a time-driven "phase 0" seal — each tick the
flusher rolls every current_epoch window older than the hot window into a
sealed epoch (new EpochSource::seal_aged_epochs
SidStoreData::seal_aged_windowsMutableEpoch::split_window_ends_before),
so it becomes flushable regardless of cadence. Parts now commit during runtime
and survive restart.

Bug 2 — exact-agg disk read-back missing. file:index/mod.rs

query_exact_agg_range / exact_agg_coverage_bounds read only in-memory
epochs, so a sum by (...) / rate query returned "No result" once its
windows were flushed-then-evicted. Fix: both now union the durable tier,
reconstructing scalar accumulators (Sum/Increase/MinMax + Multiple*) from disk
via reconstruct_exact_agg, keyed by the rebuilt label map.

Bug 3 — approx_memory_bytes ignored current_epoch. file:index/mod.rs + main.rs

It summed sealed epochs only, so the MEMORY_DIAG under-reported ("0.00–0.12
KB approx sealed bytes") and the flusher's memory-pressure trigger was
blind to the bulk of memory (which under persistence lives un-sealed in
current_epoch). Fix: count hot current_epoch + sealed; relabel the
diagnostic.

The persistence-OFF default path (#327) is unchanged: seal_aged_epochs is a
no-op when persistence_enabled is false, and the disk unions are no-ops
without a read handle.

Reproducing tests (fail on origin/main, pass here)

  • live_aged_unsealed_panes_flush_and_survive_restart — bug 1: aged
    below-cadence panes flush to disk and survive a reopen of the same dir.
  • live_exact_agg_resolves_from_disk_after_evict — bug 2: sum by (zone)
    shape + coverage bounds resolve from disk after flush+evict.
  • live_total_memory_accounts_for_current_epoch — bug 3: memory accounting
    includes un-sealed current_epoch.
  • Plus split_window_ends_before_*, seal_aged_windows_*,
    hot_window_time_seals_unsealed_aged_epochs_then_flushes.

Test plan

  • cargo test -p data_plane — 756 lib + integration pass, 0 fail
  • cargo clippy -p data_plane — exit 0, no new warnings in changed files
    (pre-existing approx_constant at forward.rs:850 unchanged)
  • Live re-validation (parent): rebuild+redeploy whole stack at this branch,
    ingest, then docker restart the data-plane and re-run the marquee
    queries.

Recommended live re-validation (forced-flush) settings

--persistence-enabled --persistence-dir=... --persistence-seal-window-count=4 --persistence-hot-window-secs=30 --persistence-flush-interval-ms=1000. A
30s hot window (below the 120s used originally) makes the time-driven seal
fire promptly so flushing is observable within ~1 min; expect non-empty
parts/ + a growing parts_manifest.log, and MEMORY_DIAG should now show
real in-memory bytes. Then docker restart and confirm
persistence recovery: live=N>0 and the quantile_over_time(...[30m]) query
returns data.

Remaining follow-up

MultipleMinMaxAccumulator (needs an out-of-band min/max sub_type not stored
in the part) and the sketch-backed accumulator forms have no generic byte
factory, so their evicted-to-disk exact-agg portion is skipped in
reconstruct_exact_agg — they remain served from memory. Sketch query_range
read-back (KLL/HLL/DDSketch as opaque bytes) is already disk-aware and
unaffected.

🤖 Generated with Claude Code

…agg from disk, and report real memory

PR #329's durable tier passed its unit tests but the first live run
(--persistence-seal-window-count=4 --persistence-hot-window-secs=120)
left parts/ empty after 13 min of ingest, lost all data on docker
restart, dropped [5m]/HLL queries under persistence, and reported
~0 KB sealed bytes. Three root causes:

1. Flush never fired (most severe). The flusher only ever flushes
   SEALED epochs, and sealing only fires on the count cadence
   (seal_window_count distinct windows). A slow/stalled series never
   reaches the cadence, so its aged windows sit un-sealed in
   current_epoch forever — never made durable. Fix: a time-driven
   "phase 0" seal — the flusher now rolls every current_epoch window
   older than the hot window into a sealed epoch each tick
   (EpochSource::seal_aged_epochs / SidStoreData::seal_aged_windows /
   MutableEpoch::split_window_ends_before) so it becomes flushable
   regardless of cadence. Parts now commit during runtime and survive
   restart.

2. Exact-agg disk read-back missing. query_exact_agg_range and
   exact_agg_coverage_bounds read only in-memory epochs, so a
   `sum by (...)` / rate query returned "No result" once its windows
   were flushed-then-evicted. Fix: both now union the durable tier,
   reconstructing scalar accumulators (Sum/Increase/MinMax + Multiple*)
   from disk via reconstruct_exact_agg, keyed by the rebuilt label map.

3. approx_memory_bytes ignored current_epoch, so the MEMORY_DIAG
   under-reported and the flusher's memory-pressure trigger was blind
   to the bulk of memory (which under persistence lives un-sealed in
   current_epoch). Fix: count hot current_epoch + sealed; relabel the
   diagnostic.

Persistence-OFF default path is unchanged (seal_aged is a no-op when
persistence_enabled is false; the disk unions are no-ops without a read
handle). Reproducing tests fail on origin/main and pass here:
live_aged_unsealed_panes_flush_and_survive_restart (#1),
live_exact_agg_resolves_from_disk_after_evict (#2),
live_total_memory_accounts_for_current_epoch (#3), plus columnar/seal
and flusher-level unit tests.

Remaining follow-up: MultipleMinMaxAccumulator (needs an out-of-band
min/max sub_type) and the sketch-backed accumulator forms still have no
generic byte factory, so their evicted-to-disk exact-agg portion is
skipped; they remain served from memory.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol
zzylol merged commit 139972d into main May 25, 2026
@zzylol
zzylol deleted the fix/sketch-durable-live branch May 25, 2026 15:21
zzylol added a commit that referenced this pull request May 25, 2026
…yable (#332)

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>
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