Skip to content

feat: complete OTLP sketch ingest → SimpleMapStore - #3

Merged
zzylol merged 1 commit into
mainfrom
feat/otlp-sketch-ingest-v2
Apr 13, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/otlp-sketch-ingest-v2

Conversation

@zzylol

@zzylol zzylol commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

Completes the TODO in otel.rs — OTLP metrics now flow into SimpleMapStore.

What changed

  • OtlpReceiver accepts Arc<dyn Store> + Arc<StreamingConfig>
  • store_otlp_sketches() parses OTLP, creates PrecomputedOutput+accumulator pairs, inserts into store
  • Both sketch payloads and raw metric points are stored
  • main.rs wires store into OtlpReceiver

Integration with DataCollector

SDK → OTel sketchcol (DataCollector) → OTLP gRPC/HTTP → ASAPQuery backend (this PR) → SimpleMapStore → queries

How to run

cargo run -- --enable-otel-ingest --otel-grpc-port 4317 --otel-http-port 4318
# Then send OTLP metrics to localhost:4317

🤖 Generated with Claude Code

zzylol added a commit that referenced this pull request Apr 13, 2026
This builds on the original PR #3 ingest path and realizes the intended
architecture: OTLP-delivered metrics (and pre-built sketches from the
DataCollector OTel collector) now flow through the precompute engine's
worker pool, which performs window-aligned aggregation per StreamingConfig
before writing to SimpleMapStore.

Architecture:
  DataCollector OTel collector
    → OTLP gRPC/HTTP (OtlpReceiver)
    → precompute engine ingest router
    → per-(agg_id, group_key) worker panes
    → StoreOutputSink → SimpleMapStore
    → query engine

Key changes
-----------
- PrecomputeEngine::new() now eagerly builds channels, router, agg_configs,
  and a shared Arc<IngestState>. The state is exposed via a new
  `ingest_state()` getter so other ingest sources can push into the same
  worker pool without duplicating setup.
- IngestState is promoted from pub(crate) to pub, alongside an
  `extract_group_key_for(series_key, config)` associated helper that other
  drivers (OTLP here, Kafka potentially later) can reuse for label→group
  extraction.
- New WorkerMessage::AccumulatorInput variant carrying a pre-built
  Box<dyn AggregateCore> with agg_id, group_key, and timestamp. Routed to
  workers by the same (agg_id, group_key) hash as GroupSamples.
- GroupState gains a `sketch_panes: BTreeMap<i64, Box<dyn AggregateCore>>`
  alongside the existing `active_panes`, plus a new
  `process_accumulator_input()` worker method that:
    * merges incoming accumulators into the covering pane via merge_with,
    * honors late-data policy (Drop / ForwardToStore),
    * emits both raw-sample and sketch-pane outputs on window close.
  `flush_all` is also extended to drain sketch panes for closed windows.
- New `merge_sketch_panes_for_window` helper mirrors the existing
  `merge_panes_for_window` — oldest pane is destructively taken; later
  panes are cloned via clone_boxed_core for still-open sliding windows.
- OtlpReceiver gains `with_ingest_state()` constructor. When wired to
  the precompute engine, OTLP requests are dispatched as GroupSamples
  (raw metric points) and AccumulatorInput (sketch payloads) through the
  engine's router. Label semantics are preserved — each point is formatted
  into a standard `metric{k1="v1",k2="v2"}` series key and passed through
  the same extract_group_key pipeline used by the Prometheus path, so
  StreamingConfig.grouping_labels drives pane keying uniformly.
- SketchPayload tuple is promoted to a structured `SketchPoint` that
  carries name, attr_name, labels, timestamp, and opaque payload bytes.
  Labels are now preserved through the sketch path (previously lost).
- main.rs constructs the precompute engine BEFORE the OTLP receiver so
  the receiver can obtain an Arc<IngestState>. Without the precompute
  engine OTLP falls back to log-only mode.

