From 5893786cbb96fef962be95b70f9d7d2b0737ef9e Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Thu, 19 Mar 2020 12:12:27 +0100 Subject: [PATCH] Polish "Add 'threads' configuration group for embedded containers" See gh-19475 --- .../autoconfigure/web/ServerProperties.java | 2 +- .../JettyWebServerFactoryCustomizer.java | 14 ++++----- .../TomcatWebServerFactoryCustomizer.java | 2 +- .../UndertowWebServerFactoryCustomizer.java | 2 +- .../web/ServerPropertiesTests.java | 29 ------------------- .../JettyWebServerFactoryCustomizerTests.java | 14 ++++----- .../src/main/resources/application.properties | 2 +- 7 files changed, 18 insertions(+), 47 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java index af8e3ab86d0..1b254ae21c1 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java @@ -1146,7 +1146,7 @@ public class ServerProperties { } @Deprecated - @DeprecatedConfigurationProperty(replacement = "server.jetty.threads.maxQueueCapacity") + @DeprecatedConfigurationProperty(replacement = "server.jetty.threads.max-queue-capacity") public Integer getMaxQueueCapacity() { return this.getThreads().getMaxQueueCapacity(); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizer.java index 7f55f5f9440..ba8b2e2681c 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizer.java @@ -76,8 +76,8 @@ public class JettyWebServerFactoryCustomizer public void customize(ConfigurableJettyWebServerFactory factory) { ServerProperties properties = this.serverProperties; ServerProperties.Jetty jettyProperties = properties.getJetty(); - ServerProperties.Jetty.Threads threadProperties = jettyProperties.getThreads(); factory.setUseForwardHeaders(getOrDeduceUseForwardHeaders()); + ServerProperties.Jetty.Threads threadProperties = jettyProperties.getThreads(); factory.setThreadPool(determineThreadPool(jettyProperties.getThreads())); PropertyMapper propertyMapper = PropertyMapper.get(); propertyMapper.from(threadProperties::getAcceptors).whenNonNull().to(factory::setAcceptors); @@ -142,12 +142,12 @@ public class JettyWebServerFactoryCustomizer }); } - private ThreadPool determineThreadPool(ServerProperties.Jetty.Threads threadProperties) { - BlockingQueue queue = determineBlockingQueue(threadProperties.getMaxQueueCapacity()); - int maxThreadCount = (threadProperties.getMax() > 0) ? threadProperties.getMax() : 200; - int minThreadCount = (threadProperties.getMin() > 0) ? threadProperties.getMin() : 8; - int threadIdleTimeout = (threadProperties.getIdleTimeout() != null) - ? (int) threadProperties.getIdleTimeout().toMillis() : 60000; + private ThreadPool determineThreadPool(ServerProperties.Jetty.Threads properties) { + BlockingQueue queue = determineBlockingQueue(properties.getMaxQueueCapacity()); + int maxThreadCount = (properties.getMax() > 0) ? properties.getMax() : 200; + int minThreadCount = (properties.getMin() > 0) ? properties.getMin() : 8; + int threadIdleTimeout = (properties.getIdleTimeout() != null) ? (int) properties.getIdleTimeout().toMillis() + : 60000; return new QueuedThreadPool(maxThreadCount, minThreadCount, threadIdleTimeout, queue); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java index 1692619038c..fff95b358da 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java @@ -79,12 +79,12 @@ public class TomcatWebServerFactoryCustomizer public void customize(ConfigurableTomcatWebServerFactory factory) { ServerProperties properties = this.serverProperties; ServerProperties.Tomcat tomcatProperties = properties.getTomcat(); - ServerProperties.Tomcat.Threads threadProperties = tomcatProperties.getThreads(); PropertyMapper propertyMapper = PropertyMapper.get(); propertyMapper.from(tomcatProperties::getBasedir).whenNonNull().to(factory::setBaseDirectory); propertyMapper.from(tomcatProperties::getBackgroundProcessorDelay).whenNonNull().as(Duration::getSeconds) .as(Long::intValue).to(factory::setBackgroundProcessorDelay); customizeRemoteIpValve(factory); + ServerProperties.Tomcat.Threads threadProperties = tomcatProperties.getThreads(); propertyMapper.from(threadProperties::getMax).when(this::isPositive) .to((maxThreads) -> customizeMaxThreads(factory, threadProperties.getMax())); propertyMapper.from(threadProperties::getMinSpare).when(this::isPositive) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java index e904f1c8fd3..01e71a3b5e9 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java @@ -88,8 +88,8 @@ public class UndertowWebServerFactoryCustomizer private void mapUndertowProperties(ConfigurableUndertowWebServerFactory factory, FactoryOptions options) { PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull(); Undertow properties = this.serverProperties.getUndertow(); - ServerProperties.Undertow.Threads threadProperties = properties.getThreads(); map.from(properties::getBufferSize).whenNonNull().asInt(DataSize::toBytes).to(factory::setBufferSize); + ServerProperties.Undertow.Threads threadProperties = properties.getThreads(); map.from(threadProperties::getIo).to(factory::setIoThreads); map.from(threadProperties::getWorker).to(factory::setWorkerThreads); map.from(properties::getDirectBuffers).to(factory::setUseDirectBuffers); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java index 4759605c86e..1d891563917 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java @@ -219,9 +219,6 @@ class ServerPropertiesTests { @Test void testCustomizeTomcatMaxThreadsDeprecated() { bind("server.tomcat.maxThreads", "10"); - assertThat(this.properties.getTomcat().getMaxThreads()).isEqualTo(10); - // Verify they are locked on same backing props to avoid further downstream - // deprecated testing assertThat(this.properties.getTomcat().getThreads().getMax()).isEqualTo(10); } @@ -235,9 +232,6 @@ class ServerPropertiesTests { @Test void testCustomizeTomcatMinSpareThreadsDeprecated() { bind("server.tomcat.min-spare-threads", "10"); - assertThat(this.properties.getTomcat().getMinSpareThreads()).isEqualTo(10); - // Verify they are locked on same backing props to avoid further downstream - // deprecated testing assertThat(this.properties.getTomcat().getThreads().getMinSpare()).isEqualTo(10); } @@ -251,9 +245,6 @@ class ServerPropertiesTests { @Test void testCustomizeJettyAcceptorsDeprecated() { bind("server.jetty.acceptors", "10"); - assertThat(this.properties.getJetty().getAcceptors()).isEqualTo(10); - // Verify they are locked on same backing props to avoid further downstream - // deprecated testing assertThat(this.properties.getJetty().getThreads().getAcceptors()).isEqualTo(10); } @@ -268,8 +259,6 @@ class ServerPropertiesTests { void testCustomizeJettySelectorsDeprecated() { bind("server.jetty.selectors", "10"); assertThat(this.properties.getJetty().getSelectors()).isEqualTo(10); - // Verify they are locked on same backing props to avoid further downstream - // deprecated testing assertThat(this.properties.getJetty().getThreads().getSelectors()).isEqualTo(10); } @@ -283,9 +272,6 @@ class ServerPropertiesTests { @Test void testCustomizeJettyMaxThreadsDeprecated() { bind("server.jetty.maxThreads", "10"); - assertThat(this.properties.getJetty().getMaxThreads()).isEqualTo(10); - // Verify they are locked on same backing props to avoid further downstream - // deprecated testing assertThat(this.properties.getJetty().getThreads().getMax()).isEqualTo(10); } @@ -299,9 +285,6 @@ class ServerPropertiesTests { @Test void testCustomizeJettyMinThreadsDeprecated() { bind("server.jetty.minThreads", "10"); - assertThat(this.properties.getJetty().getMinThreads()).isEqualTo(10); - // Verify they are locked on same backing props to avoid further downstream - // deprecated testing assertThat(this.properties.getJetty().getThreads().getMin()).isEqualTo(10); } @@ -315,9 +298,6 @@ class ServerPropertiesTests { @Test void testCustomizeJettyIdleTimeoutDeprecated() { bind("server.jetty.thread-idle-timeout", "10s"); - assertThat(this.properties.getJetty().getThreadIdleTimeout()).isEqualTo(Duration.ofSeconds(10)); - // Verify they are locked on same backing props to avoid further downstream - // deprecated testing assertThat(this.properties.getJetty().getThreads().getIdleTimeout()).hasSeconds(10); } @@ -331,9 +311,6 @@ class ServerPropertiesTests { @Test void testCustomizeJettyMaxQueueCapacityDeprecated() { bind("server.jetty.max-queue-capacity", "5150"); - assertThat(this.properties.getJetty().getMaxQueueCapacity()).isEqualTo(5150); - // Verify they are locked on same backing props to avoid further downstream - // deprecated testing assertThat(this.properties.getJetty().getThreads().getMaxQueueCapacity()).isEqualTo(5150); } @@ -361,9 +338,6 @@ class ServerPropertiesTests { @Test void testCustomizeUndertowIoThreadsDeprecated() { bind("server.undertow.ioThreads", "4"); - assertThat(this.properties.getUndertow().getIoThreads()).isEqualTo(4); - // Verify they are locked on same backing props to avoid further downstream - // deprecated testing assertThat(this.properties.getUndertow().getThreads().getIo()).isEqualTo(4); } @@ -377,9 +351,6 @@ class ServerPropertiesTests { @Test void testCustomizeUndertowWorkerThreadsDeprecated() { bind("server.undertow.workerThreads", "10"); - assertThat(this.properties.getUndertow().getWorkerThreads()).isEqualTo(10); - // Verify they are locked on same backing props to avoid further downstream - // deprecated testing assertThat(this.properties.getUndertow().getThreads().getWorker()).isEqualTo(10); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizerTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizerTests.java index 4a1c989d0e7..d685127d91d 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizerTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizerTests.java @@ -179,7 +179,7 @@ class JettyWebServerFactoryCustomizerTests { @Test void threadPoolIdleTimeoutCanBeCustomized() { - bind("server.jetty.thread-idle-timeout=100s"); + bind("server.jetty.threads.idle-timeout=100s"); JettyWebServer server = customizeAndGetServer(); QueuedThreadPool threadPool = (QueuedThreadPool) server.getServer().getThreadPool(); assertThat(threadPool.getIdleTimeout()).isEqualTo(100000); @@ -187,7 +187,7 @@ class JettyWebServerFactoryCustomizerTests { @Test void threadPoolWithMaxQueueCapacityEqualToZeroCreateSynchronousQueue() { - bind("server.jetty.max-queue-capacity=0"); + bind("server.jetty.threads.max-queue-capacity=0"); JettyWebServer server = customizeAndGetServer(); ThreadPool threadPool = server.getServer().getThreadPool(); BlockingQueue queue = getQueue(threadPool); @@ -197,8 +197,8 @@ class JettyWebServerFactoryCustomizerTests { @Test void threadPoolWithMaxQueueCapacityEqualToZeroCustomizesThreadPool() { - bind("server.jetty.max-queue-capacity=0", "server.jetty.min-threads=100", "server.jetty.max-threads=100", - "server.jetty.thread-idle-timeout=6s"); + bind("server.jetty.threads.max-queue-capacity=0", "server.jetty.min-threads=100", + "server.jetty.max-threads=100", "server.jetty.threads.idle-timeout=6s"); JettyWebServer server = customizeAndGetServer(); QueuedThreadPool threadPool = (QueuedThreadPool) server.getServer().getThreadPool(); assertThat(threadPool.getMinThreads()).isEqualTo(100); @@ -208,7 +208,7 @@ class JettyWebServerFactoryCustomizerTests { @Test void threadPoolWithMaxQueueCapacityPositiveCreateBlockingArrayQueue() { - bind("server.jetty.max-queue-capacity=1234"); + bind("server.jetty.threads.max-queue-capacity=1234"); JettyWebServer server = customizeAndGetServer(); ThreadPool threadPool = server.getServer().getThreadPool(); BlockingQueue queue = getQueue(threadPool); @@ -219,8 +219,8 @@ class JettyWebServerFactoryCustomizerTests { @Test void threadPoolWithMaxQueueCapacityPositiveCustomizesThreadPool() { - bind("server.jetty.max-queue-capacity=1234", "server.jetty.min-threads=10", "server.jetty.max-threads=150", - "server.jetty.thread-idle-timeout=3s"); + bind("server.jetty.threads.max-queue-capacity=1234", "server.jetty.min-threads=10", + "server.jetty.max-threads=150", "server.jetty.threads.idle-timeout=3s"); JettyWebServer server = customizeAndGetServer(); QueuedThreadPool threadPool = (QueuedThreadPool) server.getServer().getThreadPool(); assertThat(threadPool.getMinThreads()).isEqualTo(10); diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-jetty/src/main/resources/application.properties b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-jetty/src/main/resources/application.properties index 4d7dbe1673f..eab83fbdfd2 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-jetty/src/main/resources/application.properties +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-jetty/src/main/resources/application.properties @@ -1,3 +1,3 @@ server.compression.enabled: true server.compression.min-response-size: 1 -server.jetty.acceptors=2 +server.jetty.threads.acceptors=2