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

Half-open state does not keep rejecting, and doesn't trigger re-try #120

Closed
wants to merge 1 commit into from

Conversation

jerryjvl
Copy link
Contributor

Proposed fix for issues reported in #119

This fix addresses the fact that once the circuit goes "half-open" it lets all further requests through, not just the first one. And the follow-on issue that failure of the first request let through does not guarantee the circuit gets treated as "open" again.

@ghost ghost added the in progress label Dec 12, 2017
@coveralls
Copy link

coveralls commented Dec 12, 2017

Coverage Status

Coverage remained the same at 99.396% when pulling d05b8d0 on jerryjvl:master into 9edc171 on bucharest-gold:master.

@lance lance self-requested a review December 13, 2017 17:40
@lance lance self-assigned this Dec 13, 2017
@lance
Copy link
Member

lance commented Dec 13, 2017

@jerryjvl thanks for the contribution. That's definitely a race condition I did not consider. Would you be willing to write a test for this as well?

@lance
Copy link
Member

lance commented Dec 13, 2017

Connects to: #119

@lance
Copy link
Member

lance commented Dec 13, 2017

Landed in: 04df6f7

@jerryjvl thanks again for contributing this fix! I have added a test in 602a43c. I do not have a regular release cycle for this, so if you are need of a patch level release, let me know and I can push one out.

@lance lance closed this Dec 13, 2017
@ghost ghost removed the in progress label Dec 13, 2017
@jerryjvl
Copy link
Contributor Author

Thanks! I didn't get any messages before I checked in here, will have to check up my email settings!
Would have happily contributed a test case as well if I'd seen it earlier.

No rush, we're working off my fork for now, once this one has an official release we'll pick that up.

@lance
Copy link
Member

lance commented Dec 18, 2017

@jerryjvl s'all good - I pushed a 1.3.1 release with this fix last week: https://github.com/bucharest-gold/opossum/releases/tag/v1.3.1

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.

3 participants