Known gap (follow-up)
---------------------
The sketch payload currently reaches the worker as an empty SumAccumulator
placeholder. The plumbing — label preservation, routing, pane merging,
late-data handling — is complete and exercised end-to-end, but per-variant
`SketchEnvelope → concrete accumulator` decoders (CountMin, KLL, HLL,
CountSketch, DDSketch, ...) still need to be written. Adding them is
purely a matter of filling in `identify_sketch_type`'s arms with
`deserialize_from_bytes_arroyo`-style constructors and wiring them
through; no further engine changes are required.

Verification
------------
- cargo check --all-targets: clean
- cargo clippy --all-targets -- -D warnings: clean
- cargo fmt --check: clean
- cargo test --lib: 435 passed, 0 failed, 5 ignored

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zzylol
zzylol force-pushed the feat/otlp-sketch-ingest-v2 branch from dd22379 to 346b42f Compare April 13, 2026 03:32
Rebuilds the OTLP ingest path on top of the merged PR #1 + PR #2
changes so that OTLP-delivered metrics (and pre-built sketches from the
DataCollector OTel collector) flow through the precompute engine's
worker pool. The precompute engine performs window-aligned aggregation
per StreamingConfig before writing to SimpleMapStore, matching the
Prometheus / VictoriaMetrics ingest pattern.

Architecture:
  DataCollector OTel collector
    → OTLP gRPC/HTTP (OtlpReceiver)
    → precompute engine ingest router
    → per-(agg_id, group_key) worker panes
    → StoreOutputSink → SimpleMapStore
    → query engine

Key changes
-----------
- PrecomputeEngine::new() now eagerly builds channels, router, agg_configs
  and a shared Arc<IngestState>. The state is exposed via a new
  `ingest_state()` getter so other ingest sources can push into the same
  worker pool without duplicating setup.
- IngestState is promoted from pub(crate) to pub, alongside an
  `extract_group_key_for(series_key, config)` associated helper that other
  drivers (OTLP here, Kafka potentially later) reuse for label→group
  extraction.
- New WorkerMessage::AccumulatorInput variant carrying a pre-built
  Box<dyn AggregateCore> with agg_id, group_key, and timestamp. Routed
  to workers by the same (agg_id, group_key) hash as GroupSamples.
- GroupState gains a `sketch_panes: BTreeMap<i64, Box<dyn AggregateCore>>`
  alongside the existing `active_panes`, plus a new
  `process_accumulator_input()` worker method that:
    * merges incoming accumulators into the covering pane via merge_with,
    * honors late-data policy (Drop / ForwardToStore),
    * emits both raw-sample and sketch-pane outputs on window close.
  `flush_all` is also extended to drain sketch panes for closed windows.
- New `merge_sketch_panes_for_window` helper mirrors
  `merge_panes_for_window`: oldest pane destructively taken, later panes
  cloned via `clone_boxed_core` for still-open sliding windows.
- OtlpReceiver gains `with_ingest_state()` constructor. When wired to
  the precompute engine, OTLP requests are dispatched as GroupSamples
  (raw metric points) and AccumulatorInput (sketch payloads) through the
  engine's router. Label semantics are preserved — each point is
  formatted into a standard metric{k1="v1",k2="v2"} series key and run
  through the same extract_group_key pipeline used by the Prometheus
  path, so StreamingConfig.grouping_labels drives pane keying uniformly.
- SketchPayload tuple is promoted to a structured `SketchPoint` that
  carries name, attr_name, labels, timestamp, and opaque payload bytes.
  Labels are now preserved through the sketch path (previously lost).
- Incoming sketch payloads are wrapped in
  SketchEnvelopeAccumulator::from_proto_bytes (introduced by PR #2) and
  sent to the worker as AccumulatorInput. This preserves the full
  sketch state end-to-end; per-variant concrete decoding (CountMin →
  CountMinSketchAccumulator, KLL → DatasketchesKLLAccumulator, …) can
  layer on top later without changing the routing contract.
- main.rs constructs the precompute engine BEFORE the OTLP receiver so
  the receiver can obtain an Arc<IngestState>. Without the precompute
  engine OTLP falls back to log-only mode.

Rebase notes
------------
This commit subsumes the earlier PR #3 commit dd22379 (which wrote
directly to SimpleMapStore with SumAccumulator placeholders). The
original commit conflicted with PR #2's own OTLP changes after PR #2
landed on main; rather than carry two overlapping commits forward, the
earlier placeholder work is replaced in-place by this coherent refactor.

