App-vNext / Polly

Polly is a .NET resilience and transient-fault-handling library that allows developers to express policies such as Retry, Circuit Breaker, Timeout, Bulkhead Isolation, and Fallback in a fluent and thread-safe manner. From version 6.0.1, Polly targets .NET Standard 1.1 and 2.0+.
https://www.thepollyproject.org
BSD 3-Clause "New" or "Revised" License
13.41k stars 1.23k forks source link

[Feature request]: Indicate time of possible circuit closure in `BrokenCircuitException` #2284

Closed DL444 closed 1 month ago

DL444 commented 1 month ago

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

Related to #506.

Imagine a worker service pulling messages from a queue (e.g. Azure Service Bus) and issuing circuit-breaker protected calls to downstream HTTP endpoints. If the downstream services go down and the circuit breaks, the worker has no easy way to know when the circuit breaker would possibly become closed again.

506 recommends applications to stop processing messages for some time, and I would agree. The problem is "for how long". Knowing when the circuit breaker is likely still open without relying on actually performing and failing the request is important for message queue scenarios, because excessive retries and failures can quickly exhaust message delivery limit and deadletter the messages unnecessarily.

Applications can surely take a guess, but it still needs this piece of information to make an informed decision on how long to cease message processing. It wouldn't help if the circuit breaker is configured to break for 2 days and the application blindly guesses 2 hours.

Describe the solution you'd like

Indicate the time when the circuit breaker can possibly become closed again in BrokenCircuitException. Possible API:

  public class BrokenCircuitException : ExecutionRejectedException
  {
+     public TimeSpan? RetryAfter { get; }
  }

This matches RetryAfter from RateLimiterRejectedException. Pulling it up to ExecutionRejectedException is not advisable because it does not apply to all derived exception types.

For IsolatedCircuitException, set this to null.

Additional context

No response

martintmk commented 1 month ago

@DL444

I think this would be great addition and relatively straightforward to implement! Are you willing tho give this a shot?

Basically, you just need to update this code:

https://github.com/App-vNext/Polly/blob/a8c64be6c6b8647037130278782ab180bcb8d56b/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs#L308

And to retrieve the value until the CB is blocked you can use:

https://github.com/App-vNext/Polly/blob/a8c64be6c6b8647037130278782ab180bcb8d56b/src/Polly.Core/CircuitBreaker/Controller/CircuitStateController.cs#L20

DL444 commented 1 month ago

Are you willing tho give this a shot

Yes, I would like to try that this week.

peter-csala commented 1 month ago

Are you willing tho give this a shot

Yes, I would like to try that this week.

Hey if you need some help, please let us know.

DL444 commented 1 month ago

Hey if you need some help, please let us know.

Not at this time, I've just been busy. I actually already have a working implementation here, but still need some time to sort out testing.