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

Feature: Combine circuit breakers with TimeLimiter in Spring boot #430

Closed
charlesardsilva opened this issue Apr 18, 2019 · 37 comments
Closed
Milestone

Comments

@charlesardsilva
Copy link

This is a request for adding a feature in resilience4j.
The project could have integration between "CircuitBreaker" and "TimeLimiter" using annotation in Spring Boot project.
Hystrix there is a configuration in @HystrixCommand that allows us to set timeout to desired operation.

I understood that is possible configure CircuitBreaker and TimeLimiter only using expression.

@RobWin
Copy link
Member

RobWin commented Apr 18, 2019

We implemented a Threadpool based Bulkhead with a bounded queue which can easily be combined with a TimeLimiter in the future.
The current Bulkhead uses Semaphores.
And our CircuitBreaker does not use any ThreadPool, because it's up to the user to decide, if he wants to use one or not.

@charlesardsilva
Copy link
Author

Hi @RobWin ,
I did not understand, so for now there is no option to use @CIRCUITBREAKER+TimeLimter, this is planned for the future, I am right?.

Thanks for your attention.

@RobWin
Copy link
Member

RobWin commented Apr 18, 2019

Sure you can. Just use your own Executorservice or CompletableFuture.
But there is no TimeLimiter annotation yet.

@RobWin
Copy link
Member

RobWin commented Apr 18, 2019

But anyway we can add a new TimeLimiter Annotation.

@charlesardsilva
Copy link
Author

For now I need more time to do it, but I don't have, when I am available in the next months and it have not been done yet, I can try doing it.
Thanks a lot!!!

@RobWin
Copy link
Member

RobWin commented Apr 18, 2019

Can you invoke Future.get(5, TimrUnit.SECONDS) on your Future?

@anderseknert
Copy link

Trying to piggyback on the timelimiter topic rather than creating a new issue here - is there any reason that the time limiter module is not imported by default for the spring module? I would expect that to be the given how it's called a core module.

@RobWin
Copy link
Member

RobWin commented May 4, 2019

Yes, there are no additional Spring Boot features yet. First we need to create an Aspect which supports Reactor and RxJava as well. Shouldn't be too difficult, but nevertheless we need some time. A PR is welcome :)

@RobWin
Copy link
Member

RobWin commented Sep 13, 2019

Anyone interested in contributing this feature?

@dlsrb6342
Copy link
Member

I'll work on this issue.

@liorsamuni
Copy link

Any updates regarding this feature support?

@dlsrb6342 are you still going to contribute this feature?

@dlsrb6342
Copy link
Member

@liorsamuni
Sorry. I don't have time for this until few weeks. If you interested in this, you can work on this.

@ericwyles
Copy link

ericwyles commented Dec 2, 2019

I'm interested in this as well. @dlsrb6342 are you still planning to work on it?

EDIT: Oh sorry, I see that you just answered this question about 2 weeks ago. I might be able to look into it. Any pointers on getting started? I've never been in this code base before.

@dlsrb6342
Copy link
Member

@ericwyles
First of all, please check our CONTRIBUTING.adoc.

As @RobWin commented, we need to create an Aspect which supports Reactor and RxJava.
And also, we have to support FeignClient.
Please refer other modules(bulkhead, circuitbreaker, retry, ratelimiter) how to support Aspect and FeignClient.

@RobWin
Copy link
Member

RobWin commented Feb 3, 2020

Merged 🎉

@RobWin RobWin added this to the 1.2.1 milestone Feb 3, 2020
@RobWin RobWin closed this as completed Feb 3, 2020
@RobWin
Copy link
Member

RobWin commented Feb 4, 2020

The Spring Boot 2 demo shows how to use the Resilience4j annotations -> https://github.com/resilience4j/resilience4j-spring-boot2-demo/blob/master/src/main/java/io/github/robwin/service/BackendAService.java#L141-L148

The TimeLimiter annotations can be used for CompletableFuture, CompletionStage, Spring Reactor, RxJava2.

@charlesardsilva
Copy link
Author

Great news!!! Thanks a lot, I will notify my team about this feature!!!
Congratulations!

@ericwyles
Copy link

Thank you all for completing this! I will notify my team as well.

@RobWin
Copy link
Member

RobWin commented Feb 4, 2020

It's not released yet. Maybe tomorrow

@RobWin
Copy link
Member

RobWin commented Feb 5, 2020

v.1.3.0 has been released. Maven Central Sync might take some time.

@tarunjjwala
Copy link

@RobWin , I am using @Timelimiter(name = "service1", fallbackMethod = "service1Fallback") independently on a method without @circuitbreaker/bulkhead etc.
I am not able to set the timeout property correctly and timelimiter object is picking 1s as default timeout.
Please help with the correct property to set it.
I have tried using both resilience4j.timelimiter.configs.default.timeoutDuration=100s & resilience4j.configs.default.timeoutDuration=100s

@RobWin
Copy link
Member

RobWin commented Feb 14, 2020

What is the return type of your method?

@tarunjjwala
Copy link

return type is a pojo. Let's me define a custom example of my scenario.

`class Test{
@Timelimiter(name = "service1", fallbackMethod = "service1Fallback")
SalesResponse fetchSalesResponse(String id);

default SalesResponse service1Fallback(String id, Throwable t){
return new SalesResponse();
}
}`