Verification
------------
- cargo check --all-targets: clean
- cargo clippy --all-targets -- -D warnings: clean
- cargo fmt --check: clean
- cargo test --lib: 435 passed, 0 failed, 5 ignored

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zzylol
zzylol force-pushed the feat/otlp-sketch-ingest-v2 branch from 346b42f to 1048466 Compare April 13, 2026 03:38
@zzylol
zzylol merged commit d5443ba into main Apr 13, 2026
5 of 6 checks passed
@zzylol
zzylol deleted the feat/otlp-sketch-ingest-v2 branch April 13, 2026 12:21
zzylol added a commit that referenced this pull request Apr 18, 2026
…iction prereq)

Adds the primitive the upcoming `SchemaEvictionService` will call to
reclaim space when a retired schema hits `expires_at_ms`. Analogous
to ClickHouse's `ALTER TABLE ... DROP PARTITION` — O(1)-ish key
removal, atomic w.r.t. concurrent reads of the same agg_id.

## What's landed

`Store` trait gains:
```rust
fn drop_agg_id(&self, _agg_id: u64)
    -> Result<usize, Box<dyn std::error::Error + Send + Sync>> {
    Ok(0)
}
```
Default impl returns `Ok(0)` so non-`SimpleMapStore` implementors
keep compiling — they can opt in later.

