spring-cloud / spring-cloud-circuitbreaker

Spring Cloud Circuit Breaker API and Implementations
Apache License 2.0
332 stars 111 forks source link

TimeLimiter disable by group or instance level #202

Closed santoshdahal12 closed 1 month ago

santoshdahal12 commented 1 month ago

now

Currently, TimeLimiter can be disabled globally for all CircuitBreaker instances. This is possible through the change by this request https://github.com/spring-cloud/spring-cloud-circuitbreaker/issues/174.

However, this can be configurable to instance level or group level as well, so that for certain instances where a socket/port are already provided with timeouts and gets opened within the circuitbreakers invocation, dont need another timeout.

This was possible in Hystrix. I am trying to migrate to resilience4j and trying to achieve similar functionality .

Currently, either we have to disable globally or we have to define a custom long time out config that instances need to use even they dont need to worry about timeout.

Describe the solution you'd like We can introduce a configuration that could take map of entries like below

spring:
  cloud:
    circuitbreaker:
      resilience4j:
        enableGroupMeterFilter: true
        defaultGroupTag: customTag
        enableSemaphoreDefaultBulkhead: true
        disableThreadPool: true
        disableTimeLimiter: true
        disableTimeLimiterMap:
          group1: true
          instanceA: false
          instanceB: true

And then at, https://github.com/spring-cloud/spring-cloud-circuitbreaker/blob/main/spring-cloud-circuitbreaker-resilience4j/src/main/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4JCircuitBreakerFactory.java#L184

boolean isDisableTimeLimiter = getDisableTimeLimiterForInstanceOrGroupOrGlobal(id, groupName);

which would get boolean value based on above config passed or would default to global disableTimeLimit for each circuitbreaker instance. Something like below would be the implementation

private boolean getDisableTimeLimiterForInstanceOrGroupOrGlobal(String instance, String group) {
        Map<String, Boolean> disableTimeLimiterMap = this.resilience4JConfigurationProperties.getDisableTimeLimiterMap();
        Boolean disableTimeLimiterForInstance = disableTimeLimiterMap.get(instance);
        if (disableTimeLimiterForInstance != null) {
            return disableTimeLimiterForInstance;
        }

        if (disableTimeLimiterMap.get(group) != null) {
            return disableTimeLimiterMap.get(group);
        }
        return this.resilience4JConfigurationProperties.isDisableTimeLimiter();
    }

Describe alternatives you've considered We have to set timeout even though certain requests already provided with timeout as mentioned above. Let me know if this feature request sounds good or is trash :-)

Additional context Add any other context or screenshots about the feature request here.

ryanjbaxter commented 1 month ago

I think this makes sense. Would you be interested in providing a PR?

santoshdahal12 commented 1 month ago

I will try to work on the PR this weekend. Thanks for accepting this request.