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

React to a policy state change from outside #1356

Closed
fawques opened this issue Jun 26, 2023 · 8 comments
Closed

React to a policy state change from outside #1356

fawques opened this issue Jun 26, 2023 · 8 comments
Labels
question stale Stale issues or pull requests

Comments

@fawques
Copy link

fawques commented Jun 26, 2023

Summary: What are you wanting to achieve?
We are trying to implement something similar to what is mentioned in #287, taking into account Polly's circuit breakers to reach a quorum on a system uptime.
Our plan was to use the existing Circuit Breakers, wrapping them in a new CB policy that we could add OnBreak callbacks and react accordingly. This is because we want to let the application create their own circuit breakers and pass them in.

What code or approach do you have so far?
We have two parts, an application with a connection to a system, using a circuit breaker:

        var delay = Backoff.DecorrelatedJitterBackoffV2(settings.MedianFirstRetryDelay, settings.RetryCount);
        var retry = RetryPolicyBuilder(settings.ErrorCodes).WaitAndRetryAsync(delay, onRetry: (ex, time) => Log.Warning(ex, "Error processing request. Waiting for {RetryTimeSpan} before the next retry", time));

        var breaker = CircuitBreakerPolicyBuilder(settings.ErrorCodes).CircuitBreakerAsync(settings.ExceptionsAllowedBeforeBreaking, settings.DurationOfBreak,
            onBreak: (_, duration) => Log.Warning("Circuit breaker tripped. Circuit is open and requests won't be allowed through for {DurationOfBreak}", duration),
            onReset: () => Log.Information("Circuit closed. Requests are now allowed through"),
            onHalfOpen: () => Log.Information("Circuit is now half-opened and will test the service with the next request")
        );

        var policy = breaker.WrapAsync(retry);

        // Somewhere else
        await policy.ExecuteAsync(DoSomething)

And a library that receives a policy per system, and wraps it in a new policy updating the quorum

public void WithCircuitBreaker(Dictionary<string, ICircuitBreakerPolicy> policies)
{
    var wrapperPolicies = new Dictionary<string, ICircuitBreakerPolicy>();
    foreach (var policy in policies)
    {
        var wrapperPolicy = Policy.Handle<Exception>()
                                                .CircuitBreakerAsync(1, TimeSpan.FromMilliseconds(500), 
                                                    onBreak: IncreaseBrokenCount);
        var p = wrapperPolicy.Wrap(policy.Value);

        wrapperPolicies.Add(policy.Key, policy.Value);
    }
}

Doing this, however, the application retains the original policy, and the wrapper policy is never executed.

Is there any way of reacting to state changes in a Circuit Breaker policy, without creating a new policy wrapping it?
Or to modify the onBreak callbacks to add extra behaviour to them?

Regards

@martincostello
Copy link
Member

@martintmk Is this something that is possible in v8?

@martintmk
Copy link
Contributor

The Polly V8 is conceptually very similar to V7 in this regard. It's just that in V8 we also support async callbacks and we have richer arguments in these callbacks.

What could help is addition of circuit breaker partitioning. We could add the following optional callback to CircuitBreakerStrategyOptions:

public class CircuitBreakerStrategyOptions<TResult>
{
    public Func<ResilienceContext, string>? CircuitKeyGenerator { get; set; }
}

This would allow the circuity breaker strategy (policy) to pool the circuit breakers by the key and the application to customize how these circuit breakers are pooled.

@vguzmanp Would this enhancement help?

@PeterCsalaHbo
Copy link

In case of V7 all onXYZ callbacks of CB are NOT allowing you to do state change.
You can however manually control the CB's state outside of the policy via the Isolate and Reset calls.

If you want to have the above described functionality then you could try the following:

  • Each individual CB should register itself into a singleton component
  • Individual CB's onBreak should call into the component to indicate a failure
    • When the quorum is reached the singleton component should call the Isolate method on each CB
      • Please bear in mind the normal CB counts consecutive failures << this is pretty hard to do in the singleton

The problematic part here is the Half-Open state:

  • Either you have Open and Closed states (after a predefined period of time the singleton calls Reset on each CB)
  • Or you have to choose randomly a CB, call Reset on it and hope that instance will be used
    • All CBs' onReset should call into the singleton component to reset the rest of the CBs

The solution is complex and fragile. If possible I would try to avoid to introduce this.

@fawques
Copy link
Author

fawques commented Jun 26, 2023

This would allow the circuity breaker strategy (policy) to pool the circuit breakers by the key and the application to customize how these circuit breakers are pooled.
Would this enhancement help?

@martintmk I'm not sure that I follow you there.
This would allow us to have several strategies returning the same key based on the context, but their respective onBreak callbacks would still be the same, so we would still need some way of reacting to the state change, or adding behaviour to them.

@fawques
Copy link
Author

fawques commented Jun 26, 2023

@PeterCsalaHbo Thank you for proposing this solution

You can however manually control the CB's state outside of the policy via the Isolate and Reset calls.

Our approach was to rely on the primitives already present in Polly for the transitions to the different circuit status and their timings; and when the thresholds are reached, modify a "feature flag" that disables the system. That way the individual circuits can keep their normal flow of states, without having to force them with Isolate or Reset.

Individual CB's onBreak should call into the component to indicate a failure

Our idea was to avoid modifying the individual CB's onBreak, as the library is intended to be generic and used by different applications.
Other than this, our approach was similar to this, with onBreak increasing the "broken count" and onHalfOpen decreasing it.

The solution is complex and fragile. If possible I would try to avoid to introduce this.

I agree that it sounds complex and fragile, and adding to it, our circuit breakers are also replicated in different instances (thus the need for quorum), which adds to the complexity.
Are you aware of any other simpler solution out there?

@PeterCsalaHbo
Copy link

PeterCsalaHbo commented Jun 26, 2023

@vguzmanp Unfortunately I'm unaware of any other simpler solution :(

@github-actions
Copy link
Contributor

This issue is stale because it has been open for 60 days with no activity. It will be automatically closed in 14 days if no further updates are made.

@github-actions github-actions bot added the stale Stale issues or pull requests label Aug 26, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2023

This issue was closed because it has been inactive for 14 days since being marked as stale.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question stale Stale issues or pull requests
Projects
None yet
Development

No branches or pull requests

4 participants