feat(master): make _stats maintenance failures observable - #208
Merged
Conversation
A failing maintenance pass left no signal. `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 the state this path exists to prevent. Adds: 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 version gap since the last successful pass, and is the signal worth alerting on. The raw `master_stats_version` is not: it climbs by design and says nothing about disk. Only manifests still on storage cost anything, and that is what the gap measures. The failure path also logs at warn with both numbers, so the reason is visible without a metrics backend. Also describes every `_stats` metric, including the three from lance-format#194 that had no HELP or TYPE. Datadog's OpenMetrics check infers type from these, and the descriptions state explicitly which metric is the alerting signal. Two tests cover the bookkeeping: consecutive failures accumulate and reset on success, and 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. Co-Authored-By: Claude <noreply@anthropic.com>
Kazimierzsier
approved these changes
Jul 27, 2026
beinan
added a commit
that referenced
this pull request
Jul 27, 2026
#211)⚠️ **Stacked on #209 → #210.** Both of those commits are included here. Merge order: **#209 → #210 → this**. Once they land, this diff reduces to the UI commit alone. ## Why this exists #210 removes cold experiments from the stats table — that's what keeps a scan round proportional to *active* work. But on its own it also removes them from `GET /experiments`, which is only acceptable once they stay findable another way. **This PR is what makes retirement safe to turn on.** | | behaviour | |---|---| | **default list** | the stats table — experiments written recently enough not to have been retired | | **search** | stats table **plus the registry** for names it no longer holds; cold matches observed on demand | | **detail** | on a stats miss, resolve via registry and observe on demand rather than returning 404 | Retirement therefore removes an experiment from the *default view* and **never makes it unfindable**. The default is also the right one at scale on its own merits: a flat list of every experiment ever created is neither renderable nor interesting once there are tens of thousands. ## Two deliberate decisions **1. `observe_cold` does not write a stats row.** Reading about a retired experiment must not make it hot again — otherwise **browsing the UI would silently undo retirement**, and the table would creep back toward holding everything, which is exactly the state retirement exists to prevent. It also takes no `MasterState`, so it *structurally* cannot write one. A refactor that hands it one has to justify itself. There's a test asserting the no-rehydration property. **2. Search bounds its on-demand observations by page size.** A query matching thousands of cold experiments must not open thousands of datasets to render one page. ## Known gap — stating it rather than hiding it The two handlers changed here live in `routes.rs`, whose tests **all require etcd and are `#[ignore]`d**, so CI does not cover them. `observe_cold` is tested in `scanner.rs`, which CI does run. So the registry-fallback logic in the handlers is verified locally but not by CI. That's a pre-existing gap in the test structure, worth fixing as its own change rather than as a fourth concern here — and it's the kind of blind spot that only became visible after #202 made CI actually run integration tests. ## Testing Full workspace: 178 core + 30 master + 51 server + 5 concurrency + 1 merge-cleanup, all pass. clippy `-D warnings` and fmt clean. ## The complete picture This finishes the line of work from the original report (`/experiments` at 30–43s): | | problem | fix | |---|---|---| | **#207** | write amplification — 100 versions/round | one snapshot commit; **84× fewer versions** | | **#209** | scan cost — full observe of everything, every round | skip unchanged; cold = one open | | **#210** | table size — held every experiment ever created | retire after 7 days idle, drained first | | **this** | retirement made cold experiments invisible | active-N default + registry-backed search | | **#208** | all of the above failing silently | maintenance failure metrics | 🤖 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
2 participants
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.
Follow-up to #207. A failing maintenance pass left no signal at all.
master_stats_versions_removed_totalonly 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
unreclaimed_versionsis the signal to alert on — the version gap since the last successful pass.master_stats_versionis 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 harmlessu64counter. Only manifests still sitting on storage cost anything, and the gap is what measures those.The failure path also logs at
warnwith both numbers, so the reason is visible without a metrics backend.Descriptions
Also describes every
_statsmetric, 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:
MasterStateneeds 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 warningsandcargo fmtclean.🤖 Generated with Claude Code