fix(sdks): windowed degraded detection for pool backoff - #1499
fix(sdks): windowed degraded detection for pool backoff#1499Pangjiping wants to merge 5 commits into
Conversation
Replace the consecutive-failure counter in the sandbox pool reconciler with rate-based detection over a sliding failure window. A successful warmup no longer resets the failure count or cancels an active backoff, so pools with a sustained high failure rate and interleaved successes enter DEGRADED and stay paused until the window drains. - ReconcileState: failures recorded with timestamps, pruned by failureWindow (default 60s, new PoolConfig knob); expired backoff is renewed while the window is still hot; recovery is time-based only - Kotlin, Go, and Python (sync + async) aligned; recordSuccess removed - Regression tests: interleaved success/failure warmup streams now trigger backoff; windowed trigger, renewal, and drain-recovery tests - docs/guides/client-pool.md: document new semantics and config knob (effective from the next SDK release)
github-actions
Bot
added
documentation
![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: 7a62a3c390
ℹ️ 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".
…multiply Address review findings on #1499: - Python: move failure_window to the end of PoolConfig/AsyncPoolConfig fields and SandboxPoolSync/SandboxPoolAsync constructor params so existing positional construction keeps its meaning; add positional regression tests - Go/Kotlin: cap the exponential delay in seconds before building the duration so sustained renewals (backoffAttempts grows unbounded) can no longer overflow time.Duration (Go wraps negative) or Duration capacity (Kotlin throws); Python already caps via unbounded ints, regression test added for consistency
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 359828cd85
ℹ️ 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".
Go snapshot() only compared backoffUntil and returned the cached state, so a DEGRADED pool with no deficit could keep reporting stale DEGRADED and an unpruned failure count after the window drained. Extract refreshLocked (prune/renew/recover) shared by shouldBackoff and snapshot, and reorder Kotlin/Python snapshots to advance the state machine before reading state, failure count, and last error so all languages stay aligned.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce74346762
ℹ️ 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".
Kotlin's SandboxPool.builder() wrapper exposed degradedThreshold but not the new failureWindow knob, so Kotlin/Java callers could not set it via the normal pool builder. Add the pass-through setter plus a forwarding regression test.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be97122ad5
ℹ️ 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".
DefaultSandboxPool.Snapshot() discarded the health state refreshed by reconcileState.snapshot() (which prunes the window and runs recovery on read) and returned the cached p.healthState instead, so it could pair stale DEGRADED with fresh failureCount/backoffActive after the window drained. Use the refreshed value so Go matches Kotlin/Python snapshot behavior. Regression test drives DEGRADED then drains the window and asserts the snapshot self-recovers.
1 participant
Summary
The sandbox pool reconcile loop detected degradation with a consecutive-failure counter that any successful warmup reset to zero (
recordSuccesszeroedfailureCount,backoffAttempts, and cancelled the active backoff window). Under a sustained high failure rate with interleaved successes (e.g. ~50 failures/s + ~9.4 successes/s), the count could never reachdegradedThreshold, soDEGRADED/backoff never triggered and the pool churned for hours.This PR replaces the counter with rate-based detection over a sliding time window:
failure_windowknob (default 60s)degradedThresholdnow means failures inside the window, not consecutive failuresImplemented identically in Kotlin, Go, and Python (sync + async);
recordSuccessremoved in all languages. Docs updated (docs/guides/client-pool.md) noting the change takes effect from the next SDK release.Tests
./gradlew spotlessApply :sandbox:test :code-interpreter:test✅go test ./...✅uv run pytest tests/+ ruff + pyright ✅pnpm docs:build✅New regression tests (fail under the old counter logic):
SandboxPoolTest, GoTestReconciler_InterleavedCreatesTriggerDegraded, Pythontest_interleaved_creates_trigger_degraded_backoff)Behavior change (public)
failure_windownew config (default 60s) in all three SDKsdegradedThresholdsemantics: consecutive → windowed