Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Circuit Breaker Rolling failure window behaviour: follow up to issue #188 #197

Closed
neilgsyoung opened this issue Nov 21, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@neilgsyoung
Copy link
Contributor

commented Nov 21, 2017

We need to clarify the behaviour of Circuit Breakers as discussed in issue #188. I think that the original CircuitBreaker#testCircuitDefaultSuccessThreshold test was working as designed but that we need some clearer words in the Spec to explain the behaviour.

The test illustrates the transition of the Circuit between closed, open and half-open states. In the test, when the circuit state is closed, a rolling failure window is in play, configured through the requestVolumeThreshold and failureRatio parameters. The parameters define a rolling failure window of 4 executions with a failureRatio of 0.75. Three of the four executions fail, so that the circuit transitions to open state. The test then delays sufficiently for the circuit to transition to half-open state. In half-open state a "trial success" window comes into play. The successThreshold parameter allows the configuration of the number of trial calls that must succeed before the circuit can be closed but in the test this parameter is allowed to default to a single execution. After a successful trial execution the circuit is closed. The transition to closed state means that the trial success window is closed and a new empty rolling failure window is created. Another four executions are then required before the Circuit is opened once more.

I'd like to suggest some revised words in the Spec, together with some more explanation around the CircuitBreaker#testCircuitDefaultSuccessThreshold test and a couple of new Circuit Breaker tests.

@Emily-Jiang

This comment has been minimized.

Copy link
Member

commented Nov 27, 2017

close this issue as the PR #201 was merged.

@Emily-Jiang Emily-Jiang added this to the 1.1 milestone Nov 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.