Open walec51 opened 3 years ago
I like the idea that you can handle results and based on the result you can force a transition to open.
Comparable to RateLimiterConfig.drainPermissionsOnResult
predicate, we could add a CircuitBreakerConfig.forceOpenOnResult()
predicate.
@RobWin to handle my use case I would need something more then a predicate - I'm thinking more of an Function<ForceOpenChecksResult, Either<Throwable, T>>
as shown in the example
in ForceOpenChecksResult
one would pass if a force open is needed and optionally until which point in time should it be opened
Ok, some ideas:
1) We have to add a private void transitionToOpenState(long waitDurationInMillis)
to the CircuitBreakerStateMachine
and add another constructor to CircuitBreakerStateMachine.OpenState
so that you can use a fixed waitDurationInMillis and not use the waitIntervalFunctionInOpenState
from the CircuitBreakerConfig.
2) We should not call the methd forceOpenOnResult
, because FORCED_OPEN
is a state of its own. Maybe we should better call it transitionOnResult
which would allow to transition to any state.
3) We should rename the class ForceOpenChecksResult
to TransitionCheckResult
.
TransitionCheckResult.noTransition()
should return a singleton and means no transition. I would like to avoid creating an TransitionCheckResult
object on every call. TransitionCheckResult.transitionToOpen()
means that the CircuitBreaker transitions to OPEN, but uses the waitDurationInMillis from the Config.TransitionCheckResult.transitionToOpen(waitDurationInMillis)
means that the CircuitBreaker transitions to OPEN, but uses the the given waitDurationInMillis.TransitionCheckResult.transitionToOpenUntil(instant)
means that the CircuitBreaker transitions to OPEN but calculates the waitDurationInMillis based on the given Instant.We could also create a CircuitBreakerResultUtils
comparable to the io.github.resilience4j.core.ResultUtils
which simplifies the usage:
.transitionOnResult(
callResult -> CircuitBreakerResultUtils.isFailedAndThrown(callsResult, SomeSpecificException.class,
(ex) -> TransitionCheckResult.transitionToOpenUntil(ex.getBannedUntil()))
or
.transitionOnResult(
callResult -> CircuitBreakerResultUtils.isSuccessfulAndReturned(callsResult, Response.class,
(response) -> TransitionCheckResult.transitionToOpenUntil(response.getBannedUntil()))
In the future we could also add:
TransitionCheckResult.transitionToForcedOpen()
means that the CircuitBreaker must transition to FORCED_OPENHi @walec51 Are you still interested to implement this feature?
@RobWin yes, I have read all your suggestions and I agree with them, I just didn't have the time to start working on this - maybe I'll find some on this weekend
@RobWin finally found some time to implement this, pls take a look at my draft PR
Problem:
We have a client class that uses resiliance4j decorators. The underlining API, which the client calls, can ban our IP for some time for multiple reasons. The underlining API in its error response returns the duration of the given ban and expects that we will stop any further communication during it. Not doing so could result in an even longer ban.
Solution:
This is an ideal use case for a circuit breaker as the bans duration can exceed the period of any rate limiter that is also applied. However resilience4j circuit breaker lacks configuration options and some implementation mechanisms to handle this. Therefore I propose to add the following API:
ForceOpenChecksResult
could also have a creator method calledForceOpenChecksResult.needed()
that would force open the circuit breaker for the time configured inwaitDurationInOpenState
attribute.I'm willing to implement this feature if it will be accepted. We need this feature in this project: https://github.com/knowm/XChange where we already use resilience4j for rate limiting and retries.