Add leader proxy circuit breaker - #1132
Conversation
Warning Review limit reached
Next review available in:49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
bootjp
commented
Jul 19, 2026
Implemented the leader-proxy circuit breaker design with caller-cancellation handling, stale-identity result rejection, adapter error mappings, and coordinator caller audit. Verification:
@codex review |
There was a problem hiding this comment.
Code Review
This pull request implements a leader proxy circuit breaker to mitigate retry storms during leader elections or network partitions, introducing the transient ErrLeaderProxyCircuitOpen error and mapping it to appropriate 503 or NOTLEADER responses across DynamoDB, SQS, S3, Redis, and Admin adapters. Feedback on the changes suggests optimizing the circuit breaker's fast-fail path by caching the pre-formatted UTC RFC3339Nano string of the openUntil timestamp, thereby avoiding expensive time.Format allocations on every rejected request when the circuit is open.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:66bab630d5
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
bootjp
commented
Jul 19, 2026
Implemented the current-head review fixes in signed commit d6b1a71. Correctness evidence:
Caller/classifier/mapping audit:
Verification:
The repository-wide go test ./... was also attempted. It encountered broad pre-existing Redis/raft election timing failures under severe host contention (including adapter and internal/raftengine/etcd), while the changed kv package passed in that run and the focused adapter paths pass independently. All nine actionable current-head threads have been replied to and resolved. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:d6b1a71c19
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
bootjp
commented
Jul 19, 2026
Addressed the latest-head P2 findings in signed commit 2266774. Correctness evidence:
Audit:
Verification:
All three current-head P2 threads have been replied to and resolved. @codex review |
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Uh oh!
There was an error while loading. Please reload this page.
This document declares itself the canonical ownership index, so rows that call finished work unimplemented and unowned send contributors off to write a competing design. Seven rows named work that is already an ancestor of this commit: - route catalog delta watch (#1117, 6c7a66e) -> implemented owner - SST ingest snapshot transfer (#1130, 910a97e) -> implemented owner - leader-proxy circuit breaker (#1132, 56e36e9) -> implemented owner - physical snapshot offload (#1131, 764db2d) -> partial owner, which records M0/M1 implemented and still owns M2/M3 - TSO ceiling state machine (#1150, 0e85c82) -> merged, separated from the group-0 issuance work that remains open - autosplit committed-window reader (#1152, afec059) -> M3 is partially implemented, not absent - S3 offload transport and backfill (77ea547, #1126/5eaaa05d) -> the transport path is merged; the focused design names reference counting, GC readiness, and legacy migration as the remaining blockers Each cited commit was confirmed to be an ancestor of this revision, and every referenced design document was confirmed to exist. The completion rule also could not be satisfied by every row: it demanded each implemented row's focused owner be promoted to `implemented`, but the shared Pebble cache row names only PR #1082 as its canonical owner and a pull request has no status to promote. That made the roadmap permanently ineligible for promotion regardless of what shipped. Merged implementation evidence now closes such a row; an open pull request still does not. Claude-Session: https://claude.ai/code/session_013rNHooj7NF3giihWVba8QE
The previous commit marked catalog delta/watch and SST ingest snapshot transfer as implemented in the §4 tables but left them listed in §6 as designs still to be written, so the document contradicted itself. Steps 2 and 4 now name only the open work, using the same "no longer in this list" note §6 already carries for the shared Pebble cache. The §4.4 note under the leader-proxy row had the same problem and was not flagged: it argued the admin ErrLeaderUnavailable mapping is not evidence that the data-plane proxy "has the proposed circuit breaker", while the row directly above it now records that breaker as implemented. Reworded to keep the original point -- the admin mapping was never the evidence -- while recording that PR #1132 closed the gap. Claude-Session: https://claude.ai/code/session_013rNHooj7NF3giihWVba8QE
Author: bootjp
Summary
Safety and risk
Verification
go test ./... -timeout=20mgo test -racefor leader proxy, connection cache, and retry classificationgolangci-lint run ./... --timeout=10mgit diff --check