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

Bring back simple Circuitbreaker #1854

Closed
Epibatidin opened this issue Dec 12, 2023 · 5 comments
Closed

Bring back simple Circuitbreaker #1854

Epibatidin opened this issue Dec 12, 2023 · 5 comments

Comments

@Epibatidin
Copy link

Is your feature request related to a specific problem? Or an existing feature?

I try to add some resilience to a relatively small project.
We had some horrible expierence with Azure Service Bus just not being available or being throttled.
If this happens often enough we just want to fallback to not doing async message processing at all.
WE discussed about it and came to the conclussion that normally everything works just fine but then from 1 moment to the other everything just falls apart.
I picked the circuitbreaker and fallback for that. if it fails 20 times in a row disable the servicebus feature and fallback to sync processing.
in polly V8 there is only the advances (in v7 terms) circuitbreaker, i have read through the documentation and all the if then else and it depends there and i am not sure how to pick any of them.
like yes 20 as a threshhold - but will we get 20 messages at all. what if 10 messages fail but only 19 messages arived it not a threashold of 50% .

the business is just not large enough for all the monitoring and discussion and thinking through it to pick the right numbers for the advanced (yet default) circuit breaker setup.

Describe the solution you'd like

bring back the basic circuit breaker.

if it fails 20 times in a row - open it. thats its.

Additional context

currently i look into combining the polly v7 basic cirucit braker with my v8 fallback resilience pipeline. i think that will work but i wont get the new fanyness like integrated telemetry.

telemetry is currently high in value.

i have stitched togehter a horrible reflection magic solution to get all the internals like the ConsecutiveCountCircuitController into the advanced circuit resilience pipeline. but i hope it will never make it
through the review process !

a easy workaround here would be to make the ConsecutiveCountCircuitController public so it can be added as a strategy.

@martintmk
Copy link
Contributor

Hey @Epibatidin, if you throughput is so low have you considered that circuit breaker is really not needed? What benefit would introducing it bring for you?

You can still fine tune the advanced CB for low throughputs:

  • SamplingDuration: 2 minutes
  • MinimumThroughput: 10
  • FailureRatio: 0.5

So if over the sampling duration with minimum throughput of 10 more than 50% of requests fail, the CB will be opened. What in the configuration above doesn't work for you scenario?

I picked the circuitbreaker and fallback for that. if it fails 20 times in a row disable the servicebus feature and fallback to sync processing.

if service bus is throttled then even for low throughputs the CBs should trigger, you can increase sampling duration to even grater value is your rates are low.

@Epibatidin
Copy link
Author

so many considerations. is it to low or to high. is it to fast or slow. is it to long or to short.
in order to give proper answers to all these ever changing dimensions i would need to monitor the system. and put a lot
of efford into thinking this through. thats just miles over miles away down the road.

from my point of view i would prefer to enable a advanced feature when the system is advanced enough to require it.

i will put efford into thinking your suggestion through. but last time the servicebus took 140 seconds (+ internal retry ) to timeout. which is not in the scope of the purposed 2 minutes. so i then also need to think about that timout settings ?

V7 it is !
thanks for your help.

@martincostello
Copy link
Member

You can still use the 8.x.x NuGet packages, the old v7 API is still there in the Polly assembly.

@martintmk
Copy link
Contributor

fyi, at some point the simple circuit breaker was considered to be part of V8. during the API review we dropped it, because in most scenarios it would lead to incorrect usage.

#1444

@Epibatidin
Copy link
Author

thanks for providing that additional information havent seen it before.
so simplecircuitbreaker will then someday really be dropped and using the v7 api from the v8 assembly is just a shortlived workaround until v9 hits.

then a viable solution would be to move out the simplecircuitbreaker into a contrib package so that all the people willing to use it wrong can do so.

"can be used wrong" is just true for every tool.
i will most likely use the advancedcircuitbreaker wrong because i put in the wrong numbers.
i seen alot of code changes but cant find any reasoning. do you mean #1543 ?

@Epibatidin Epibatidin closed this as not planned Won't fix, can't repro, duplicate, stale Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants