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

[sczyh30]

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 https://github.com/alibaba/Sentinel/pull/1490#discussion_r462079959

[sczyh30]

cc @jasonjoo2010 @wavesZh @louyuting

[jasonjoo2010]

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]

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

[jasonjoo2010]

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]

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]

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