fix(master): compact and version-prune _stats so it stops growing unbounded - #194
Merged
Merged
Conversation
…ounded _stats.rollout.lance is written delete-then-append on every stats upsert, so each scan round adds versions and fragments per experiment, and Lance retains every historical manifest until explicitly cleaned. On a long-running master the chain reached 170k+ versions, making cold start and GET /api/v1/experiments progressively slower. Add StatsStore::maintain(): compact_files with materialize_deletions, then cleanup_old_versions with a grace window, reloading the handle around both. The scanner runs it under the existing stats-writer coordination lock on the first round and every Nth round after (STATS_MAINTENANCE_EVERY_N_SCANS, default 12; STATS_HISTORY_TTL_SECS, default 3600), bounded by a timeout and never failing the scan round. Co-Authored-By: Claude <noreply@anthropic.com>
beinan
added a commit
that referenced
this pull request
Jul 27, 2026
Follow-up to #207. A failing maintenance pass left **no signal at all**. `master_stats_versions_removed_total` only moves on success, so a pass failing for days — object-store outage, permissions, a genuinely stuck compaction — looked **identical to a pass with nothing to reclaim**, while old manifests piled back up and the table walked back toward exactly the state that path exists to prevent. ## New metrics ``` master_stats_maintenance_failures_total counter master_stats_maintenance_consecutive_failures gauge, 0 after any success master_stats_unreclaimed_versions gauge ``` **`unreclaimed_versions` is the signal to alert on** — the version gap since the last successful pass. `master_stats_version` is deliberately *not* the signal: it climbs by design and says nothing about disk. This distinction caused real confusion on the original report ("version 246,000+ and climbing") — the number itself is a harmless `u64` counter. Only manifests still sitting on storage cost anything, and the gap is what measures those. The failure path also logs at `warn` with both numbers, so the reason is visible without a metrics backend. ## Descriptions Also describes every `_stats` metric, **including the three from #194 that had no HELP or TYPE**. Datadog's OpenMetrics check infers type from these, and the description text states explicitly which metric is the alerting signal so it isn't rediscovered the hard way. ## Testing Two tests on the bookkeeping: - consecutive failures accumulate and reset on success - **a failure before any successful pass reports the full backlog** rather than a zero gap — the case a freshly-upgraded, already-bloated deployment hits first, and the one most likely to be got wrong `MasterState` needs etcd to construct, so the counters are tested directly rather than through an `#[ignore]`d integration test that CI would skip. 21 master + 1 metrics tests pass; `clippy --workspace --all-targets -D warnings` and `cargo fmt` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <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.
Problem
_stats.rollout.lanceaccumulates a new Lance version on every stats upsert and is never compacted or version-pruned. On a long-running master it reaches hundreds of thousands of versions (observed 172,789, climbing ~1/min), which makes cold start andGET /api/v1/experimentsprogressively slower.StatsStore::upsertdoes delete-then-append (WriteMode::Append), so each upsert creates ≥1 version. The periodic scan upserts one row per experiment everySTATS_SCAN_INTERVAL_SECS, andupdate_stats_after_compactionupserts again after each compaction — hundreds of versions per hour, unbounded. This is the same class of problem #178 fixed for the task table, which didn't cover_stats.Fix
Proposals 1 and 2 from the issue:
StatsStore::maintain(older_than)—compact_fileswithmaterialize_deletions: true(every upsert leaves a deletion) into one large fragment, thencleanup_old_versionswith a grace window. The handle is reloaded around both so the cleanup sees the rewritten manifest and subsequent reads see the compacted version.stats-writercoordination lock, so only one replica ever rewrites the dataset and it can't race a concurrent scan. Fires on the first round (an existing deployment shouldn't wait N intervals to reclaim a 170k chain) and every Nth round after.Cleanup never touches versions newer than the grace window, so readers on another replica holding a recent version are unaffected.
Config
STATS_MAINTENANCE_EVERY_N_SCANS120disables. At the default 300s scan interval that's hourly.STATS_HISTORY_TTL_SECS3600Metrics
master_stats_maintenance_duration_seconds,master_stats_versions_removed_total,master_stats_version(gauge — the thing that was silently climbing).Tests
maintain_bounds_versions_and_preserves_rows— 40 upserts, assert versions actually reclaimed and fragments compacted, rows intact and correct afterwards, store still writable.maintain_respects_grace_window— a wide TTL removes nothing.cargo test -p lance-context-masterandcargo clippy --all-targetsclean.🤖 Generated with Claude Code