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

remove PENDING_CLOSE flag #7

Closed
wants to merge 1 commit into from

Conversation

wei-lee
Copy link

@wei-lee wei-lee commented Dec 15, 2016

We have come across a problem when try to use this module. We found that the circuit breaker will be kept open if it's going to the "halfOpen" state the second time. There will be no way to close the circuit breaker afterwards.

After a little of debugging, it looks like the problem is caused by the PENDING_CLOSE flag. I can't really tell why we need the flag, and removing it fix the problem.

The case is being shown in the test.

@lance can you please review this and see if I am right?

Thanks.

lance added a commit that referenced this pull request Dec 20, 2016
@lance
Copy link
Member

lance commented Dec 20, 2016

@wei-lee thanks for making the contribution. But we actually do need PENDING_CLOSE.

It's a real edge case, but the reason we have PENDING_CLOSE is because the circuit should only ever attempt to execute once when in the HALF_OPEN state. In a scenario where lots and lots of requests are occurring in parallel, many could execute while the circuit is HALF_OPEN. By checking the PENDING_CLOSE state, we ensure this doesn't happen.

You did discover a bug, however. So thank you! The PENDING_CLOSE flag was never being reset. I've fixed that, and used your test case in my latest commit.

@lance lance closed this Dec 20, 2016
lance added a commit that referenced this pull request Dec 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants