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

Confusion about ErrTooManyRequests error #30

Open
bigunyak opened this issue Nov 7, 2020 · 4 comments
Open

Confusion about ErrTooManyRequests error #30

bigunyak opened this issue Nov 7, 2020 · 4 comments

Comments

@bigunyak
Copy link

bigunyak commented Nov 7, 2020

Hi,

Thanks a lot for your library, it's simple, yet powerful! :)
Could you please clarify one confusion I have about ErrTooManyRequests error? I don't quite understand from the code when this error could happen.
I see the if state == StateHalfOpen && cb.counts.Requests >= cb.maxRequests condition in the beforeRequest() method but from the rest of the code it doesn't look like it could ever be met. This is because we either get enough consecutive requests to trip circuit to close with this condition if cb.counts.ConsecutiveSuccesses >= cb.maxRequests { cb.setState(StateClosed, now) } or we go back to open state because of a single failure in onFailure(). Or did I misunderstand something?

@YoshiyukiMineo
Copy link
Member

If you call cb.Execute in parallel, you can call too many beforeRequest before afterRequest.

@tegaralaga
Copy link

tegaralaga commented Feb 13, 2021

If you call cb.Execute in parallel, you can call too many beforeRequest before afterRequest.

AFAIK beforeRequest will not executed in parallel since it has mutex. I try to get ErrTooManyRequests just last night, but couldn't get it.

@chiendo97
Copy link

Hi @tegaralaga

You can reproduce the ErrToManyRequests by this test case.

func TestTooManyRequests(t *testing.T) {
	customCB.setState(StateHalfOpen, time.Now())

	// 3 consecutive successes
	succeedLater(customCB, time.Duration(1000)*time.Millisecond) 
	succeedLater(customCB, time.Duration(1000)*time.Millisecond)
	succeedLater(customCB, time.Duration(1000)*time.Millisecond)

	time.Sleep(time.Duration(100) * time.Millisecond)

	assert.Equal(t, Counts{3, 0, 0, 0, 0}, customCB.counts)
	assert.Equal(t, succeed(customCB), ErrTooManyRequests)
}

@rthier
Copy link

rthier commented Jul 7, 2021

I'm using the "two steps" circuit breaker and is very easy to get the ErrTooManyRequests.
The problem is that after this error the state half open never change!

The state machine stuck by the condition mentioned in this issue:
if state == StateHalfOpen && cb.counts.Requests >= cb.maxRequests

Maybe this is a issue when use the "two steps" feature, but probably a public function to force a state or reset the counters would be helpful to handle this behavior.

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

No branches or pull requests

5 participants