Make queue refills single-flight on the caller's connection - #103
Merged
wasabipesto merged 1 commit intoAug 19, 2026
Merged
Conversation
Every request that observed a low queue launched its own bulk claim, and each of those refills checked out a second pool connection while its request held the first. Under fleet load that meant 4-6 identical refills landing together - the niceonly queue was observed at 1,226 against the 250 a single refill can reach - and sampling pg_stat_activity live showed the resulting bursts saturating the 10-connection pool in 10-20s stretches about once a minute, fast-failing unrelated requests (including /submit) into 5s-timeout 503s at ~20/minute. Two changes, one per mechanism: - A per-queue refill gate (std Mutex + try_lock). The winner refills; contenders skip straight to popping rather than waiting. The winner re-checks the queue depth under the gate, so a thread that observed a low queue but won the gate after someone else already refilled does not refill again. A poisoned gate is treated as contended: skip, and let the fallback serve. - Refills borrow the requesting handler's connection instead of checking out a second one. A refilling request now holds one connection, not two. The startup prefills keep their own checkout; they run before any traffic exists to contend with. Why skipping (not waiting) is safe now: a request that finds the queue empty while a refill is in flight falls through to the direct claim paths, which are all chunk-scoped or indexed since wasabipesto#100/wasabipesto#102 and cost tens of milliseconds. When this trade-off was last evaluated (issue wasabipesto#98) the empty-queue fallback was an unindexed table walk, which is why a try_lock gate was rejected then and is fine now. Tests are DB-gated unit tests in the api crate (same NICE_TEST_DATABASE_URL convention as common/tests/claim_queries.rs), and both properties red/green against the pre-fix behaviors: - 8 barrier-synced threads hit an empty queue with the pool sized exactly to their held connections. Exactly one bulk claim must run, asserted in the database (rows stamped) and the queue-depth ceiling. Removing the gate fails it (herd), and reverting the refill to a second checkout fails it differently (starves against the exhausted pool, 0 rows claimed). - A sequential drain past the threshold, asserting claim ordering and exactly two bulk claims for 252 pops. Co-Authored-By: Claude Opus 5 (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
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.
Fixes the remaining pool-exhaustion 503s (issue #98 §5 plus a mechanism the issue didn't list). Before building this I re-verified against production that it still targets the live problem — all numbers below are from today.
The problem, measured live
With #100–#102 deployed,
/claim/detailedworks and theNextscans are gone, but the API still fast-fails ~20 requests/minute (up from ~10 two days ago as the fleet grew): 126 × 503 in a 6.4-minute window, every one at exactlyelapsed_ms=5000— the pool-checkout timeout. They hit/submitand both claim endpoints indiscriminately, which is the signature of the pool being the bottleneck, not any endpoint.Sampling
pg_stat_activityat 0.6 s intervals shows what holds the pool: sustained 10–20 s stretches of 5–6 concurrenteligible_chunkqueries (the Thin bulk-refill/fallback shape), recurring about once a minute. Two mechanisms produce that fromfield_queue.rs:At the current drain rates (niceonly ~52/s, detailed-thin ~10/s) the queues cross their refill thresholds every 3–10 seconds, so these two mechanisms fire constantly.
The fix
std::sync::Mutex<()>+try_lock). The winner refills; contenders skip straight to popping rather than waiting. The winner re-checks the queue depth under the gate, so a thread that observed a low queue but won the gate after someone else already refilled doesn't refill again (this also makes the tests deterministic). Poisoned gate = contended: skip.claim_niceonly/claim_detailed_thinnow take&mut PgPooledConnection; both call sites already hold one. The startup prefills keep their own checkout — they run before any traffic exists to contend with.No schema changes, no client-visible changes, same bind patterns.
Why skipping the gate (not waiting) is safe now
Issue #98 evaluated exactly this
try_lockshape and warned against it: at the time, an empty detailed queue fell through to an unindexed table walk, so 1.2%-of-samples-empty was expensive. That objection is obsolete — since #100/#102 every empty-queue fallback is chunk-scoped or partial-indexed and costs tens of milliseconds. A request that loses the gate mid-refill gets a fast direct claim, not a scan. Refill capacity (200 fields/~50 ms niceonly, 100/~65 ms detailed) exceeds observed drain by two orders of magnitude, so empty windows stay rare and brief.Tests, and their red/green evidence
DB-gated unit tests in the api crate (same
NICE_TEST_DATABASE_URLconvention ascommon/tests/claim_queries.rs; diesel added as a dev-dependency for fixtures). The core test: 8 barrier-synced threads hit an empty queue with the pool sized exactly to their held connections (held across the whole window via a second barrier, like real in-flight requests). Exactly one bulk claim may run, asserted both in the database (rows stamped) and against the queue-depth ceiling.Both pre-fix behaviors fail it, differently:
left != 200).left: 0), after the checkout timeout.Plus a sequential drain test (252 pops → exactly 2 bulk claims, claim order preserved) and the detailed queue's own gate. The
claim_queriessuite passes unchanged.What this should do in production
The once-a-minute saturation stretches were: refill herd (4–6 queries) + doubled checkouts + normal traffic > 10 connections. After this, the same moment costs one refill on an already-held connection. I'd expect the
elapsed_ms=5000503s to drop to zero at current traffic;/statusqueue sizes should stop overshooting 250/150 (any reading abovethreshold + refill amountafter deploy would mean the herd is back).Not addressed here, deliberately: the
Randomstrategy's slow tail (pivots landing in the oversized-range_sizecl=0 region scan up to 36 s) — separate PR, as discussed.Deploy: build + up, nothing else.
🤖 Generated with Claude Code