Add opt-in upstream pooling and account capacity limits - #29
Merged
Merged
Conversation
Own bounded cookie-free clients in the ASGI lifespan and release account-scoped leases on completion, failover and cancellation. Preserve strict routes, free-first admission and existing replay rules. Expose compatible defaults through CLI, environment and WebUI settings. Validate all 51 backend test scripts, real loopback TCP reuse, and isolated native deployment with real client disconnects. Keep version 1.2.6.
Reviewer's GuideAdds opt-in trusted-origin HTTP connection pooling and cancellation-safe, per-process account capacity leases, integrated into credential routing and request ownership while preserving existing proxy, timeout, retry, failover, and routing behavior; configuration, metrics, documentation, and comprehensive resource tests are included. Flow diagram for per-account capacity admission and releaseflowchart TD
A[Generation request] --> B[Select credential within existing routing rules]
B --> C{Account capacity available?}
C -- No --> D[503 credential_concurrency_limit]
C -- Yes --> E[Acquire CredentialLease]
E --> F[Perform inference and failover if needed]
F --> G{Request completes, cancels, or rotates account?}
G --> H[release_credential]
H --> I[Capacity available again]
C -. Limit 0 .-> E
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="app/settings.py" line_range="46-49" />
<code_context>
minimum=0, maximum=10),
"retry_write_timeout": _item(False, "boolean", "写超时参与重放",
env="CODEBUDDY2API_RETRY_WRITE_TIMEOUT"),
+ "upstream_keepalive": _item(False, "boolean", "上游连接复用", mode="restart",
+ env="CODEBUDDY2API_UPSTREAM_KEEPALIVE"),
+ "max_inflight_per_account": _item(0, "integer", "单账号在途上限（0 不限制）",
+ env="CODEBUDDY2API_MAX_INFLIGHT_PER_ACCOUNT", minimum=0, maximum=10000),
"audit_max_bytes": _item(256 * 1024 * 1024, "integer", "审计明细预算", minimum=1024**2, maximum=1024**4),
"audit_retention_days": _item(30, "integer", "审计明细保留天数", minimum=1, maximum=36500),
</code_context>
<issue_to_address>
**nitpick:** .env.example is not updated with CODEBUDDY2API_UPSTREAM_KEEPALIVE or CODEBUDDY2API_MAX_INFLIGHT_PER_ACCOUNT, so the environment template does not document the two newly exposed runtime settings despite the PR claiming the template is synchronized.
**Triggers:** When operators configure the service from the supplied environment template.
**Suggested fix:** Add both environment variables, with their false/0 defaults, to `.env.example`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. If the pooled client or request-scoped lease handling is wrong, authentication headers or upstream connection state could be exposed across requests, or valid inference traffic could be rejected until capacity is released; requests already sent or data exposed would not be undone by reverting. Reverting would stop the new pooling and capacity behavior for future requests, but would not repair those already-occurred effects.
Comment on lines
+46
to
+49
| "upstream_keepalive": _item(False, "boolean", "上游连接复用", mode="restart", | ||
| env="CODEBUDDY2API_UPSTREAM_KEEPALIVE"), | ||
| "max_inflight_per_account": _item(0, "integer", "单账号在途上限（0 不限制）", | ||
| env="CODEBUDDY2API_MAX_INFLIGHT_PER_ACCOUNT", minimum=0, maximum=10000), |
There was a problem hiding this comment.
nitpick: .env.example is not updated with CODEBUDDY2API_UPSTREAM_KEEPALIVE or CODEBUDDY2API_MAX_INFLIGHT_PER_ACCOUNT, so the environment template does not document the two newly exposed runtime settings despite the PR claiming the template is synchronized.
Triggers: When operators configure the service from the supplied environment template.
Suggested fix: Add both environment variables, with their false/0 defaults, to .env.example.
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.
Changes
Verification
Rollback
Set the account limit to 0; disable keepalive and restart. Before downgrading source, remove new CLI options and restore a compatible control-store backup without the new persisted keys.
Summary by Sourcery
Enable opt-in upstream connection pooling and enforce account-level inference capacity limits without changing the default routing or request behavior.
New Features:
Enhancements:
Deployment:
Documentation:
Tests: