feat(conductor): add per-provider inflight blocking queue#4032
feat(conductor): add per-provider inflight blocking queue#4032francomascareloai wants to merge 1 commit into
Conversation
Problem: under burst 403/quota conditions a provider could receive
unbounded concurrent upstream calls, exhausting credentials and
amplifying failures (thundering herd into a saturated provider).
Solution: add a per-provider inflight limiter with a bounded blocking
queue. Each provider key gets a buffered channel semaphore of size
MaxInflightPerProvider. Requests acquire a slot before dispatching to
the upstream executor; when the semaphore is full, new requests block
up to MaxQueueWaitMS (then return 503 provider_backpressure, retryable)
instead of piling onto a saturated provider.
Config keys (under `provider-resilience`):
- max-inflight-per-provider: caps concurrent upstream calls per
provider key. 0 disables the limiter entirely (default: 4).
Clamped to [0, 10000].
- max-queue-wait-ms: max time a request waits for a permit before
being rejected with 503 provider_backpressure. 0 waits until ctx
cancel (default: 30000 = 30s). Clamped to [0, 300000] (5 min).
Wiring: the limiter is acquired in all three execute*MixedOnce paths
(non-stream, count-tokens, stream) plus the antigravity credits
fallback (Execute and ExecuteStream). The streaming paths transfer
permit ownership to a wrapper goroutine that releases when the stream
completes, the client disconnects (ctx cancel), or the upstream
closes the channel — with a drain-on-cancel guard to prevent producer
goroutine leaks.
Design notes:
- The mutex is released BEFORE the blocking select to avoid
head-of-line blocking across providers.
- releaseFunc is idempotent (sync.Once) so a double-release cannot
steal a different acquirer's token.
- Client ctx cancellation returns ctx.Err() and does NOT inflate
the backpressure counter (only genuine queue timeouts do).
- Panic guards release the permit and re-panic so the framework
recover still handles it.
Metrics (ResilienceMetricsSnapshot):
- cliproxy_provider_backpressure_reject_total
- cliproxy_provider_queue_wait_ns_sum
- cliproxy_conductor_permit_total
- cliproxy_conductor_permit_ns_sum
Tests: conductor_resilience_test.go covers blocking queue semantics,
queue-wait timeout, ctx-cancel unblock (no backpressure inflation),
no head-of-line blocking across providers, streaming release on
channel close, streaming wrapper drain on inner cancel (goroutine
leak guard), releaseFunc idempotency, disabled/nil-manager safety,
metrics snapshot, and a concurrent throughput benchmark.
Co-Authored-By: Claude <noreply@anthropic.com>
|
This pull request targeted The base branch has been automatically changed to |
There was a problem hiding this comment.
Code Review
This pull request implements per-provider inflight request limiting (Provider Resilience) using a bounded blocking queue based on buffered channels. It introduces new configuration parameters, integrates permit acquisition and release across execution and streaming paths, and adds comprehensive tests to verify the resilience behavior. The feedback suggests optimizing the permit acquisition logic by avoiding unnecessary context allocations when no queue timeout is configured.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| var waitCtx context.Context | ||
| var cancel context.CancelFunc | ||
| if pr.MaxQueueWaitMS > 0 { | ||
| waitCtx, cancel = context.WithTimeout(ctx, time.Duration(pr.MaxQueueWaitMS)*time.Millisecond) | ||
| } else { | ||
| waitCtx, cancel = context.WithCancel(ctx) | ||
| } | ||
| defer cancel() |
There was a problem hiding this comment.
When MaxQueueWaitMS is 0 (wait forever), creating a cancelable child context via context.WithCancel(ctx) is unnecessary and introduces allocation and parent-child registration overhead. Instead, you can use the parent ctx directly when no timeout is configured.
| var waitCtx context.Context | |
| var cancel context.CancelFunc | |
| if pr.MaxQueueWaitMS > 0 { | |
| waitCtx, cancel = context.WithTimeout(ctx, time.Duration(pr.MaxQueueWaitMS)*time.Millisecond) | |
| } else { | |
| waitCtx, cancel = context.WithCancel(ctx) | |
| } | |
| defer cancel() | |
| var waitCtx context.Context | |
| if pr.MaxQueueWaitMS > 0 { | |
| var cancel context.CancelFunc | |
| waitCtx, cancel = context.WithTimeout(ctx, time.Duration(pr.MaxQueueWaitMS)*time.Millisecond) | |
| defer cancel() | |
| } else { | |
| waitCtx = ctx | |
| } |
Summary
Adds a per-provider inflight request limiter with a bounded blocking queue. When concurrent requests to a single provider exceed the configured limit, additional requests wait for a slot (up to a configurable timeout) instead of being immediately rejected with HTTP 503
provider_backpressure.This prevents burst-driven upstream rate-limiting (e.g. 403 "access paused" / 429) when many concurrent subagents hit the same provider simultaneously.
Problem
When N concurrent requests target the same upstream provider and exceed the provider's real concurrency tolerance, the proxy forwards all of them and the provider responds with
403 access paused/429— which can force credential revocation and disrupt all users of that provider. There is currently no backpressure mechanism that absorbs short bursts.Solution
A per-provider counting semaphore (
chan struct{}of capacityMaxInflightPerProvider):MaxInflightPerProvider == 0→ no-op).selectwith a boundedcontext.WithTimeout(MaxQueueWaitMS). On timeout, returns a retryable*auth.Error(HTTP 503,Retryable: true).defer, which would leak acrosscontinuein the failover loop). Panic-recover is iteration-scoped via an IIFE closure that releases the permit and re-panics.Chunks. Both the inner and outerctx.Donebranches drainoriginalChunksto avoid blocking the upstream producer on client disconnect mid-stream.releaseFuncusessync.Onceso a double-release never steals a token from a different acquirer.New config keys
provider-resilience.max-inflight-per-provider40disables the limiter (no-op fast path). Clamped to[0, 10000].provider-resilience.max-queue-wait-ms300000= wait forever (not recommended). Clamped to[0, 300000].Metrics
Exposed via
Manager.ResilienceMetricsSnapshot():cliproxy_provider_backpressure_reject_totalcliproxy_provider_queue_wait_ns_sumcliproxy_conductor_permit_attempts_totalcliproxy_conductor_permit_ns_sumTests
New file
sdk/cliproxy/auth/conductor_resilience_test.go(645 lines, 9 tests, all pass with-race):TestBlockingQueuePerProvider— N goroutines, limit K: K acquire immediately, N-K block, release propagates.TestQueueWaitTimeout— 2nd request gets 503 aftermax-queue-wait-ms.TestCtxCancelUnblocksWaiter— cancelled waiter returnsctx.Err(), no goroutine leak.TestNoHeadOfLineBlocking— saturated provider A does not block provider B (<50ms).TestStreamingReleaseOnChannelClose— permit released whenChunkscloses.TestStreamingWrapperDrainsOnInnerCancel— innerctx.DonedrainsoriginalChunks(no producer deadlock on mid-stream cancel).TestReleaseFuncIdempotent— double-release does not steal another acquirer's token (len(limiter)stays 1).TestAcquireProviderPermitDisabled/ nil-safety — no-op whenMaxInflightPerProvider == 0or nil manager/cfg.Scope (intentionally narrow)
This PR contains only the inflight blocking queue. It does not include:
Validation
go vet ./internal/config/... ./sdk/cliproxy/auth/...→ clean.go test ./sdk/cliproxy/auth/ -race→ 9/9 PASS.go build ./cmd/server→ success (56MB binary).trading-systems-reviewer→FEEDBACK_CONSENSUS: PASS(0 HIGH/CRITICAL, 1 LOW naming nit resolved: metric renamedconductor_permit_total→conductor_permit_attempts_totalto reflect it counts attempts).Files
internal/config/config.go(+37) —ProviderResilienceConfig, constants, defaults, clamping.sdk/cliproxy/auth/conductor.go(+314/-5) —acquireProviderPermit,releaseFunc,ResilienceMetricsSnapshot, 5 acquire sites, 2 streaming wrappers.sdk/cliproxy/auth/conductor_resilience_test.go(new, 645) — 9 tests.Backward compatibility
Default behavior is unchanged:
MaxInflightPerProviderdefaults to4, but operators who do not setprovider-resiliencein config get the code default. Settingmax-inflight-per-provider: 0fully disables the limiter (no-op fast path, zero channel ops).