From 02a7c22f40f7e4b992e277ae58c60421b6a49421 Mon Sep 17 00:00:00 2001 From: Moritz Halbritter Date: Thu, 3 Aug 2023 17:25:28 +0200 Subject: [PATCH] Revert "Use virtual threads in JmsHealthIndicator if enabled" This reverts commit 6fc585c5d2e19ad1a07b65bbb6decddae78b854e. --- ...JmsHealthContributorAutoConfiguration.java | 19 +--------- ...althContributorAutoConfigurationTests.java | 18 --------- .../boot/actuate/jms/JmsHealthIndicator.java | 38 ++----------------- .../actuate/jms/JmsHealthIndicatorTests.java | 18 +++------ 4 files changed, 11 insertions(+), 82 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/jms/JmsHealthContributorAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/jms/JmsHealthContributorAutoConfiguration.java index 64ba0e4568..aa7d72ab27 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/jms/JmsHealthContributorAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/jms/JmsHealthContributorAutoConfiguration.java @@ -16,7 +16,6 @@ package org.springframework.boot.actuate.autoconfigure.jms; -import java.time.Duration; import java.util.Map; import jakarta.jms.ConnectionFactory; @@ -32,16 +31,12 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.jms.activemq.ActiveMQAutoConfiguration; import org.springframework.boot.autoconfigure.jms.artemis.ArtemisAutoConfiguration; -import org.springframework.boot.autoconfigure.thread.Threading; import org.springframework.context.annotation.Bean; -import org.springframework.core.env.Environment; -import org.springframework.core.task.SimpleAsyncTaskExecutor; /** * {@link EnableAutoConfiguration Auto-configuration} for {@link JmsHealthIndicator}. * * @author Stephane Nicoll - * @author Moritz Halbritter * @since 2.0.0 */ @AutoConfiguration(after = { ActiveMQAutoConfiguration.class, ArtemisAutoConfiguration.class }) @@ -51,10 +46,8 @@ import org.springframework.core.task.SimpleAsyncTaskExecutor; public class JmsHealthContributorAutoConfiguration extends CompositeHealthContributorConfiguration { - private static final Duration TIMEOUT = Duration.ofSeconds(5); - - public JmsHealthContributorAutoConfiguration(Environment environment) { - super((connectionFactory) -> new JmsHealthIndicator(connectionFactory, getTaskExecutor(environment), TIMEOUT)); + public JmsHealthContributorAutoConfiguration() { + super(JmsHealthIndicator::new); } @Bean @@ -63,12 +56,4 @@ public class JmsHealthContributorAutoConfiguration return createContributor(connectionFactories); } - private static SimpleAsyncTaskExecutor getTaskExecutor(Environment environment) { - SimpleAsyncTaskExecutor taskExecutor = new SimpleAsyncTaskExecutor("jms-health-indicator"); - if (Threading.VIRTUAL.isActive(environment)) { - taskExecutor.setVirtualThreads(true); - } - return taskExecutor; - } - } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/jms/JmsHealthContributorAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/jms/JmsHealthContributorAutoConfigurationTests.java index e630b95c78..c8fa3c6962 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/jms/JmsHealthContributorAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/jms/JmsHealthContributorAutoConfigurationTests.java @@ -17,8 +17,6 @@ package org.springframework.boot.actuate.autoconfigure.jms; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.condition.EnabledForJreRange; -import org.junit.jupiter.api.condition.JRE; import org.springframework.boot.actuate.autoconfigure.health.HealthContributorAutoConfiguration; import org.springframework.boot.actuate.jms.JmsHealthIndicator; @@ -26,9 +24,6 @@ import org.springframework.boot.actuate.ldap.LdapHealthIndicator; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.jms.artemis.ArtemisAutoConfiguration; import org.springframework.boot.test.context.runner.ApplicationContextRunner; -import org.springframework.boot.testsupport.assertj.SimpleAsyncTaskExecutorAssert; -import org.springframework.core.task.SimpleAsyncTaskExecutor; -import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -36,7 +31,6 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests for {@link JmsHealthContributorAutoConfiguration}. * * @author Phillip Webb - * @author Moritz Halbritter */ class JmsHealthContributorAutoConfigurationTests { @@ -49,18 +43,6 @@ class JmsHealthContributorAutoConfigurationTests { this.contextRunner.run((context) -> assertThat(context).hasSingleBean(JmsHealthIndicator.class)); } - @Test - @EnabledForJreRange(min = JRE.JAVA_21) - void shouldUseVirtualThreadsIfEnabled() { - this.contextRunner.withPropertyValues("spring.threads.virtual.enabled=true").run((context) -> { - JmsHealthIndicator jmsHealthIndicator = context.getBean(JmsHealthIndicator.class); - assertThat(jmsHealthIndicator).isNotNull(); - Object taskExecutor = ReflectionTestUtils.getField(jmsHealthIndicator, "taskExecutor"); - assertThat(taskExecutor).isInstanceOf(SimpleAsyncTaskExecutor.class); - SimpleAsyncTaskExecutorAssert.assertThat((SimpleAsyncTaskExecutor) taskExecutor).usesVirtualThreads(); - }); - } - @Test void runWhenDisabledShouldNotCreateIndicator() { this.contextRunner.withPropertyValues("management.health.jms.enabled:false") diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/jms/JmsHealthIndicator.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/jms/JmsHealthIndicator.java index fd1322f59c..3c8f961d53 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/jms/JmsHealthIndicator.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/jms/JmsHealthIndicator.java @@ -16,7 +16,6 @@ package org.springframework.boot.actuate.jms; -import java.time.Duration; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -29,15 +28,11 @@ import org.apache.commons.logging.LogFactory; import org.springframework.boot.actuate.health.AbstractHealthIndicator; import org.springframework.boot.actuate.health.Health; import org.springframework.boot.actuate.health.HealthIndicator; -import org.springframework.core.log.LogMessage; -import org.springframework.core.task.AsyncTaskExecutor; -import org.springframework.core.task.SimpleAsyncTaskExecutor; /** * {@link HealthIndicator} for a JMS {@link ConnectionFactory}. * * @author Stephane Nicoll - * @author Moritz Halbritter * @since 2.0.0 */ public class JmsHealthIndicator extends AbstractHealthIndicator { @@ -46,33 +41,9 @@ public class JmsHealthIndicator extends AbstractHealthIndicator { private final ConnectionFactory connectionFactory; - private final AsyncTaskExecutor taskExecutor; - - private final Duration timeout; - - /** - * Creates a new {@link JmsHealthIndicator}, using a {@link SimpleAsyncTaskExecutor} - * and a timeout of 5 seconds. - * @param connectionFactory the connection factory - * @deprecated since 3.2.0 for removal in 3.4.0 in favor of - * {@link #JmsHealthIndicator(ConnectionFactory, AsyncTaskExecutor, Duration)} - */ - @Deprecated(since = "3.2.0", forRemoval = true) public JmsHealthIndicator(ConnectionFactory connectionFactory) { - this(connectionFactory, new SimpleAsyncTaskExecutor("jms-health-indicator"), Duration.ofSeconds(5)); - } - - /** - * Creates a new {@link JmsHealthIndicator}. - * @param connectionFactory the connection factory - * @param taskExecutor the task executor used to run timeout checks - * @param timeout the connection timeout - */ - public JmsHealthIndicator(ConnectionFactory connectionFactory, AsyncTaskExecutor taskExecutor, Duration timeout) { super("JMS health check failed"); this.connectionFactory = connectionFactory; - this.taskExecutor = taskExecutor; - this.timeout = timeout; } @Override @@ -94,19 +65,18 @@ public class JmsHealthIndicator extends AbstractHealthIndicator { } void start() throws JMSException { - JmsHealthIndicator.this.taskExecutor.execute(() -> { + new Thread(() -> { try { - if (!this.latch.await(JmsHealthIndicator.this.timeout.toMillis(), TimeUnit.MILLISECONDS)) { + if (!this.latch.await(5, TimeUnit.SECONDS)) { JmsHealthIndicator.this.logger - .warn(LogMessage.format("Connection failed to start within %s and will be closed.", - JmsHealthIndicator.this.timeout)); + .warn("Connection failed to start within 5 seconds and will be closed."); closeConnection(); } } catch (InterruptedException ex) { Thread.currentThread().interrupt(); } - }); + }, "jms-health-indicator").start(); this.connection.start(); this.latch.countDown(); } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/jms/JmsHealthIndicatorTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/jms/JmsHealthIndicatorTests.java index f52082859d..2e005d591d 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/jms/JmsHealthIndicatorTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/jms/JmsHealthIndicatorTests.java @@ -16,8 +16,6 @@ package org.springframework.boot.actuate.jms; -import java.time.Duration; - import jakarta.jms.Connection; import jakarta.jms.ConnectionFactory; import jakarta.jms.ConnectionMetaData; @@ -28,8 +26,6 @@ import org.mockito.stubbing.Answer; import org.springframework.boot.actuate.health.Health; import org.springframework.boot.actuate.health.Status; -import org.springframework.core.task.AsyncTaskExecutor; -import org.springframework.core.task.SimpleAsyncTaskExecutor; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.BDDMockito.given; @@ -45,10 +41,6 @@ import static org.mockito.Mockito.mock; */ class JmsHealthIndicatorTests { - private static final Duration TIMEOUT = Duration.ofMillis(100); - - private final AsyncTaskExecutor taskExecutor = new SimpleAsyncTaskExecutor(); - @Test void jmsBrokerIsUp() throws JMSException { ConnectionMetaData connectionMetaData = mock(ConnectionMetaData.class); @@ -57,7 +49,7 @@ class JmsHealthIndicatorTests { given(connection.getMetaData()).willReturn(connectionMetaData); ConnectionFactory connectionFactory = mock(ConnectionFactory.class); given(connectionFactory.createConnection()).willReturn(connection); - JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory, this.taskExecutor, TIMEOUT); + JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory); Health health = indicator.health(); assertThat(health.getStatus()).isEqualTo(Status.UP); assertThat(health.getDetails()).containsEntry("provider", "JMS test provider"); @@ -68,7 +60,7 @@ class JmsHealthIndicatorTests { void jmsBrokerIsDown() throws JMSException { ConnectionFactory connectionFactory = mock(ConnectionFactory.class); given(connectionFactory.createConnection()).willThrow(new JMSException("test", "123")); - JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory, this.taskExecutor, TIMEOUT); + JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory); Health health = indicator.health(); assertThat(health.getStatus()).isEqualTo(Status.DOWN); assertThat(health.getDetails()).doesNotContainKey("provider"); @@ -82,7 +74,7 @@ class JmsHealthIndicatorTests { given(connection.getMetaData()).willReturn(connectionMetaData); ConnectionFactory connectionFactory = mock(ConnectionFactory.class); given(connectionFactory.createConnection()).willReturn(connection); - JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory, this.taskExecutor, TIMEOUT); + JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory); Health health = indicator.health(); assertThat(health.getStatus()).isEqualTo(Status.DOWN); assertThat(health.getDetails()).doesNotContainKey("provider"); @@ -98,7 +90,7 @@ class JmsHealthIndicatorTests { given(connection.getMetaData()).willReturn(connectionMetaData); willThrow(new JMSException("Could not start", "123")).given(connection).start(); given(connectionFactory.createConnection()).willReturn(connection); - JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory, this.taskExecutor, TIMEOUT); + JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory); Health health = indicator.health(); assertThat(health.getStatus()).isEqualTo(Status.DOWN); assertThat(health.getDetails()).doesNotContainKey("provider"); @@ -117,7 +109,7 @@ class JmsHealthIndicatorTests { }).given(connection).close(); ConnectionFactory connectionFactory = mock(ConnectionFactory.class); given(connectionFactory.createConnection()).willReturn(connection); - JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory, this.taskExecutor, TIMEOUT); + JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory); Health health = indicator.health(); assertThat(health.getStatus()).isEqualTo(Status.DOWN); assertThat((String) health.getDetails().get("error")).contains("Connection closed");