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 opened write after first fire if it fails. #217

Closed
fylhsq123 opened this issue Jul 19, 2018 · 3 comments
Closed

Circuit breaker opened write after first fire if it fails. #217

fylhsq123 opened this issue Jul 19, 2018 · 3 comments

Comments

@fylhsq123
Copy link

Node.js Version:
8.9.4
Operating System:
MacOS High Sierra 10.13.6 (Darwin Kernel Version 17.7.0)
Steps to Produce Error:
I'm getting circuit breaker opened write after first fire if it fails.
For example I creating circuit breaker with following options:

const options = {
        timeout: 1000,
        errorThresholdPercentage: 50,
        rollingCountTimeout: 20000,
        resetTimeout: 30000
};
breaker = circuitBreaker(makeRequest, options);

And then if I'm getting failure on my first fire it calculates like 100% failures which is higher then 50% I set in options.
I believe this is happening because of this part of code in a module ('./lib/circuit.js'):

const errorRate = stats.failures / stats.fires * 100;
if (errorRate > circuit.options.errorThresholdPercentage ||
    circuit.options.maxFailures >= stats.failures ||
    circuit.halfOpen) {
    circuit.open();
}

Maybe you should gather some statistic first without opening circuit but with handling fails. I mean like 10 fires or make additional parameter for that.

@lance
Copy link
Member

lance commented Jul 19, 2018

I am aware of this issue, but I'm not sure this is a bug. The fact is that one execution of the function with one failure is 100% failure rate. But I will leave this open for now. It's possible that we can modify this conditional check to override if the amount of data is insufficient. But it's not clear to me the best and cleanest way to do this.

@fylhsq123
Copy link
Author

I suppose it could be some additional parameter where user can set number of requests, before the circuit breaker will calculate the health of the service and decide if we need circuit to be opened.
What do you think about it?

lance added a commit that referenced this issue Jul 20, 2018
This feature allows the user to let the circuit warm up
before opening, even if every request is a failure or timeout.
The warmup duration is the value provided for
options.rollingCountTimeout.

Fixes: #217
@ghost ghost added the in progress label Jul 20, 2018
@lance
Copy link
Member

lance commented Jul 20, 2018

@fylhsq123 not exactly what you suggested, but you can test out my branch in the pull request to see what you think. Instead of a hard number, the circuit will ignore failures/timeouts for the purposes of open/close state until the options.rollingCountTimeout value has expired. Let me know how this works for you.

@lance lance closed this as completed in #218 Aug 2, 2018
lance added a commit that referenced this issue Aug 2, 2018
This feature allows the user to let the circuit warm up
before opening, even if every request is a failure or timeout.
The warmup duration is the value provided for
options.rollingCountTimeout.

Fixes: #217
@ghost ghost removed the in progress label Aug 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants