From 5fede3bcbed3dcafb2d1d395e13fa12fdfa3a0e3 Mon Sep 17 00:00:00 2001 From: Ryan Baxter <524254+ryanjbaxter@users.noreply.github.com> Date: Fri, 8 Sep 2023 17:10:36 -0400 Subject: [PATCH 1/4] Updating include --- README.adoc | 2 -- docs/src/main/asciidoc/spring-cloud-circuitbreaker.adoc | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/README.adoc b/README.adoc index 1598c884..df2bd41d 100644 --- a/README.adoc +++ b/README.adoc @@ -438,7 +438,6 @@ public Customizer slowCustomizer() { == Building - :jdkversion: 17 === Basic Compile and Test @@ -519,7 +518,6 @@ The generated eclipse projects can be imported by selecting `import existing pro from the `file` menu. - == Contributing :spring-cloud-build-branch: master diff --git a/docs/src/main/asciidoc/spring-cloud-circuitbreaker.adoc b/docs/src/main/asciidoc/spring-cloud-circuitbreaker.adoc index 18dfa995..3e98d653 100755 --- a/docs/src/main/asciidoc/spring-cloud-circuitbreaker.adoc +++ b/docs/src/main/asciidoc/spring-cloud-circuitbreaker.adoc @@ -15,7 +15,7 @@ include::spring-cloud-circuitbreaker-spring-retry.adoc[] == Building -include::https://raw.githubusercontent.com/spring-cloud/spring-cloud-build/4.0.x/docs/src/main/asciidoc/building-jdk8.adoc[] +include::https://raw.githubusercontent.com/spring-cloud/spring-cloud-build/4.0.x/docs/src/main/asciidoc/building.adoc[] == Contributing From 6a38f818d163b3b55c9ae288e9dbc4ff5ca8e159 Mon Sep 17 00:00:00 2001 From: buildmaster Date: Thu, 21 Sep 2023 01:06:45 +0000 Subject: [PATCH 2/4] Bumping versions --- README.adoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.adoc b/README.adoc index df2bd41d..5d0742fe 100644 --- a/README.adoc +++ b/README.adoc @@ -537,7 +537,7 @@ author credit if we do. Active contributors might be asked to join the core tea given the ability to merge pull requests. === Code of Conduct -This project adheres to the Contributor Covenant https://github.com/spring-cloud/spring-cloud-build/blob/master/docs/src/main/asciidoc/code-of-conduct.adoc[code of +This project adheres to the Contributor Covenant https://github.com/spring-cloud/spring-cloud-build/blob/{spring-cloud-build-branch}/docs/src/main/asciidoc/code-of-conduct.adoc[code of conduct]. By participating, you are expected to uphold this code. Please report unacceptable behavior to spring-code-of-conduct@pivotal.io. @@ -548,7 +548,7 @@ added after the original pull request but before a merge. * Use the Spring Framework code format conventions. If you use Eclipse you can import formatter settings using the `eclipse-code-formatter.xml` file from the - https://raw.githubusercontent.com/spring-cloud/spring-cloud-build/master/spring-cloud-dependencies-parent/eclipse-code-formatter.xml[Spring + https://raw.githubusercontent.com/spring-cloud/spring-cloud-build/{spring-cloud-build-branch}/spring-cloud-dependencies-parent/eclipse-code-formatter.xml[Spring Cloud Build] project. If using IntelliJ, you can use the https://plugins.jetbrains.com/plugin/6546[Eclipse Code Formatter Plugin] to import the same file. From 37bdfe635284f65bb84e7bb9cf214d8c62d74533 Mon Sep 17 00:00:00 2001 From: Renette Ros Date: Wed, 8 Feb 2023 13:50:46 +0200 Subject: [PATCH 3/4] Fix timelimiter not interrupting bulkhead calls Fixes gh-163 --- .../Resilience4JCircuitBreaker.java | 50 +++-- .../Resilience4jBulkheadProvider.java | 25 ++- ...BulkheadAndTimeLimiterIntegrationTest.java | 172 ++++++++++++++++++ .../Resilience4JCircuitBreakerTest.java | 22 +++ 4 files changed, 253 insertions(+), 16 deletions(-) create mode 100644 spring-cloud-circuitbreaker-resilience4j/src/test/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4JBulkheadAndTimeLimiterIntegrationTest.java diff --git a/spring-cloud-circuitbreaker-resilience4j/src/main/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4JCircuitBreaker.java b/spring-cloud-circuitbreaker-resilience4j/src/main/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4JCircuitBreaker.java index b5bf389c..f2c9c185 100644 --- a/spring-cloud-circuitbreaker-resilience4j/src/main/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4JCircuitBreaker.java +++ b/spring-cloud-circuitbreaker-resilience4j/src/main/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4JCircuitBreaker.java @@ -36,6 +36,7 @@ * @author Ryan Baxter * @author Andrii Bohutskyi * @author 荒 + * @author Renette Ros */ public class Resilience4JCircuitBreaker implements CircuitBreaker { @@ -96,32 +97,53 @@ public T run(Supplier toRun, Function fallback) { this.circuitBreakerConfig, tags); circuitBreakerCustomizer.ifPresent(customizer -> customizer.customize(defaultCircuitBreaker)); if (bulkheadProvider != null) { - return bulkheadProvider.run(this.groupName, toRun, fallback, defaultCircuitBreaker, timeLimiter, tags); + + if (executorService != null) { + Supplier> futureSupplier = () -> executorService.submit(toRun::get); + Callable timeLimitedCall = TimeLimiter.decorateFutureSupplier(timeLimiter, futureSupplier); + Callable bulkheadCall = bulkheadProvider.decorateCallable(this.groupName, tags, timeLimitedCall); + Callable circuitBreakerCall = io.github.resilience4j.circuitbreaker.CircuitBreaker + .decorateCallable(defaultCircuitBreaker, bulkheadCall); + return getAndApplyFallback(circuitBreakerCall, fallback); + } + else { + Callable bulkheadCall = bulkheadProvider.decorateCallable(this.groupName, tags, toRun::get); + Callable circuitBreakerCall = io.github.resilience4j.circuitbreaker.CircuitBreaker + .decorateCallable(defaultCircuitBreaker, bulkheadCall); + return getAndApplyFallback(circuitBreakerCall, fallback); + } } else { if (executorService != null) { Supplier> futureSupplier = () -> executorService.submit(toRun::get); - Callable restrictedCall = TimeLimiter.decorateFutureSupplier(timeLimiter, futureSupplier); + Callable restrictedCall = TimeLimiter.decorateFutureSupplier(timeLimiter, futureSupplier); Callable callable = io.github.resilience4j.circuitbreaker.CircuitBreaker .decorateCallable(defaultCircuitBreaker, restrictedCall); - try { - return callable.call(); - } - catch (Throwable t) { - return fallback.apply(t); - } + return getAndApplyFallback(callable, fallback); } else { Supplier decorator = io.github.resilience4j.circuitbreaker.CircuitBreaker .decorateSupplier(defaultCircuitBreaker, toRun); - try { - return decorator.get(); - } - catch (Throwable t) { - return fallback.apply(t); - } + return getAndApplyFallback(decorator, fallback); } + } + } + + private static T getAndApplyFallback(Supplier supplier, Function fallback) { + try { + return supplier.get(); + } + catch (Throwable t) { + return fallback.apply(t); + } + } + private static T getAndApplyFallback(Callable callable, Function fallback) { + try { + return callable.call(); + } + catch (Throwable t) { + return fallback.apply(t); } } diff --git a/spring-cloud-circuitbreaker-resilience4j/src/main/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4jBulkheadProvider.java b/spring-cloud-circuitbreaker-resilience4j/src/main/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4jBulkheadProvider.java index 32b4f654..70ab52f9 100644 --- a/spring-cloud-circuitbreaker-resilience4j/src/main/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4jBulkheadProvider.java +++ b/spring-cloud-circuitbreaker-resilience4j/src/main/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4jBulkheadProvider.java @@ -37,6 +37,7 @@ /** * @author Andrii Bohutskyi + * @author Renette Ros */ public class Resilience4jBulkheadProvider { @@ -120,8 +121,7 @@ private Supplier> decorateBulkhead(final String id, final Resilience4jBulkheadConfigurationBuilder.BulkheadConfiguration configuration = configurations .computeIfAbsent(id, defaultConfiguration); - if (semaphoreDefaultBulkhead - || (bulkheadRegistry.find(id).isPresent() && !threadPoolBulkheadRegistry.find(id).isPresent())) { + if (useSemaphoreBulkhead(id)) { Bulkhead bulkhead = bulkheadRegistry.bulkhead(id, configuration.getBulkheadConfig(), tags); Supplier> completionStageSupplier = () -> CompletableFuture.supplyAsync(supplier); return Bulkhead.decorateCompletionStage(bulkhead, completionStageSupplier); @@ -133,6 +133,27 @@ private Supplier> decorateBulkhead(final String id, final } } + public Callable decorateCallable(final String id, final Map tags, + final Callable callable) { + Resilience4jBulkheadConfigurationBuilder.BulkheadConfiguration configuration = configurations + .computeIfAbsent(id, defaultConfiguration); + + if (useSemaphoreBulkhead(id)) { + Bulkhead bulkhead = bulkheadRegistry.bulkhead(id, configuration.getBulkheadConfig(), tags); + return Bulkhead.decorateCallable(bulkhead, callable); + } + else { + ThreadPoolBulkhead threadPoolBulkhead = threadPoolBulkheadRegistry.bulkhead(id, + configuration.getThreadPoolBulkheadConfig(), tags); + return () -> threadPoolBulkhead.decorateCallable(callable).get().toCompletableFuture().get(); + } + } + + private boolean useSemaphoreBulkhead(String id) { + return semaphoreDefaultBulkhead + || (bulkheadRegistry.find(id).isPresent() && threadPoolBulkheadRegistry.find(id).isEmpty()); + } + private Callable decorateTimeLimiter(final Supplier> supplier, TimeLimiter timeLimiter) { final Supplier> futureSupplier = () -> supplier.get().toCompletableFuture(); return timeLimiter.decorateFutureSupplier(futureSupplier); diff --git a/spring-cloud-circuitbreaker-resilience4j/src/test/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4JBulkheadAndTimeLimiterIntegrationTest.java b/spring-cloud-circuitbreaker-resilience4j/src/test/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4JBulkheadAndTimeLimiterIntegrationTest.java new file mode 100644 index 00000000..62a86112 --- /dev/null +++ b/spring-cloud-circuitbreaker-resilience4j/src/test/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4JBulkheadAndTimeLimiterIntegrationTest.java @@ -0,0 +1,172 @@ +/* + * Copyright 2013-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.circuitbreaker.resilience4j; + +import java.time.Duration; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; + +import io.github.resilience4j.timelimiter.TimeLimiterConfig; +import org.junit.Test; +import org.junit.jupiter.api.Assertions; +import org.junit.runner.RunWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.client.circuitbreaker.CircuitBreaker; +import org.springframework.cloud.client.circuitbreaker.CircuitBreakerFactory; +import org.springframework.cloud.client.circuitbreaker.Customizer; +import org.springframework.cloud.client.circuitbreaker.NoFallbackAvailableException; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.stereotype.Service; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.web.bind.annotation.RestController; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests that time limiter threads are interrupted correctly when used with a bulkhead + * + * @author Renette Ros + */ +@RunWith(SpringRunner.class) +@SpringBootTest(classes = Resilience4JBulkheadAndTimeLimiterIntegrationTest.Application.class) +@DirtiesContext +public class Resilience4JBulkheadAndTimeLimiterIntegrationTest { + + public static final String SLOW_BULKHEAD = "slowBulkhead"; + + public static final String SLOW_THREAD_POOL_BULKHEAD = "slowThreadPoolBulkhead"; + + @Autowired + Application.DemoService service; + + @Test + public void testBulkheadThreadInterrupted() { + InterruptibleTask interruptibleTask = new InterruptibleTask(); + NoFallbackAvailableException exception = Assertions.assertThrows(NoFallbackAvailableException.class, + () -> service.bulkheadTimeout(interruptibleTask)); + assertThat(exception).hasCauseInstanceOf(TimeoutException.class); + Assertions.assertTrue(interruptibleTask.interrupted(), "Thread should be interrupted"); + } + + @Test + public void testThreadPoolBulkheadThreadInterrupted() { + InterruptibleTask interruptibleTask = new InterruptibleTask(); + NoFallbackAvailableException exception = Assertions.assertThrows(NoFallbackAvailableException.class, + () -> service.threadPoolBulkheadTimeout(interruptibleTask)); + assertThat(exception).hasRootCauseExactlyInstanceOf(TimeoutException.class); + Assertions.assertTrue(interruptibleTask.interrupted(), "Thread should be interrupted"); + } + + @Test + public void testBulkheadFastCallNotInterrupted() { + Assertions.assertEquals(Application.CompletionStatus.SUCCESS, service.bulkheadFast()); + } + + @Test + public void testThreadPoolFastCallNotInterrupted() { + Assertions.assertEquals(Application.CompletionStatus.SUCCESS, service.threadPoolBulkheadFast()); + } + + static class InterruptibleTask { + + private final AtomicBoolean interrupted = new AtomicBoolean(false); + + public Application.CompletionStatus run(int sleepMillis) { + try { + Thread.sleep(sleepMillis); + return Application.CompletionStatus.SUCCESS; + } + catch (InterruptedException ignored) { + interrupted.set(true); + Thread.currentThread().interrupt(); + return Application.CompletionStatus.INTERRUPTED; + } + } + + boolean interrupted() { + return interrupted.get(); + } + + } + + @Configuration(proxyBeanMethods = false) + @EnableAutoConfiguration + @RestController + protected static class Application { + + @Bean + public Customizer slowBulkheadCustomizer() { + TimeLimiterConfig timeLimiterConfig = TimeLimiterConfig.custom().timeoutDuration(Duration.ofMillis(500)) + .build(); + return circuitBreakerFactory -> circuitBreakerFactory + .configure(builder -> builder.timeLimiterConfig(timeLimiterConfig), SLOW_BULKHEAD); + } + + @Bean + public Customizer bulkheadProviderCustomizer() { + return provider -> { + provider.addBulkheadCustomizer(bulkhead -> { + }, SLOW_BULKHEAD); + provider.addThreadPoolBulkheadCustomizer(builder -> { + }, SLOW_THREAD_POOL_BULKHEAD); + }; + } + + enum CompletionStatus { + + SUCCESS, INTERRUPTED + + } + + @Service + public static class DemoService { + + private final CircuitBreaker bulkhead; + + private final CircuitBreaker threadPoolBulkhead; + + DemoService(CircuitBreakerFactory cbFactory) { + this.bulkhead = cbFactory.create(SLOW_BULKHEAD); + this.threadPoolBulkhead = cbFactory.create(SLOW_THREAD_POOL_BULKHEAD); + } + + public CompletionStatus bulkheadTimeout(InterruptibleTask interruptibleTask) { + return bulkhead.run(() -> interruptibleTask.run(2_000)); + } + + public CompletionStatus threadPoolBulkheadTimeout(InterruptibleTask interruptibleTask) { + return threadPoolBulkhead.run(() -> interruptibleTask.run(2_000)); + } + + public CompletionStatus bulkheadFast() { + return bulkhead.run(() -> new InterruptibleTask().run(100)); + } + + public CompletionStatus threadPoolBulkheadFast() { + return threadPoolBulkhead.run(() -> new InterruptibleTask().run(100)); + } + + } + + } + +} diff --git a/spring-cloud-circuitbreaker-resilience4j/src/test/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4JCircuitBreakerTest.java b/spring-cloud-circuitbreaker-resilience4j/src/test/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4JCircuitBreakerTest.java index a8e57541..388b8572 100644 --- a/spring-cloud-circuitbreaker-resilience4j/src/test/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4JCircuitBreakerTest.java +++ b/spring-cloud-circuitbreaker-resilience4j/src/test/java/org/springframework/cloud/circuitbreaker/resilience4j/Resilience4JCircuitBreakerTest.java @@ -99,6 +99,17 @@ public void runWithBulkheadProvider() { assertThat(cb.run(() -> "foobar")).isEqualTo("foobar"); } + @Test + public void runWithBulkheadProviderAndNoThreadPool() { + properties.setDisableThreadPool(true); + CircuitBreaker cb = new Resilience4JCircuitBreakerFactory(CircuitBreakerRegistry.ofDefaults(), + TimeLimiterRegistry.ofDefaults(), + new Resilience4jBulkheadProvider(ThreadPoolBulkheadRegistry.ofDefaults(), BulkheadRegistry.ofDefaults(), + new Resilience4JConfigurationProperties()), + properties).create("foo"); + assertThat(cb.run(() -> "foobar")).isEqualTo("foobar"); + } + @Test public void runWithBulkheadProviderAndGroupName() { CircuitBreaker cb = new Resilience4JCircuitBreakerFactory(CircuitBreakerRegistry.ofDefaults(), @@ -109,6 +120,17 @@ public void runWithBulkheadProviderAndGroupName() { assertThat(cb.run(() -> "foobar")).isEqualTo("foobar"); } + @Test + public void runWithBulkheadProviderAndGroupNameAndNoThreadPool() { + properties.setDisableThreadPool(true); + CircuitBreaker cb = new Resilience4JCircuitBreakerFactory(CircuitBreakerRegistry.ofDefaults(), + TimeLimiterRegistry.ofDefaults(), + new Resilience4jBulkheadProvider(ThreadPoolBulkheadRegistry.ofDefaults(), BulkheadRegistry.ofDefaults(), + new Resilience4JConfigurationProperties()), + properties).create("foo", "groupFoo"); + assertThat(cb.run(() -> "foobar")).isEqualTo("foobar"); + } + @Test public void runWithFallbackBulkheadProvider() { CircuitBreaker cb = new Resilience4JCircuitBreakerFactory(CircuitBreakerRegistry.ofDefaults(), From e67b11c8deead19ae2ca77197f3b1cd58e760e92 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Wed, 29 Nov 2023 09:06:05 -0500 Subject: [PATCH 4/4] Build 3.0.x branch --- .github/workflows/maven.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/maven.yaml b/.github/workflows/maven.yaml index 0bdc4c79..303d38c8 100644 --- a/.github/workflows/maven.yaml +++ b/.github/workflows/maven.yaml @@ -5,9 +5,9 @@ name: Build on: push: - branches: [ main, 2.1.x ] + branches: [ main, 3.0.x, 2.1.x ] pull_request: - branches: [ main, 2.1.x ] + branches: [ main, 3.0.x, 2.1.x ] jobs: build: