Repository navigation
feat(llm): add shared request pacing and cooldown - #418
cpakkamisaac-sae wants to merge 1 commit into
Conversation
Signed-off-by: Clement Pakkam Isaac <cpakkamisaac@nvidia.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (10)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughAdds optional shared request pacing and capped overload cooldown to local admission groups and the broker. Structured Retry-After values from supported provider errors can update cooldown deadlines before permit release. The change also adds configuration validation, protocol updates, documentation, and tests. ChangesShared admission control
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant Provider
participant UnifiedLLM
participant retry_after_delay
participant CooldownAdmissionPermit
participant AdmissionGroup
participant QueuedRequest
Provider->>UnifiedLLM: Return provider error
UnifiedLLM->>retry_after_delay: Parse structured Retry-After
retry_after_delay-->>UnifiedLLM: Return delay or None
UnifiedLLM->>CooldownAdmissionPermit: Publish cooldown delay
CooldownAdmissionPermit->>AdmissionGroup: Extend shared admission deadline
AdmissionGroup-->>QueuedRequest: Grant after deadline and available capacity
Merge Risk: ⚪ Minimal · up to No actionable issue remains in the reviewed change; it is ready for normal merge checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Closes #417
Builds on #349 without changing its default concurrency-only behavior.
Why
An active-call ceiling does not necessarily control attempt frequency. Fast requests and independent retries can exceed a provider's request quota even while respecting
max_in_flight. Applications can now opt into shared timing policy at the existing provider-attempt boundary.Changes
requests_per_secondminimum-spacing admission andmax_cooldownbounded, sharedRetry-Afterfeedback to process-local groups and the application-owned host-local broker. Both default toNone.CooldownAdmissionPermitprotocol.Validation
Completed locally on current main (
564a3401) before opening the issue or PR:ResourceWarningandPytestUnraisableExceptionWarningtreated as errors. These include 23 real loopback HTTP end-to-end cases using existing Chat and Responses transports, not mocked provider calls.The complete
unifiedllm.pytype check retains an existing cache-mapping annotation error, reproduced on unchanged main. The shared pacing change does not touch that code; the generic Pyright pre-commit hook was checked separately rather than claimed green.Focused reproduction:
Against the synthetic rate quota, six logical calls through either HTTP transport produced:
The paced run takes about 1.26 seconds; the baseline stops quickly after exhausted retries. This demonstrates completion/retry behavior for this configured fixture, not a general production performance claim. SDK retries are disabled in these comparisons.
Boundaries
Pacing controls NOOA admission grants, not exact remote wire counts or tokens. Already granted/offered leases are not revoked by later feedback. A per-report cooldown cap can resume before the provider's requested delay; this is documented as an application choice. The broker remains host-local; multi-host coordination, adaptive concurrency, queue-size limits, and production gateway validation are separate work. No new dependencies.