feat(core): add per-worker resilience and HTTP pool config types - #799
Conversation
Add foundational types for per-worker resilience configuration: - `HttpPoolConfig` and `ResilienceUpdate` in protocol layer (WorkerSpec) following the existing `HealthCheckUpdate` PATCH-style pattern - `ResolvedResilience` in model_gateway for merging router defaults with per-worker overrides at worker construction time - `build_worker_http_client()` for constructing isolated per-worker HTTP clients with TLS support from RouterConfig Signed-off-by: Chang Su <chang.s.su@oracle.com>
📝 WalkthroughWalkthroughThis PR extends the worker configuration system with per-worker HTTP pool and resilience settings. New Changes
Sequence Diagram(s)sequenceDiagram
participant WS as WorkerSpec
participant HC as HTTP Client Builder
participant RC as Resilience Config
participant C as Client
participant RR as ResolvedResilience
WS->>HC: http_pool: HttpPoolConfig
WS->>RC: resilience: ResilienceUpdate
HC->>HC: Merge HttpPoolConfig<br/>with RouterConfig defaults
HC->>HC: Apply TLS certificates<br/>(if configured)
HC->>C: Build reqwest::Client
RC->>RR: resolve_resilience(<br/>overrides)
RR->>RR: Merge retry settings<br/>with base config
RR->>RR: Merge circuit breaker<br/>with base config
RR->>RR: Compute enabled flags
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request lays the groundwork for enhanced configurability within the system by introducing new data structures and utility functions. It enables future capabilities for individual workers to define their own HTTP connection pooling and resilience parameters, such as retry mechanisms and circuit breaker settings, overriding global router defaults. The changes are purely structural and additive, ensuring backward compatibility and setting the stage for more granular control over worker behavior. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces foundational types for per-worker resilience and HTTP pool configuration, including HttpPoolConfig and ResilienceUpdate. It also adds utility functions to build per-worker HTTP clients and resolve resilience settings by merging router-level defaults with worker-specific overrides. The changes are well-structured and include good test coverage. My review includes a suggestion to enhance the flexibility of timeout configurations by allowing them to be disabled, which would be a valuable addition. This approach aligns with best practices for using Option in builder patterns.
![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: 17638fa900
ℹ️ 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".
| max_retries: overrides.max_retries.unwrap_or(base_retry.max_retries), | ||
| initial_backoff_ms: overrides | ||
| .initial_backoff_ms | ||
| .unwrap_or(base_retry.initial_backoff_ms), | ||
| max_backoff_ms: overrides | ||
| .max_backoff_ms | ||
| .unwrap_or(base_retry.max_backoff_ms), | ||
| backoff_multiplier: overrides | ||
| .backoff_multiplier | ||
| .unwrap_or(base_retry.backoff_multiplier), | ||
| jitter_factor: overrides.jitter_factor.unwrap_or(base_retry.jitter_factor), |
There was a problem hiding this comment.
When base_retry_enabled is false and the worker leaves disable_retry unset, this still copies base_retry.max_retries into resolved.retry. The existing retry path only consumes a RetryConfig (RouterConfig::effective_retry_config() in model_gateway/src/config/types.rs disables retries by forcing max_retries = 1, and routers pass that config straight into RetryExecutor), so wiring resolved.retry into the current router/worker stack will silently re-enable retries for deployments that set disable_retries: true unless every call site remembers to special-case retry_enabled.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not an issue. The caller passes base_retry from RouterConfig::effective_retry_config(), which already sets max_retries = 1 when disable_retries: true. Additionally, the upcoming execute_with_resilience (next PR in the series) checks retry_enabled before entering the retry loop — that flag is the gatekeeper, not max_retries. The two mechanisms are complementary by design.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@model_gateway/src/core/resilience.rs`:
- Around line 42-49: The resolve_resilience function currently uses
DEFAULT_RETRYABLE_STATUS_CODES when overrides.retry.retryable_status_codes is
None, which bypasses the router-level default; update resolve_resilience so that
when ResilienceUpdate (overrides) has retry.retryable_status_codes == None it
uses base_retry.retryable_status_codes.clone() (the router default) instead of
DEFAULT_RETRYABLE_STATUS_CODES, and only fall back to
DEFAULT_RETRYABLE_STATUS_CODES if both override and base_retry have no value;
apply the same fix wherever the code currently defaults straight to
DEFAULT_RETRYABLE_STATUS_CODES (e.g., the retry handling around
retryable_status_codes) so router defaults are respected (refer to function
resolve_resilience, type RetryConfig, and field retryable_status_codes).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 50be1a06-b8f7-4114-9155-319e9d366555
📒 Files selected for processing (4)
crates/protocols/src/worker.rsmodel_gateway/src/core/http_client.rsmodel_gateway/src/core/mod.rsmodel_gateway/src/core/resilience.rs
…-project#799) Signed-off-by: Chang Su <chang.s.su@oracle.com>
…-project#799) Signed-off-by: Chang Su <chang.s.su@oracle.com>
2 participants
Description
Problem
The model gateway has global retry, circuit breaker, and HTTP client config — all workers share the same settings. Workers cannot have per-worker overrides for resilience behavior or connection pools, creating implicit coupling and preventing fine-grained tuning.
Solution
Add foundational config types for per-worker resilience. Protocol layer gets flat optional config types (
HttpPoolConfig,ResilienceUpdate) following the existingHealthCheckUpdatePATCH-style pattern. Runtime layer resolves them against router defaults at worker creation time viaResolvedResilience. A per-worker HTTP client builder isolates connection pools per worker.This is the first PR in the per-worker resilience refactor series — types only, no behavior changes, fully backwards compatible.
Changes
HttpPoolConfigandResilienceUpdatestructs toWorkerSpecin the protocol layer (crates/protocols/src/worker.rs)ResolvedResiliencestruct andresolve_resilience()function inmodel_gateway/src/core/resilience.rsbuild_worker_http_client()utility inmodel_gateway/src/core/http_client.rsmodel_gateway/src/core/mod.rsTest Plan
cargo test -p openai-protocol— all 36 tests pass (no regressions)cargo test -p smg --lib core::resilience— 7 new tests covering default resolution, retry/CB overrides, disable flags, custom retryable codescargo test -p smg --lib core::http_client— 2 new tests for default and overridden client constructioncargo test -p smg --lib— all 435 tests pass, 0 failuresChecklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspassesSummary by CodeRabbit