fix(pools): wake waiters when failed borrowers free capacity - #212
Merged
Conversation
abnegate
requested review from
ArnabChatterjee20k and
eldadfux
as code owners
September 6, 2026 00:52
GitHub 401s installation tokens when iss is the string form of an App ID. Keep Client IDs as strings. Retry GitLab OAuth bootstrap instead of exiting on the first empty token so compose --wait can recover.
The linked VCS job authenticates with a non-numeric issuer that GitHub 401s. Classify Iv1 Client IDs without printing the secret.
The first cut of this fix gave the adapter a reason. Adapter::notify() was documented as "capacity is freed without an idle resource", and pop() answered with a Wakeup::Capacity sentinel that the pool matched on — so an adapter, whose whole job is holding idle resources and serialising access, had to carry the pool's capacity vocabulary, and every adapter that never blocks still inherited a concept it could not act on. An adapter does not need to know why. It needs to release whoever is parked, and let them re-read the real state for themselves. Adapter::unblock() says only "it is worth looking again"; pop() goes back to answering "here is one" or "not now"; Wakeup is deleted. The pool keeps the capacity rules that were always its own, and retries within the SAME deadline, so looping never extends the budget a caller asked for. push() releases a parked caller too, because a returned resource is also a state change a waiter is waiting on, and one primitive should cover both. Signals stay coalesced to one, so a caller that takes a resource passes the baton on while any remain idle — otherwise a burst of returns arriving with nobody parked would release a single caller and leave the rest waiting beside idle resources. Co-Authored-By: Claude Opus 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.
A request already waiting for a pooled connection currently times out when another caller discards its failed connection, even though capacity is now free. The same happens when a concurrent connection initializer fails. This occurs in Cloud regional HTTP clients after a transport exception and affects both pools 2.0.2 and 2.1.1.
Notify blocked Swoole acquisitions when capacity changes without returning an idle resource, then recheck capacity under the original acquisition deadline. Notifications use a separate coalesced channel and do not inflate idle counts, create replacement connections during destruction, retry the failed caller, or replay an HTTP request. Existing adapters retain their default behavior. Returning a connection also serializes its ownership handoff: a waiting borrower can resume inline during channel notification, and must not have its new active checkout removed by the previous borrower.
Validation: the public discard/creation waiter regressions and three-borrower ownership regression were each observed failing without their fixes; the full package suite now passes (97 tests, 388 assertions), including active/idle/capacity accounting, competing waiters and unchanged timeout budgets.
bin/monorepo check poolsandbin/monorepo validatepass. Cloud's public regional-client regression remains red on its locked dependency until this exact source passes canonical CI and is published through the documented Split Dev workflow. No consumer vendor files were patched.CI prerequisite under investigation: the linked VCS job fails GitHub App authentication before remote fixture work. A temporary CI-only preflight now reports only credential-shape booleans, JWT issuer type/equality, and fixed read-only GET /app status with an allowlisted error category. It never logs tokens, identifiers, keys or raw responses; the original E2E command remains intact. This diagnostic must be removed, or replaced with a separately justified tested permanent gate, before the final source handoff. No dependency publication or consumer propagation has occurred.