spring-cloud / spring-cloud-circuitbreaker

Spring Cloud Circuit Breaker API and Implementations
Apache License 2.0
329 stars 110 forks source link

Resilience4j causes thread leaks #114

Closed tsingheng closed 3 years ago

tsingheng commented 3 years ago

spring-cloud-circuitbreaker-resilience4j-2.0.1

  1. I found that resilience4j creates a thread pool for every feign method. There are hundreds of feign method in my project. So resilience4j will create thousands of bulkhead thread. org.springframework.cloud.circuitbreaker.resilience4j.Resilience4jBulkheadProvider(line 115)

    private <T> Supplier<CompletionStage<T>> decorateBulkhead(final String id, final Supplier<T> supplier) {
        // I would like id to be the name of @FeignClient
        Resilience4jBulkheadConfigurationBuilder.BulkheadConfiguration configuration = configurations
                .computeIfAbsent(id, defaultConfiguration);
    
        if (bulkheadRegistry.find(id).isPresent() && !threadPoolBulkheadRegistry.find(id).isPresent()) {
            Bulkhead bulkhead = bulkheadRegistry.bulkhead(id, configuration.getBulkheadConfig());
            CompletableFuture<T> asyncCall = CompletableFuture.supplyAsync(supplier);
            return Bulkhead.decorateCompletionStage(bulkhead, () -> asyncCall);
        }
        else {
            ThreadPoolBulkhead threadPoolBulkhead = threadPoolBulkheadRegistry.bulkhead(id,
                    configuration.getThreadPoolBulkheadConfig());
            return threadPoolBulkhead.decorateSupplier(supplier);
        }
    }
  2. TimeLimiter creates newSingleThreadScheduledExecutor for every feign invoke. It may lead thread leaks. org.springframework.cloud.circuitbreaker.resilience4j.Resilience4jBulkheadProvider(line 104)

    public <T> T run(String id, Supplier<T> toRun, Function<Throwable, T> fallback, CircuitBreaker circuitBreaker,
            TimeLimiter timeLimiter) {
        Supplier<CompletionStage<T>> bulkheadCall = decorateBulkhead(id, toRun);
        Supplier<CompletionStage<T>> timeLimiterCall = timeLimiter
                .decorateCompletionStage(Executors.newSingleThreadScheduledExecutor(), bulkheadCall);
        Supplier<CompletionStage<T>> circuitBreakerCall = circuitBreaker.decorateCompletionStage(timeLimiterCall);
        try {
            return circuitBreakerCall.get().toCompletableFuture().get();
        }
        catch (Exception e) {
            System.out.println("exception " + e.getMessage());
            return fallback.apply(e);
        }
    }
spencergibb commented 3 years ago

Maybe bulkhead isn't the proper strategy for hundreds of feign methods and it should be moved to a different layer. Also see #113 for the option to default to semaphore.