feat(sketch-db): Store::drop_agg_id + SimpleMapStore impl (Phase 5 eviction prereq) - #39
Merged
Merged
Conversation
…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
…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>
zzylol
added a commit
that referenced
this pull request
Apr 18, 2026
…d schemas (#41) 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 24, 2026
fix(gorilla-merger): custom StoreAPI over tsdb.DB (bypass thanos TSDBStore crash, #39)
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
Adds the primitive the upcoming
SchemaEvictionServicewill call to reclaim space when a retired schema hitsexpires_at_ms. Analogous to ClickHouse'sDROP PARTITION— O(1)-ish key removal, atomic w.r.t. concurrent reads for the same agg_id.Store::drop_agg_id(agg_id) -> Result<usize, _>trait method. Default impl returnsOk(0)(non-SimpleMapStore implementors opt in later).SimpleMapStoreGlobal+SimpleMapStorePerKeyoverrides: count + remove from main map, clear earliest-ts index, clear read_counts (Global only). Leaves metric-keyed indices alone since other agg_ids may share the metric.Test plan
Next
BackfillRegistry::create_checkedadds data-retention check (method B from design discussion).SchemaEvictionServicetokio task consumingdrop_agg_id.🤖 Generated with Claude Code