`SimpleMapStoreGlobal::drop_agg_id` and
`SimpleMapStorePerKey::drop_agg_id` override with real eviction:
* Count the windows being dropped (for the return value + audit log)
* Remove the per-agg_id entry from the main store map
* Clear `earliest_timestamp_per_aggregation_id[agg_id]`
* Clear `read_counts[agg_id]` (Global only — PerKey doesn't have it)
* Leaves `metrics` / `items_inserted` alone (those are metric-keyed,
  not agg-keyed; other agg_ids under the same metric survive)

## Contract documented on the trait

* **Idempotent**: unknown `agg_id` is a no-op, returns `Ok(0)`.
* **Atomic w.r.t. reads for same agg_id**: Global grabs the store's
  Mutex, PerKey uses DashMap's per-shard atomic remove.
* **Does NOT touch registries**: caller is responsible for removing
  the schema / backfill-job entries.

## Test plan

7 new tests in `drop_agg_id_tests`:
- Global + PerKey variants × {removes target, unknown agg no-op,
  clears earliest_ts index}.
- Drop-then-reinsert works as a fresh agg (no residual state).

710 lib tests total (up from 703); clippy + fmt clean.

## Next

PR #3: `BackfillRegistry::create_checked` gains `persistence_delete_older_than_ms`
check so jobs requesting `start_ms` outside the retention window get
rejected up-front with `CreateError::OutOfRetention`.

PR #4: `SchemaEvictionService` tokio task that consumes `drop_agg_id`
to actually clean up expired schemas.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request Apr 18, 2026
…iction prereq) (#39)

Adds the primitive the upcoming `SchemaEvictionService` will call to
reclaim space when a retired schema hits `expires_at_ms`. Analogous
to ClickHouse's `ALTER TABLE ... DROP PARTITION` — O(1)-ish key
removal, atomic w.r.t. concurrent reads of the same agg_id.

## What's landed

`Store` trait gains:
```rust
fn drop_agg_id(&self, _agg_id: u64)
    -> Result<usize, Box<dyn std::error::Error + Send + Sync>> {
    Ok(0)
}
```
Default impl returns `Ok(0)` so non-`SimpleMapStore` implementors
keep compiling — they can opt in later.

`SimpleMapStoreGlobal::drop_agg_id` and
`SimpleMapStorePerKey::drop_agg_id` override with real eviction:
* Count the windows being dropped (for the return value + audit log)
* Remove the per-agg_id entry from the main store map
* Clear `earliest_timestamp_per_aggregation_id[agg_id]`
* Clear `read_counts[agg_id]` (Global only — PerKey doesn't have it)
* Leaves `metrics` / `items_inserted` alone (those are metric-keyed,
  not agg-keyed; other agg_ids under the same metric survive)

## Contract documented on the trait

* **Idempotent**: unknown `agg_id` is a no-op, returns `Ok(0)`.
* **Atomic w.r.t. reads for same agg_id**: Global grabs the store's
  Mutex, PerKey uses DashMap's per-shard atomic remove.
* **Does NOT touch registries**: caller is responsible for removing
  the schema / backfill-job entries.

## Test plan

7 new tests in `drop_agg_id_tests`:
- Global + PerKey variants × {removes target, unknown agg no-op,
  clears earliest_ts index}.
- Drop-then-reinsert works as a fresh agg (no residual state).

710 lib tests total (up from 703); clippy + fmt clean.

## Next

PR #3: `BackfillRegistry::create_checked` gains `persistence_delete_older_than_ms`
check so jobs requesting `start_ms` outside the retention window get
rejected up-front with `CreateError::OutOfRetention`.

PR #4: `SchemaEvictionService` tokio task that consumes `drop_agg_id`
to actually clean up expired schemas.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request Apr 21, 2026
Addresses TODO.md blocker #3. Complements the in-process
`e2e_feedback_loop_tests` in `simple_engine.rs` (which swaps the
`HotReloadStreamingConfig` directly from a mock `ControllerClient`)
by adding the full HTTP round-trip:

  backend /api/v1/query → engine-miss →
  HttpControllerClient POST /api/v1/plan (mock controller) →
  controller POSTs canned StreamingConfig YAML back to
  backend /api/v1/streaming-config →
  backend GET /api/v1/streaming-config reflects the new plan.

Two tests in `tests/capability_miss_http_e2e_tests.rs`:

* `http_capability_miss_feedback_loop_closes_over_http` —
  measures `time_to_plan_ready` (localhost floor ~20 ms),
  asserts the exact agg_id the controller pushed is present on
  the backend, and verifies the mock controller observed the
  documented `{kind: "capability_miss", ...}` payload.
* `http_capability_miss_repeat_query_is_idempotent_over_http` —
  proves a repeat query on the now-covered metric does NOT
  trigger a second capability-miss notify, locking down the
  loop's idempotency guarantee at the HTTP boundary.

Scope note documented in the module preamble: OTLP data
ingestion between the plan push and the repeat query is
deferred to the cross-process compose harness tracked in
DataCollector/TODO.md — what this file locks down is the
HTTP-boundary behaviour of the feedback loop.

Test count: 777 → 779.
zzylol added a commit that referenced this pull request Apr 21, 2026
)

Addresses TODO.md blocker #2 subitem #3. Queries answered by the
sketch DB now carry a theoretical accuracy bound back to the
client as two fields on the Prometheus HTTP response:

* **A — structured `accuracy` (top-level)**:
  ```json
  "accuracy": {
    "epsilon": 0.008125,
    "delta":   0.0,
    "kind":    "relative_cardinality",
    "per_segment": [...]   // populated on schema-timeline crossings
  }
  ```
* **C — human-readable `infos` (Prometheus 3.0 / Grafana 11+)**:
  ```json
  "infos": ["accuracy: ε=0.008125, δ=0, kind=relative_cardinality"]
  ```

Both are standard Prometheus-tolerant extensions — unknown
top-level fields are ignored by the upstream client/Grafana 10.
A regression test confirms a stripped-down "standard Prometheus"
decoder round-trips through our extended response.

## Wire shape

`warnings` stays reserved for partial-result / fallback
advisories (PR #49's schema-timeline dispatcher still uses it);
accuracy gets its own dedicated field so the semantics don't
mix.

When a query crosses a schema-timeline boundary, `per_segment`
lists each segment's `(agg_id, range_ms, profile)`. The
top-level `profile` is the max-ε, max-δ envelope across
segments — a conservative upper bound.

## Changes

* `stores/sketch_db/accuracy.rs`:
  * `AccuracyEnvelope { profile, per_segment }` + builders
    (`single`, `from_segments`)
  * `PerSegmentAccuracy { agg_id, range_ms, profile }`
  * `AccuracyProfile::summary()` / `AccuracyEnvelope::summary()`
    emit the `infos` one-liner.
* `engines/query_result.rs`: `InstantVector` / `RangeVector`
  carry `accuracy: Option<AccuracyEnvelope>`; new
  `QueryResult::{accuracy(), with_accuracy()}`.
* `drivers/query/adapters/prometheus_http.rs`:
  `PrometheusResponse::{infos, accuracy}` fields +
  `with_accuracy()` builder.
  `format_success_response` / `format_range_success_response`
  thread `result.accuracy()` onto the response.
* `engines/simple_engine.rs`:
  * `SimpleEngine::accuracy_envelope_for(agg_id)` — single-
    aggregation helper.
  * `execute_context` attaches single-agg accuracy.
  * Timeline dispatch builds per-segment accuracy list and
    attaches the multi-segment envelope.

## Tests

777 → 784 (+5 green):
* `prometheus_response_carries_accuracy_top_level_and_infos_mirror`
* `prometheus_response_without_accuracy_skips_both_fields`
* `prometheus_response_per_segment_contains_all_segments_with_worst_case_top`
* `accuracy_coexists_with_warnings_without_interference`
* `promql_standard_client_can_decode_response_ignoring_extensions`

clippy + fmt clean.
zzylol added a commit that referenced this pull request May 13, 2026
Post-M2.3 reorg #3 of 8. Renames `sketch_db/store/` → `sketch_db/index/`.
The directory now names what it actually does (it's the sid index +
per-sid columnar substrate, not the umbrella implementation unit it
was when both store/ + persistence/ lived inside).

Mechanical:

- `git mv sketch_db/store sketch_db/index`
- `pub mod store;` → `pub mod index;` in `sketch_db/mod.rs`
- `sketch_db::store::*` → `sketch_db::index::*` across the codebase
  (52 references)
- Type `SketchStore` keeps its name for now; rename is a follow-up
  decision (the type is just one resident of the index/ directory,
  alongside `epoch_columnar`).

783/783 lib tests pass.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 13, 2026
#185)

Schema retirement #3. Repoint `ASAPQueryEngine::timeline_for_query`
from `SchemaRegistry::timeline_for_metric` to the sid-level
`storage_engines::sketch_db::query::timeline::timeline_for_metric`
landed in #183. The cross-reconfigure dispatcher
(`try_handle_query_promql_via_timeline`) now reads its segments from
the sid catalog rather than from `SchemaRegistry`.

The sid-level timeline populates `TimelineSegment.agg_id` with a
content-hash of `(metric, agg_kind, group_by_keys)` rather than a
`StreamingConfig.aggregation_id`. Until schema retirement #5 ports
the per-segment dispatch to sid-level evaluation, the
segment-→-aggregation_config lookup inside the dispatcher is
best-effort: when no segment resolves to an in-config aggregation
the dispatcher returns `None` so the caller falls back to the
default single-agg path instead of regressing cross-reconfigure
queries to empty-result-plus-warnings.

The schema retirement plan keeps the `schema_registry` field on
`ASAPQueryEngine` alive for now — it's still referenced by the
ingest barrier and the swap-handler driver. Both go away in
retirements #4 + #5.

Two tests in `tests/schema_timeline_dispatch_tests.rs` are marked
`#[ignore]`: they build two distinct `AggregationConfig`s with
identical content (same metric / Sum / `host` grouping). In the
sid catalog those collapse to one signature group → one segment,
so the dispatcher can no longer reproduce the
schema-boundary-stitch scenario from a SchemaRegistry-shaped
fixture. The third single-schema regression test still passes
unchanged. Re-enabling these is part of retirement #5 (sid-level
dispatch) or a fixture rewrite that uses two genuinely distinct
signatures.

787/787 lib tests pass; 5 ignored.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 13, 2026
…ble from otel (#187)

Schema retirement step #3. Move the §6.3 write-side ingest barrier
from the `SchemaRegistry`'s agg_id-keyed `is_writable` into the
sid catalog directly.

`SketchStore::ingest_precompute_for_agg_config` now returns `None`
when the sid it resolves to already exists in `Retired` or
`Expired` status — the same drop semantics the retired
`schemas.is_writable(agg_id)` check provided, just keyed on the
sid the sketch_db actually understands. New sids (instance not
yet registered) still proceed to register + append; the gate only
applies once `lifecycle::reconcile_from_streaming_config` (PR #186)
has flipped the sid out of `Active`.

Drop the three `schemas.is_writable(agg_id)` checks in otel.rs:

- `route_otlp_to_precompute` raw-sample loop
- `route_otlp_to_precompute` sketch-envelope loop
- `route_modified_otlp_sketches_to_precompute` legacy worker-push
  loop

The `samples_blocked_by_schema_barrier` counter and its companion
Prometheus metric stay alive but now read 0 in steady state — the
sid-level drops happen inside SketchStore without crossing the
schema-keyed counter. Once the schema/ module is fully retired the
counter wiring moves to sid-level (or gets renamed).

793/793 lib tests pass; 5 ignored.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 18, 2026
…llas3 bucket (B1 downstream bundle) (#279)

Three follow-ups to ASAPCollector#383 (agent OpAMP-apply) and
companion PRs, bundled because they all touch
control_plane/src/emit/stage_config.rs.

Fix 1 (Issue #2): thread X-Agent-ID into opamp emit
  The controller's emitted opamp extension block didn't include
  `headers: X-Agent-ID: <id>` -- so when the agent applies a
  controller-pushed config and Docker restarts it, the
  reconnected agent has no agent-id and the controller's OpAMP
  server can't identify it. Empty `/api/v1/agents` after restart.
  Thread agent_id from replan / main into the three emit
  functions; add to each opamp block.

  Per-agent call sites (`push_config_to_agent`, the per-agent
  re-emit loop inside `replan_metric`, and the bootstrap GET path
  when `pinned_agent_id` is set) thread the real agent id.
  Broadcast call sites that don't have a single agent in scope
  (handle_plan's typed-stage-split push, handle_rollback, replan
  fallback) emit the literal `$AGENT_ID` placeholder and rely on
  the agent container's env to expand it at boot.

Fix 2 (Issue #3): ASAP_AGENT_MEMORY_LIMIT_MIB env knob
  memory_limiter was hardcoded to 1280 MiB. Smoke agent at 1.5 GiB
  trips the soft limit under the 5-sketch + gorillas3 workload.
  Read ASAP_AGENT_MEMORY_LIMIT_MIB from the controller's env
  (default 1280, mirroring the build_gorillas3_yaml env-substitute
  pattern). `spike_limit_mib` scales as `max(256, limit/5)` so the
  ratio stays sensible as operators tune the limit. Operators
  raise both the env var AND the agent container's cgroup limit
  together.

Fix 3 (gorillas3 Bucket Phase 2): drop `bucket:` from emit
  ASAPCollector#387 made the gorillas3 `Bucket` field a no-op:
  validation + log + TSDBBucket fallback retired. Controller
  doesn't need to emit it anymore. Remove the bucket: line from
  build_gorillas3_yaml. ASAPCollector#387's gorillas3 Config
  struct still has the `Bucket` field (mapstructure compat) but
  it's now unread.

Test plan:
  * `cargo test -p control_plane --lib`: 706 (699 baseline + 7 new)
  * X-Agent-ID assertion: emitted yaml under emit_edge_yaml (both
    legacy + 5-sketch routing) and emit_gateway_yaml contains
    `X-Agent-ID:` with the threaded agent_id; broadcast callers
    preserve the `$AGENT_ID` placeholder verbatim.
  * memory_limit assertion: emitted yaml's memory_limiter.limit_mib
    matches ASAP_AGENT_MEMORY_LIMIT_MIB env (or 1280 default), with
    spike_limit_mib scaling as max(256, limit/5).
  * bucket: absence: build_gorillas3_yaml output does NOT contain
    a top-level `bucket:` line (but DOES contain `tsdb_bucket:`).

Closes B1-downstream Issues #2 + #3 + gorillas3-Bucket Phase 2.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 25, 2026
…agg from disk, and report real memory (#330)

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