feat(sketch-db): Phase 5 eviction — SchemaEvictionService background task - #41
Merged
Merged
Conversation
…d schemas
The last piece of the Phase 5 eviction chain. Consumes the
primitives from PRs 2 + 3 (drop_agg_id, create_checked) to make
retired schemas actually reclaim space when they pass expires_at_ms,
instead of accumulating forever in the store.
## What's landed
`src/stores/sketch_db/schema_eviction.rs`:
* `SchemaEvictionConfig { poll_interval, dry_run }` — service
tuning, independent from `SchemaRegistry::retirement_retention`
which stays a data-model property.
* `SchemaEvictionService::new(schemas, backfill, store, config)`
and `.spawn() -> SchemaEvictionHandle` for the tokio task.
* `SchemaEvictionHandle::shutdown().await` for clean ctrl-c.
* `SchemaEvictionService::run_once()` — pub-crate synchronous
sweep, exposed so tests can drive deterministically.
* `warn_if_retention_inverted(data_retention, retirement_retention)`
— startup check. If `persistence_delete_older_than <
retirement_retention`, the two retention layers race each other
on expired data; log a `warn!` with actionable message.
Sweep behaviour (per `run_once`):
1. List all `AggStatus::Expired` schemas.
2. For each Expired schema's agg_id:
a. Find any `Running` backfill jobs targeting that agg_id
→ cancel them (writing to data about to drop = wasted work).
Conform to the user's Phase 5 direction: "留着 backfill 也没
用,直接 cancel 然后删掉".
b. `store.drop_agg_id(agg_id)` → evict the windows.
c. `schema_registry.remove_schema(agg_id)` → drop the registry
entry so next tick doesn't re-evict.
3. Log audit line with `agg_id`, `metric`, `retired_at_ms`,
`evicted_windows`, `running_backfills_cancelled`.
`--schema-eviction-dry-run` skips steps 2b/2c — every other step
runs including logging and running-backfill discovery — so the
operator can validate a new retention value before letting it
delete anything.
## Schema module changes
* `DEFAULT_RETIREMENT_RETENTION`: **1h → 24h**. Old default was
too short for typical analyst workflows. Per the user's rule
of thumb `persistence_delete_older_than > retirement_retention`,
24h leaves room for day-over-day comparisons without fighting
data retention.
* `SchemaRegistry::set_retention(duration)` — non-test method so
`main.rs` can override at startup (previously only
`set_retention_for_testing` existed).
* `SchemaRegistry::retirement_retention() -> Duration` — getter
so the eviction service can log the effective value and the
retention-inversion check can read it.
* `SchemaRegistry::remove_schema(agg_id) -> Option<AggSchema>`
— idempotent removal, triggers persist-to-disk if configured
so restart doesn't reintroduce the evicted entry.
## main.rs wiring
* New CLI flags:
* `--enable-schema-eviction` (off by default)
* `--schema-eviction-poll-secs` (default 300)
* `--schema-eviction-dry-run` (off by default)
* On startup, if enabled AND precompute is running: spawns the
service with the schema registry from `precompute_ingest_state`.
* Invokes `warn_if_retention_inverted` before spawn so operators
see the ordering violation immediately on restart.
* `schema_eviction_handle.shutdown().await` slotted into the
existing graceful-shutdown chain.
## Test plan
- [x] 5 new unit tests:
* `run_once_drops_expired_agg_data` — fixture with Expired
agg 1 + Active agg 2 → sweep drops agg 1's data, removes
agg 1 from registry, leaves agg 2 alone.
* `run_once_is_noop_with_no_expired_schemas` — Active-only
registry → no writes.
* `dry_run_logs_but_does_not_drop` — data + registry entry
survive under `dry_run: true`.
* `cancels_running_backfill_for_expired_agg` — Running job on
agg_id=1 → sweep cancels before dropping data.
* `retention_inverted_warning_fires_only_when_inverted`
(doesn't panic on any of the three branches).
- [x] 718 lib tests pass (up from 713).
- [x] clippy `--workspace --all-targets --tests -- -D warnings` clean.
- [x] `cargo fmt -- --check` clean.
## What this completes
The Phase 5 eviction story end-to-end:
1. PR #39 — `Store::drop_agg_id` primitive.
2. PR #40 — `create_checked` rejects backfills outside retention.
3. **This PR** — background service consumes both to actually reclaim
space when schemas retire + expire.
Follow-up (optional, per-operator): per-agg retention override via
`AggregationConfig.parameters["retirement_retention_secs"]`, audit
log persistence, `/api/v1/db/schemas/:agg_id/extend_retention`
manual extension endpoint.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
zzylol
added a commit
that referenced
this pull request
May 5, 2026
* refactor: retire sketch-core mirror * refactor: switch consumer imports to asap_sketchlib::sketches::* Update PR #73 against the reorganized asap_sketchlib (PR #36): the runtime sketches no longer live under a dedicated `asap::` module — they were merged into the existing `src/sketches/` layout (single home per sketch concept, ASAP-runtime types appended to the file that already holds the high-throughput in-process variant). Mechanical path swaps in asap-query-engine: - `asap_sketchlib::asap::dd_sketch::*` → `::sketches::ddsketch::*` - `asap_sketchlib::asap::count_min::*` → `::sketches::countmin::*` - `asap_sketchlib::asap::count_sketch::*` → `::sketches::count::*` - `asap_sketchlib::asap::hll_sketch::*` → `::sketches::hll::*` - `asap_sketchlib::asap::kll::*` → `::sketches::kll::*` - `asap_sketchlib::asap::count_min_with_heap::*` → `::sketches::cms_heap::*` - `asap_sketchlib::asap::hydra_kll::*` → `::sketches::hydra_kll::*` - `asap_sketchlib::asap::set_aggregator::*` → `::sketches::set_aggregator::*` - `asap_sketchlib::asap::delta_set_aggregator::*`→ `::sketches::delta_set_aggregator::*` - `asap_sketchlib::asap::config::*` → `::asap_runtime::*` Naming-conflict renames carried through to the consumers: - `HllDelta` → `HllSketchDelta` (octo_delta::HllDelta still wins the short name) - `HeapItem` → `CmsHeapItem` (common::input::HeapItem still wins the short name) main.rs aliases `asap_sketchlib::asap_runtime as config` so the existing clap derive references (`config::DEFAULT_CMS_IMPL`, `config::configure(...)`) still work without touching the rest of the bin. Tests: - `cargo build --workspace` → clean - `cargo test -p query_engine_rust --lib precompute_operators` → 141 passed, 0 failed Depends on ProjectASAP/asap_sketchlib#36 (force-pushed `e473ccc`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: align CountSketchDelta consumer with sketchlib-go wire format Track the additive `hh_keys` field on `asap_sketchlib::CountSketchDelta` so the proto delta path constructs the type with all fields filled in. Sends an empty `hh_keys` for now: the vendored Rust proto bindings in `asap_otel_proto::sketchlib::v1` haven't been regenerated against the latest `.proto` (which carries `hh_keys` on the Go side). The TopK rebuild on the proto-delta path will fire once those bindings sync; the sketchlib-go-aligned semantics are already in place underneath. Bumps the asap_sketchlib git dep to `refactor/wire-format-align-go` (see asap_sketchlib PR #37). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(asap_sketchlib): bump pin past PR #39 module renames Three asap_sketchlib modules were renamed in upstream PR #39: - sketches::countmin → sketches::countminsketch - sketches::count → sketches::countsketch - sketches::cms_heap → sketches::countminsketch_topk Backend consumers updated. Cargo.toml pin moved from refactor/wire-format-align-go branch to main (which now also has hh_keys restoration via PR #42 and DDSketch + KLL byte parity via #40 + #41). Unblocks ASAPCollector Phase 3 step 3 (backend consumes asap-precompute-rs). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 5, 2026
ASAPQuery-backend ingest path now uses asap-precompute-rs (Rust edge runtime) for shared logic: envelope parsing, delta application, sketch state reconstruction, merge. Query-side engine (PromQL aggregation, storage, query planning) stays in this repo. Phase 3 step 3 of the ASAP edge-framework migration. DDSketch + KLL byte parity on upstream asap_sketchlib (#40, #41). HLL/CountSketch/CMS still pending — functional tests gated with #[ignore = "blocked on ASAPCollector#243"]. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
zzylol
added a commit
that referenced
this pull request
May 5, 2026
…ic (#76) ASAPQuery-backend ingest path now uses asap-precompute-rs (Rust edge runtime) for shared logic: envelope parsing, delta application, sketch state reconstruction, merge. Query-side engine (PromQL aggregation, storage, query planning) stays in this repo. Phase 3 step 3 of the ASAP edge-framework migration. DDSketch + KLL byte parity on upstream asap_sketchlib (#40, #41). HLL/CountSketch/CMS still pending — functional tests gated with #[ignore = "blocked on ASAPCollector#243"]. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 task
zzylol
added a commit
that referenced
this pull request
May 14, 2026
…237) `kll_envelope_round_trip_through_backend_adapter` asserted a byte-identical `snapshot → reconstruct_via_runtime → snapshot` round-trip, but that is not achievable in-repo: `reconstruct_via_runtime(KLLSketch, …)` rebuilds the sketch by replaying the envelope's retained `items` through `KLLWrapper::apply_delta` → `update()`. For an input large enough to compact (the test feeds 400 items at k=200 → 2 levels), that re-feed re-compacts the already-compacted item set — lossily, and with a fresh RNG seed (`None`) rather than the original's `Some(42)`. The re-snapshot is a valid KLL summary but not byte-identical. The test's "single-level layout" rationale was simply wrong for this input. KLL's coin state *is* on the wire (`KllState.coin`), but `asap-precompute-rs` has no `KLL::from_wire_state` to consume `levels`/`items`/`coin` directly — `apply_delta` is item-replay only. True byte parity needs that upstream API, tracked in `asap_sketchlib`#41. So this matches the existing pattern for HLL/CS/CMS in this file: `#[ignore]` with a precise reason, gap visible, CI green. The structural round-trip test for KLL stays live. File header updated to move KLL into the "byte parity blocked on upstream" bucket. 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
1 participant
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
Final PR in the 4-PR eviction chain (§5.1 origin tag →
drop_agg_id→create_checkedretention →SchemaEvictionService). Adds the background task that actually deletes Expired schemas' data from the store, closing the gap in original Phase 2a whereExpiredonly transitioned state but never dropped data.What's in this PR
src/stores/sketch_db/schema_eviction.rs(~400 lines):SchemaEvictionConfig { poll_interval, dry_run }(default: 5 min, dry_run=false)SchemaEvictionService::{new, spawn, run, run_once}SchemaEvictionHandlewithDrop-based shutdownwarn_if_retention_inverted(data_retention, retirement_retention)— startup guardrail (we recommenddata_retention > retirement_retention)run_once()flow per Expired schema: cancel running backfills →Store::drop_agg_id(agg_id)→SchemaRegistry::remove_schema(agg_id)SchemaRegistryadditions:set_retention(Duration),retirement_retention() -> Duration,remove_schema(agg_id) -> Option<AggSchema>(idempotent, triggers persist).1h→24h(more realistic default; inversion warning still fires if ops picks shorter-than-data-retention).main.rsCLI wiring:--enable-schema-eviction,--schema-eviction-poll-secs(default 300),--schema-eviction-dry-run; graceful shutdown chained.Why the deletion model matters
Before this PR an "Expired" schema only lost write admission; its sketches sat in the store indefinitely (live memory + disk footprint). This lines the chain up with ClickHouse's DROP PARTITION model: schema lifecycle change → bulk O(1) removal of that agg_id's buckets, not row-by-row TTL scan.
Backfill-vs-eviction policy: if a schema goes Expired while a backfill is still mid-flight for that agg_id, we cancel + delete — the backfill output is wasted work once the schema itself is gone. (Method B on create-side: reject up-front if
create_checkedwould land data outside retention; this PR handles the in-flight case.)Test plan
cargo test -p asap-query-engine --lib— 718 tests pass (up from 713)cargo clippy --all-targets -- -D warningscleancargo fmt --all -- --checkclean--enable-schema-eviction --schema-eviction-poll-secs 10 --schema-eviction-dry-run, flip a schema Expired via API, observe "would drop" log🤖 Generated with Claude Code