Repository navigation
Reuse the HTTP connection pool for remote plugin requests - #49100
Conversation
## Why Each call to `PluginsConfigInput::remote_plugin_service_config` created a new HTTP client pool, preventing connection reuse across service configurations. ## What changed Share one lazily initialized, route-aware pool across a `PluginsConfigInput` and its clones. Newly constructed inputs retain separate pools, while service configurations continue to use the current endpoint and product SKU and requests use the supplied authentication. ## Testing Add tests for lazy initialization, pool sharing across repeated calls and early clones, isolation between new inputs, and requests using updated endpoints, product SKUs, and authentication with the shared pool. GitOrigin-RevId: 3e929c34052798779a4f153e4db03b3f75b6f87d
cde1bfc to
bfdb157
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cde1bfca54
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| chatgpt_base_url, | ||
| http_client_factory, | ||
| product_sku, | ||
| remote_http_clients: Arc::new(OnceLock::new()), |
There was a problem hiding this comment.
Config::plugins_config_input() calls PluginsConfigInput::new every time (codex-rs/core/src/config/mod.rs:1716-1725), and normal paths such as the app-server helper immediately call config.plugins_config_input().remote_plugin_service_config() (codex-rs/app-server/src/request_processors/plugins.rs:159-160). Initializing a fresh Arc<OnceLock<_>> here therefore creates a fresh pool for each ordinary remote-plugin operation; reuse occurs only when a caller happens to retain or clone one PluginsConfigInput. Keep the pool in configuration/session-level state (resetting it on a real config change) so sequential production requests can actually reuse connections.
Useful? React with 👍 / 👎.
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
GitHub Actions
Inactive
Reuse the HTTP connection pool for remote plugin requests
Why
Each call to
PluginsConfigInput::remote_plugin_service_configcreated a new HTTP client pool, preventing connection reuse across service configurations.What changed
Share one lazily initialized, route-aware pool across a
PluginsConfigInputand its clones. Newly constructed inputs retain separate pools, while service configurations continue to use the current endpoint and product SKU and requests use the supplied authentication.Testing
Add tests for lazy initialization, pool sharing across repeated calls and early clones, isolation between new inputs, and requests using updated endpoints, product SKUs, and authentication with the shared pool.