@RobWin
Copy link
Member

RobWin commented Feb 14, 2020

You can only use TimeLimiter with CompletableFuture, Reactor or RxJava2 return types.

@tarunjjwala
Copy link

Why is timeLimiter not extended to plain objects like other annotation? Also, is there any way to leverage this feature in my scenario?

@RobWin
Copy link
Member

RobWin commented Feb 14, 2020

Because a Timelimiter can only interrupt the execution and return an exception, if it is not running in the same thread.
Resilience4j is no running your method in another thread implicitly. You have to decide it explicitly by either using a Threadpool Bulkhead, your own ExecutorService or Reactor.

@tarunjjwala
Copy link

Thanks for the answer!

@balakumaran-sugumar
Copy link

Thanks for last pointer, this helped. I was getting IllegalArgument exception till then. CompletableFuture solved the issue

@akhil-gupta01
Copy link

akhil-gupta01 commented Apr 10, 2020

@RobWin Here is the piece of code on which I am trying to implement @Timelimiter:

@TimeLimiter(name = "backendA", fallbackMethod = "defaultConfirmPayment")
  private CompletableFuture<ResponseEntity<String>> getConfirmPaymentCompletableFuture (
      HttpEntity<JSONObject> httpEntity) throws InterruptedException {
    log.info("Looking up " + httpEntity.getBody());
    ResponseEntity<String> responseEntity =
        restTemplate.exchange(ovopayURL, HttpMethod.POST, httpEntity, String.class);
    Thread.sleep(1000L);
    return CompletableFuture.completedFuture(responseEntity);
  }

  @SuppressWarnings("unused")
  private CompletableFuture<ResponseEntity<String>> defaultConfirmPayment (
      OvopayOrderRequest request, Throwable e) {
    log.info("reverse transaction failed for merchant txn id: {}",
        request.getTransactionRequestData().getMerchantInvoice());
    return CompletableFuture.completedFuture(new ResponseEntity<String>(HttpStatus.NOT_FOUND));
  }

Here is the application.yml file:

resilience4j.timelimiter:
  configs:
    default:
      cancelRunningFuture: true
      timeoutDuration: 1s
  instances:
    backendA:
      baseConfig: default

But the time limiter is not working. Can you please tell, what I am doing wrong?

@charlesardsilva
Copy link
Author

@RobWin e @akhil-gupta01 it is happing the same with me.

@CircuitBreaker(name = "myBean", fallbackMethod = "fallback")
@TimeLimiter(name = ""myBean"")
@Component
@AllArgsConstructor
public class MyClass {

    private MyService myService;

    public CompletableFuture<String> perform() {
        return CompletableFuture.completedFuture(myService.slowService());
    }

    private CompletableFuture<String> fallback(Exception e) {
        return CompletableFuture.completedFuture("fallback");
    }
}

My configuration


resilience4j.circuitbreaker:
    instances:
        myBean:
            failureRateThreshold: 40
            waitDurationInOpenState: 20s
            minimumNumberOfCalls: 5


resilience4j.timelimiter:
    instances:
        myBean:
            timeoutDuration: 2s
            cancelRunningFuture: true

When I call myClass.perform, this method spend more than 3s, but the timelimiter does not "interrupt" the operation.

@RobWin
Copy link
Member

RobWin commented Apr 23, 2020

@charlesardsilva @akhil-gupta01

The TimeLimiter or CircuitBreaker do not run a Task in a Threadpool. You either have to combine the Timelimiter with @Bulkhead(name = "myBean", type = Type.THREADPOOL) or use CompletableFuture.supplyAsync() in your code.

@charlesardsilva
Copy link
Author

Hi @RobWin , thanks for your help! It's worked.

@Elewyth
Copy link

Elewyth commented Jun 22, 2020

@charlesardsilva @akhil-gupta01

The TimeLimiter or CircuitBreaker do not run a Task in a Threadpool. You either have to combine the Timelimiter with @Bulkhead(name = "myBean", type = Type.THREADPOOL) or use CompletableFuture.supplyAsync() in your code.

@RobWin using @Bulkhead(name = "myBean", type = Type.THREADPOOL) still gives me an IllegalReturnTypeException (using a simple List<MyPojo> as return type). Changing everything to use CompletableFuture would work, but is not what I prefer. Should @Bulkhead actually work with @TimeLimiter?

@RobWin
Copy link
Member

RobWin commented Jun 22, 2020

No, you can't limit the time of your current running Thread. You have to spawn the task in another thread in order to limit the execution time. That's why you can't combine a TimeLimiter with a sync method signature.

@Elewyth
Copy link

Elewyth commented Jun 22, 2020

Yeah, it makes sense, of course, that you cannot limit the time of the current running Thread. Your previous answer suggested that combining @BulkHead (ThreadPool) with @TimeLimiter would limit the time of the underlying ThreadPool-Threads automatically - not requiring explicit CompletableFuture signature.

I guess I will just implement timeouts on my RestTemplate for now, thanks anyway.

@sidarunoyo
Copy link

yes

hexmind pushed a commit to hexmind/resilience4j that referenced this issue Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests