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

TimeLimiter disable by group or instance level #202

Closed
santoshdahal12 opened this issue Jul 26, 2024 · 2 comments · Fixed by #203
Closed

TimeLimiter disable by group or instance level #202

santoshdahal12 opened this issue Jul 26, 2024 · 2 comments · Fixed by #203
Labels
enhancement New feature or request
Milestone

Comments

@santoshdahal12
Copy link

now

Currently, TimeLimiter can be disabled globally for all CircuitBreaker instances. This is possible through the change by this request #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
Copy link
Contributor

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

@ryanjbaxter ryanjbaxter added enhancement New feature or request and removed waiting-for-triage labels Jul 26, 2024
@santoshdahal12
Copy link
Author

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

@ryanjbaxter ryanjbaxter linked a pull request Jul 28, 2024 that will close this issue
@ryanjbaxter ryanjbaxter added this to the 3.1.3 milestone Jul 29, 2024
ryanjbaxter pushed a commit that referenced this issue Jul 29, 2024
@github-project-automation github-project-automation bot moved this to Done in 2023.0.4 Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants