Resilience4j: io.github.resilience4j.circuitbreaker.CircuitBreaker is not thread-safe

[srchen1987]

Dear all,I try it and find CircuitBreaker is not thread-safe ,
I see the CircuitBreaker source code :A CircuitBreaker instance is thread-safe can be used to decorate multiple requests.

my code
```package com.anywide.resilience4j.client;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Supplier;
import io.github.resilience4j.circuitbreaker.CircuitBreaker;
import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig;
import io.vavr.control.Try;
public class Test {
public interface BackendService {
String doSomethingWithArgs(String name);
String doSomethingThrowException ()throws Exception ;
}
static BackendService backendService = new BackendService() {
AtomicInteger id = new AtomicInteger(0);

    @Override
    public String doSomethingWithArgs(String s) {
        System.out.println("call time :" + id.getAndIncrement());
        throw new RuntimeException("bug...");

// return "success";
}

    @Override
    public String doSomethingThrowException() {
        System.out.println("doSomethingThrowexception");
        throw new RuntimeException("bug...");
    }
};

static CircuitBreakerConfig circuitBreakerConfig = CircuitBreakerConfig.custom().failureRateThreshold(5)
        .ringBufferSizeInHalfOpenState(10).ringBufferSizeInClosedState(15)
        .enableAutomaticTransitionFromOpenToHalfOpen().build();
static CircuitBreaker circuitBreaker = CircuitBreaker.of("mybreaker", circuitBreakerConfig);

public static void main(String[] args) throws InterruptedException {
    ExecutorService ex = Executors.newFixedThreadPool(20);
    for (int i = 0; i < 30; i++) {
        ex.execute(() -> {
            Supplier<String> supplier = CircuitBreaker.decorateSupplier(circuitBreaker,
                    () -> backendService.doSomethingWithArgs("world"));
            Try.ofSupplier(supplier).onFailure(e -> {
                System.out.println(circuitBreaker.getState() + "\t" + e.getMessage() + "\t"
                        + circuitBreaker.getMetrics().getFailureRate());
            }).onSuccess(e -> {
                System.out.println(Thread.currentThread().getName() + "\t" + e);
            });
        });
    }
    ex.shutdown();

}

}
```
this code will print "call time 23"
change Executors.newFixedThreadPool(20); to Executors.newFixedThreadPool(1);
at this time print "call time 14"
Thank you for you time

[RobWin]

Hello,

the CircuitBreaker is thread-safe as follows :

• The state of a CircuitBreaker is stored in a AtomicReference
• The CircuitBreaker uses atomic operations to update the state with side-effect-free functions.
• Updates to the RingBitSet (ring buffer) are synchronized.

That means atomicity should be guaranteed and only one thread is able to update the state or ring buffer at a point in time.

BUT the CircuitBreaker does not synchronize the executed function. That means the function itself is not part of the critical section.
Otherwise a CircuitBreaker would introduce a huge performance penalty and bottleneck. A slow function would have a huge negative impact to the overall performance/throughput.

Summary:
If 20 concurrent threads ask for the permission to execute a function and the state of the CircuitBreaker is closed, all threads are allowed to invoke the function. Even if the ringBufferSizeInClosedState is 15. ringBufferSizeInClosedState does not control that only 15 calls are allowed to run concurrently. If more than 5% of the function calls fail, the CircuitBreaker transitions to OPEN and further permission requests are rejected.
NOTE: Please keep in mind that failureRateThreshold is a percentage value. You configured 5% and not 5 calls.

If you want to restrict the number of concurrent threads, please use a Bulkhead. You can combine a Bulkhead and a CircuitBreaker.

[srchen1987]

Thank you for RobWin's reply,I know your means,I know failureRateThreshold is percentage.
I have tried Bulkhead before ,But I don't want to control concurrency.
I still struggle with "A CircuitBreaker instance is thread-safe can be used to decorate multiple requests.".
can you remove it ?

[RobWin]

But the internal state of a CircuitBreaker is thread-safe and a CircuitBreaker MUST be used to decorate multiple functional calls. Otherwise it does not make sense at all.

The implementation of Polly is similar -> https://github.com/App-vNext/Polly/wiki/Circuit-Breaker#thread-safety-and-locking

[srchen1987]

a CircuitBreaker MUST be used to decorate multiple functional calls. Otherwise it does not make sense at all.
I don't think so,My usage scenarios:
I can use the TimeLimiter to implement timeout control.
function A(){
call B();
}
function B(){
//http://www.xxx.com/user/info.do
request timeout 300ms.

}
when I call the B funtion 5% timeout ,I want to open the CircuitBreaker.Bug at this time 100 requests concurrent access the A function ,I can't promise to open CircuitBreaker after five requests.
How can I do ?

[storozhukBM]

@RobWin thank you for answers above. I found them fully reasonable and descriptive. But maybe we encounter this problem because of our common knowledge of the low level circuit breaker API and architecture, that is not typically known for our users.
@srchen1987 thank you also for this question. I understand your problem and behavior that you want to achieve. Unfortunately, the thing that you are asking is not possible from a physics standpoint, because it demands the prediction of future events, which is not achievable. I'll do my best to describe the actual situation that you faced, but please fill free to ask any additional questions.
As you understand, the circuit breaker is a thread-safe data structure that collects information about already happened events that are results of a particular action and exposes its opinion about the safety of performing the same action in the current moment.
So let us use sequence diagrams to see what happened in the case of a single thread in your pool:

As you can see with a single thread, everything is simple because all operations are sequential.
But in multithreaded mode, everything is a lot more complicated:

In your case with 20 threads, circuit breaker gives you 20 permissions because no error happened at this given point of time and even after a first cycle some threads actually managed to get more permissions before all others registered their failures from the previous cycle.

Hopefully, these diagrams will help you understand that behavior that you see is expected, and the only way you can deal with would be some sort of concurrency restrictions as bulkhead or rate limiter.

I can also recommend my article about circuit breaker implementation details that can help with the understanding of its capabilities https://medium.com/@storozhuk.b.m/circuit-breaker-implementation-in-resilience4j-992af908c413

[srchen1987]

@RobWin @storozhukBM Thanks again. Let me think about it .

[srchen1987]

@storozhukBM @RobWin
Thanks again，I just looked at the above answer，I knew
the thing that you are asking is not possible from a physics standpoint, because it demands the prediction of future events, which is not achievable.
because
In my case with 20 threads, circuit breaker gives me 20 permissions because no error happened at this given point of time and even after a first cycle some threads actually managed to get more permissions before all others registered their failures from the previous cycle.