improve circuit breaker to reject excess attempts in half-open #323
Conversation
This file contains 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
|
Weird, that test passes for me just fine locally. Need to investigate. |
|
OK, it does fail fairly often when I run the test repeatedly. |
|
Got it, it's a concurrency issue that causes an assertion to fail, which causes executor shutdown, which causes thread interruptions, which manifests as a weird test failure :-) |
|
Actually we've had the concurrency issue several times before, always solved it with using a pair of |
Previously, circuit breakers in the half-open state would let all invocations go through. In case of any failure, it would move back to open, and in case of a configured number of consecutive successes, it would move to closed. This is fine and passes the TCK, but under high load, this can cause a thundering herd problem on the guarded service. A better solution, implemented by this commit, is: in half-open, only allow the first `successThreshold` invocations to go through (also known as "probe invocations"), and reject the rest. If any of those probe invocations fail, move back to open; if all of them succeed, move to closed.
Ladicek
force-pushed
the
half-open-circuit-breaker-reject-excess-attempts
branch
from
30650c1
to
ac5419f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Previously, circuit breakers in the half-open state would let
all invocations go through. In case of any failure, it would
move back to open, and in case of a configured number of consecutive
successes, it would move to closed.
This is fine and passes the TCK, but under high load, this can
cause a thundering herd problem on the guarded service.
A better solution, implemented by this commit, is: in half-open,
only allow the first
successThresholdinvocations to go through(also known as "probe invocations"), and reject the rest. If any
of those probe invocations fail, move back to open; if all of them
succeed, move to closed.
Fixes #319