-
Notifications
You must be signed in to change notification settings - Fork 118
Description
Describe the bug
When using a timelimiter + semaphore bulkhead, if the timelimiter's time is exceeded, the underlying thread executing is not interrupted but the bulkhead permit is released. This means if the restricted section inside the bulkhead is consistently timing out, more concurrent requests are submitted than what the bulkhead should allow.
This happens because the Resilience4JBulkheadProvider uses CompletableFuture.supplyAsync and Completable Futures can't be interrupted.
From the CompletableFuture.cancel docs:
Parameters: mayInterruptIfRunning - this value has no effect in this implementation because interrupts are not used to control processing.
Additional Article on CompletableFuture and interrupts: https://nurkiewicz.com/2015/03/completablefuture-cant-be-interrupted.html
Affected Versions
2.x and 3.x
Sample
Sample repository: https://github.com/Mistborn94/spring-r4j-bulkhead-demo
The sample repository contains two test classes that are almost identical: BulkheadDisabledTests and BulkheadEnabledTests. BulkheadDisabledTests passes but BulkheadEnabledTests fails because the thread is not interrupted.
Possible solution
- Use
Bulkhead.decorateSupplier
instead ofBulkhead.decorateCompletionStage
to create the bulkhead and then use an executor service with a normal future (possibly the same one that is already used in theResilience4JCircuitBreaker
) for the time limiter. - Swap the order of the bulkhead and the timelimiter so the time limiter happens inside the bulkhead and can interrupt the task (This would also fix the thread pool bulkhead which works only with CompletionStage and has a similar problem)
I would be willing to submit a pull request for this