[8.8.0] Remote: add opt-in recovery for the failure circuit breaker - #30294
Open
eaugene wants to merge 1 commit into
Open
[8.8.0] Remote: add opt-in recovery for the failure circuit breaker#30294eaugene wants to merge 1 commit into
eaugene wants to merge 1 commit into
Conversation
github-actions
Bot
added
team-Remote-Exec
eaugene
marked this pull request as draft
July 16, 2026 02:11
Member
|
@eaugene Let's merge to the master |
Contributor
Author
|
Sure @iancha1992 . I'll hold off till #30260 is merged to master |
…azelbuild#30260) The failure circuit breaker (`--experimental_circuit_breaker_strategy=failure`) only transitions `ACCEPT_CALLS -> REJECT_CALLS`; it never uses the half-open `TRIAL_CALL` state that `Retrier.CircuitBreaker` defines and `Retrier` already handles. Once tripped it rejects every remaining remote call for the build, so a burst of transient cache errors disables the remote cache for the rest of the invocation. Under `--remote_download_minimal` (Build without the Bytes) this is unrecoverable: breaker-rejected prefetch reads are converted to `CacheNotFoundException` and treated as lost inputs, action rewinding re-runs the generating actions, and the still-open breaker rejects those re-executions too. Implements `TRIAL_CALL` half-open recovery in `FailureCircuitBreaker`, gated behind a new `--experimental_remote_circuit_breaker_recovery_delay` (`Duration`, default `0` = today's permanent-open behaviour, so nothing changes unless it is enabled). The breaker becomes a small CAS-driven state machine (`CLOSED`/`OPEN`/`RECOVERING`/`PROBING`): after tripping, a single probe is scheduled for after the recovery delay; `state()` hands `TRIAL_CALL` to exactly one in-flight caller; a successful probe closes the breaker and starts a fresh window while a failed probe re-opens it and reschedules. The `State` each call observed is passed into `recordSuccess`/`recordFailure` so only the probe can resolve itself, and stale window-decrement tasks are invalidated via a generation counter. Backport of bazelbuild#30260 to the release-8.8.0 baseline, adapted where 8.8.0 predates master prerequisites: - `RemoteOptions` values are read as public `@Option` fields (8.8.0 predates the getter refactor); the new recovery-delay option is added in that style. - The min-fail-count option (bazelbuild#30178) is absent on 8.8.0, so its constructor parameter, trip-guard term, and tests are omitted; the breaker keeps 8.8.0's min-call-count-only trip criterion. - `//third_party:mockito` is added to the circuitbreaker test deps (already present on master) for the new mock-scheduler-based recovery tests. As on master, end-to-end circuit-breaker x Build-without-the-Bytes integration coverage is left to a follow-up to keep this change focused on the breaker state machine. Fixes bazelbuild#30304.
eaugene
force-pushed
the
backport-remote-circuit-breaker-recovery-8.8.0
branch
from
July 22, 2026 16:22
89194e9 to
75a501a
Compare
eaugene
marked this pull request as ready for review
July 22, 2026 17:41
Contributor
Author
|
#30260 is merged to master . looking forward to your reviews @iancha1992 |
Member
iancha1992
enabled auto-merge
July 22, 2026 17:49
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
2 participants
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.
Requesting inclusion in 8.8.0
Please consider this for the 8.8.0 release. We depend on
--remote_download_minimal(Build without the Bytes), where a tripped failure circuit breaker is currently unrecoverable for the rest of the invocation: a burst of transient remote-cache errors trips the breaker, the breaker-rejected prefetch reads become lost inputs, action rewinding re-runs the generating actions, and the still-open breaker rejects those re-executions too — failing the build. This backport makes that recoverable behind an opt-in flag, with no change to default behaviour.Backport of #30260 (tracking issue #30304)
Now that #30260 has landed on
master(asd1a022d0eb), this is the cherry-pick of that commit ontorelease-8.8.0, with the merge conflicts resolved and adapted to the 8.8.0 baseline. It reflects the final, reviewed version of #30260 (the earlier revision of this PR predated the review changes).Adaptations vs. the master commit, where 8.8.0 predates master prerequisites:
RemoteOptionsvalues are read as public@Optionfields (8.8.0 predates master's getter refactor); the new--experimental_remote_circuit_breaker_recovery_delayoption is added in that style.--experimental_remote_min_call_count_to_compute_failure_rateis already present onrelease-8.8.0(via [8.8.0] Remote: make the failure circuit breaker's minimum call count configurable #30234) and is used unchanged; the orthogonal min-fail-count half of Remote: make the failure circuit breaker's minimum call/fail counts configurable #30178 was not backported, so its constructor parameter, trip-guard clause and tests are omitted.Retrier.CircuitBreakerinterface change (recordSuccess/recordFailurenow take the observedState) is threaded through 8.8.0's existingshouldRetryretry flow.//third_party:mockitois added to the circuitbreaker test deps (already present on master) for the new mock-scheduler-based recovery tests.failureDetails()is already onrelease-8.8.0via [8.8.0] Remote: include failure statistics in circuit breaker rejection messa… #30384.Otherwise it mirrors #30260's
TRIAL_CALLhalf-open state machine (CLOSED/OPEN/RECOVERING/PROBING), gated behind the new--experimental_remote_circuit_breaker_recovery_delay(Duration, default0= today's permanent-open behaviour, so nothing changes unless it is enabled).As on master, the end-to-end circuit-breaker × Build-without-the-Bytes recovery integration test is left to a follow-up to keep this change focused on the breaker state machine.
Testing
Built and tested on
release-8.8.0(basea62d241f39) with the branch-pinned Bazel 8.7.0 and--lockfile_mode=error://src/test/java/com/google/devtools/build/lib/remote/circuitbreaker:circuitbreaker— PASSED//src/test/java/com/google/devtools/build/lib/remote:RemoteTests --test_filter=RetrierTest— PASSEDReview
Requesting review for inclusion in the 8.8.0 release. Thanks!