Repository navigation
fix(master): report pool permits at acquire/release, split claiming from running - #348
Merged
beinan merged 1 commit intoOct 8, 2026
Conversation
…rom running The gauge added in lance-format#339 was set once per poll iteration, before any permit was taken. A claim that blocks on etcd for minutes - the exact situation the gauge exists to show - kept reporting the pre-claim value, which can be zero. Update at the permit boundaries instead and label by holder: `state="claiming"` while the etcd claim is outstanding, `state="running"` from task spawn until the permit drops. The runbook row for "tasks waiting" now distinguishes capacity (running at cap), slow admission (claiming high) and nothing eligible (both low). Refs lance-format#333. Review finding on lance-format#339. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
17 of 18 tasks
beinan
added a commit
that referenced
this pull request
Oct 8, 2026
Part of #333. Fixes review finding **5 (panic path)** on #348 (credit: production review). ## Problem `running.increment` before spawn, `running.decrement` after `run_task` returns. A panic in `run_task` unwinds past the decrement: permits are released by `Drop`, the gauge is not, so `master_task_pool_in_use{state="running"}` reads as consumed forever. ## Fix `GaugeHold(Gauge)` with `Drop` → decrement, held across `run_task`. Three lines. ## Validation Type-level: the decrement is now in `Drop`, which runs on both normal return and unwind. No new test — asserting on a `metrics::Gauge` value needs a recorder harness this crate's tests do not have; the existing poller test (`reserved_resident_merge_finishes_while_legacy_pool_is_saturated`) passes against etcd, clippy `-D warnings` and fmt clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <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.
Part of #333. Fixes a defect in #339 found in review (credit: production review).
Problem
master_task_pool_in_usewas set once per poll iteration before any permit was taken. A claim blocked on etcd for minutes — the case the gauge is for — kept showing the pre-claim value, possibly zero.Fix
Gauge updated at the permit boundaries, labelled by holder:
state="claiming"during the etcd claim,state="running"from task spawn until permit drop. Runbook row updated: running at cap ⇒ capacity; claiming high ⇒ slow/blocked admission scan; both low ⇒ nothing eligible.Validation
Clippy
-D warnings, fmt, typos clean;reserved_resident_merge_finishes_while_legacy_pool_is_saturatedpasses locally against etcd (exercises the poller path).🤖 Generated with Claude Code