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

[DISCUSSION] Circuit breaker will remain half-open state forever when the request is blocked by upcoming rules #1638

Open
sczyh30 opened this issue Jul 30, 2020 · 7 comments
Labels
area/circuit-breaking Issues or PRs related to circuit breaking help wanted Extra attention is needed kind/bug Category issues or prs related to bug.

Comments

@sczyh30
Copy link
Member

sczyh30 commented Jul 30, 2020

Issue Description

Type: bug report

Describe what happened (or what feature you want)

The circuit breaker won't recover from half-open state when the request is actually blocked by upcoming rules. For example, there are two degrade rules of the same resource: R1(circuit breaker state=OPEN, recoveryTimeout=10s) and R2(circuit breaker state=OPEN, recoveryTimeout=20s)

There may be circumstances when R1 has reached the recovery timepoint but R2 has not. If a request comes, there will be transformation: R1(OPEN → HALF-OPEN), R2(OPEN)

This request will be allowed by R1 but rejected by R2, thus finally blocked. The invocation won't actually occur, so it will NEVER complete. For state transformation from HALF-OPEN to OPEN/CLOSED, it should happen only when invocation completes, so for R1 the associated circuit breaker state will be HALF-OPEN forever. Actually this could happen when there are any blocked rules after R1 (not only degrade rules).

This is a fatal bug and should be carefully resolved. We may need a temporary workaround for the half-open case in 1.8.0, then improve the overall design later. Discussions are welcomed.

Original discussions can be found in #1490 (comment)

@sczyh30 sczyh30 added kind/bug Category issues or prs related to bug. priority/urgent Most important, need to be worked on as soon as possible area/circuit-breaking Issues or PRs related to circuit breaking labels Jul 30, 2020
@sczyh30 sczyh30 added this to the 1.8.0 milestone Jul 30, 2020
@sczyh30
Copy link
Member Author

sczyh30 commented Jul 30, 2020

cc @jasonjoo2010 @wavesZh @louyuting

@jasonjoo2010
Copy link
Collaborator

I have spent some time thinking about it and now I have a proposal on it.

We can rename onRequestComplete to onRequestSuccess indicating it will be called only when requests were executed successfully. Thus, logics which are related on transmitting from Closed to Open will remain in it.

Introduce a linked list typed handlers structure into CtEntry (Which brings a new abstracted method into Entry) and migrate stateful logics like transmitting from Open to Half to the handlers bond to entries.

Any new ideas?

@jasonjoo2010
Copy link
Collaborator

And i am glad to submit an implementation of such concepts later to make it clearly.

@jasonjoo2010
Copy link
Collaborator

I have spent some time thinking about it and now I have a proposal on it.

We can rename onRequestComplete to onRequestSuccess indicating it will be called only when requests were executed successfully. Thus, logics which are related on transmitting from Closed to Open will remain in it.

Introduce a linked list typed handlers structure into CtEntry (Which brings a new abstracted method into Entry) and migrate stateful logics like transmitting from Open to Half to the handlers bond to entries.

Any new ideas?

Another design is to guarantee onRequestComplete invoked every time. Sure we should replan its arguments.

@sczyh30
Copy link
Member Author

sczyh30 commented Aug 4, 2020

And i am glad to submit an implementation of such concepts later to make it clearly.

POC implementations are welcomed.

Any new ideas?

Another idea that may eradicate the problem (but not elegant): ensure only one circuit breaker can be created per resource (the logic of different strategies might need to be composed)

@sczyh30
Copy link
Member Author

sczyh30 commented Aug 18, 2020

#1645 has been merged as a temporary workaround (for 1.8.0). Further discussions may be conducted later for a sound solution.

@sczyh30 sczyh30 added help wanted Extra attention is needed and removed priority/urgent Most important, need to be worked on as soon as possible labels Aug 18, 2020
@sczyh30 sczyh30 removed this from the 1.8.0 milestone Aug 18, 2020
@sczyh30 sczyh30 changed the title [BUG] Circuit breaker will remain half-open state forever when the request is blocked by upcoming rules [DISCUSSION] Circuit breaker will remain half-open state forever when the request is blocked by upcoming rules Aug 18, 2020
@glacier-archit
Copy link

Has this problem been resolved?My program integration with Sentinel still has this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/circuit-breaking Issues or PRs related to circuit breaking help wanted Extra attention is needed kind/bug Category issues or prs related to bug.
Projects
None yet
Development

No branches or pull requests

3 participants