From 8e5e6d6f5e72a301b4df099c53e2ce4cbab63c0f Mon Sep 17 00:00:00 2001 From: cbono Date: Mon, 30 Dec 2019 21:24:32 -0600 Subject: [PATCH 1/2] Add configuration for embedded Jetty max queue capacity. Closes gh-19493 --- .../autoconfigure/web/ServerProperties.java | 13 +++ ...ttyConstrainedQueuedThreadPoolFactory.java | 102 +++++++++++++++++ ...ReactiveWebServerFactoryConfiguration.java | 14 ++- .../ServletWebServerFactoryConfiguration.java | 14 ++- .../web/ServerPropertiesTests.java | 7 ++ ...onstrainedQueuedThreadPoolFactoryTest.java | 108 ++++++++++++++++++ ...ebServerFactoryAutoConfigurationTests.java | 50 ++++++++ ...ebServerFactoryAutoConfigurationTests.java | 54 +++++++++ .../jetty/JettyThreadPoolFactory.java | 39 +++++++ 9 files changed, 399 insertions(+), 2 deletions(-) create mode 100644 spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyConstrainedQueuedThreadPoolFactory.java create mode 100644 spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/JettyConstrainedQueuedThreadPoolFactoryTest.java create mode 100644 spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyThreadPoolFactory.java 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 b4233590616c..85bfb5fe2f64 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 @@ -1033,6 +1033,11 @@ public static class Jetty { */ private Integer minThreads = 8; + /** + * Maximum capacity of the thread pools backing queue. + */ + private Integer maxQueueCapacity; + /** * Maximum thread idle time. */ @@ -1098,6 +1103,14 @@ public Integer getMaxThreads() { return this.maxThreads; } + public void setMaxQueueCapacity(Integer maxQueueCapacity) { + this.maxQueueCapacity = maxQueueCapacity; + } + + public Integer getMaxQueueCapacity() { + return this.maxQueueCapacity; + } + public void setThreadIdleTimeout(Duration threadIdleTimeout) { this.threadIdleTimeout = threadIdleTimeout; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyConstrainedQueuedThreadPoolFactory.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyConstrainedQueuedThreadPoolFactory.java new file mode 100644 index 000000000000..4ad0d10da6b5 --- /dev/null +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyConstrainedQueuedThreadPoolFactory.java @@ -0,0 +1,102 @@ +/* + * Copyright 2012-2019 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.boot.autoconfigure.web.embedded; + +import java.time.Duration; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.SynchronousQueue; + +import org.eclipse.jetty.util.BlockingArrayQueue; +import org.eclipse.jetty.util.thread.QueuedThreadPool; + +import org.springframework.boot.autoconfigure.web.ServerProperties; +import org.springframework.boot.autoconfigure.web.ServerProperties.Jetty; +import org.springframework.boot.web.embedded.jetty.JettyThreadPoolFactory; + +/** + * A {@link JettyThreadPoolFactory} that creates a thread pool that uses a backing queue + * with a max capacity if the {@link Jetty#maxQueueCapacity} is specified. If the max + * capacity is not specified then the factory will return null thus allowing the standard + * Jetty server thread pool to be used. + * + * @author Chris Bono + * @since 2.3.0 + */ +public class JettyConstrainedQueuedThreadPoolFactory implements JettyThreadPoolFactory { + + private ServerProperties serverProperties; + + public JettyConstrainedQueuedThreadPoolFactory(ServerProperties serverProperties) { + this.serverProperties = serverProperties; + } + + /** + *

+ * Creates a {@link QueuedThreadPool} with the following settings (if + * {@link Jetty#maxQueueCapacity} is specified): + *

+ * @return thread pool as described above or {@code null} if + * {@link Jetty#maxQueueCapacity} is not specified. + */ + @Override + public QueuedThreadPool create() { + + Integer maxQueueCapacity = this.serverProperties.getJetty().getMaxQueueCapacity(); + + // Max depth is not specified - let Jetty server use its defaults in this case + if (maxQueueCapacity == null) { + return null; + } + + BlockingQueue queue; + if (maxQueueCapacity == 0) { + /** + * This queue will cause jetty to reject requests whenever there is no idle + * thread available to handle them. If this queue is used, it is strongly + * recommended to set _minThreads equal to _maxThreads. Jetty's + * QueuedThreadPool class may not behave like a regular java thread pool and + * may not add threads properly when a SynchronousQueue is used. + */ + queue = new SynchronousQueue<>(); + } + else { + /** + * Create a queue of fixed size. This queue will not grow. If a request + * arrives and the queue is empty, the client will see an immediate + * "connection reset" error. + */ + queue = new BlockingArrayQueue<>(maxQueueCapacity); + } + + Integer maxThreadCount = this.serverProperties.getJetty().getMaxThreads(); + Integer minThreadCount = this.serverProperties.getJetty().getMinThreads(); + Duration threadIdleTimeout = this.serverProperties.getJetty().getThreadIdleTimeout(); + + return new QueuedThreadPool((maxThreadCount != null) ? maxThreadCount : 200, + (minThreadCount != null) ? minThreadCount : 8, + (threadIdleTimeout != null) ? (int) threadIdleTimeout.toMillis() : 60000, queue); + } + +} diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryConfiguration.java index e03963cd3d09..0e2371212cae 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryConfiguration.java @@ -24,8 +24,12 @@ import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.autoconfigure.web.ServerProperties; +import org.springframework.boot.autoconfigure.web.embedded.JettyConstrainedQueuedThreadPoolFactory; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.web.embedded.jetty.JettyReactiveWebServerFactory; import org.springframework.boot.web.embedded.jetty.JettyServerCustomizer; +import org.springframework.boot.web.embedded.jetty.JettyThreadPoolFactory; import org.springframework.boot.web.embedded.netty.NettyReactiveWebServerFactory; import org.springframework.boot.web.embedded.netty.NettyRouteProvider; import org.springframework.boot.web.embedded.netty.NettyServerCustomizer; @@ -101,6 +105,7 @@ TomcatReactiveWebServerFactory tomcatReactiveWebServerFactory( @Configuration(proxyBeanMethods = false) @ConditionalOnMissingBean(ReactiveWebServerFactory.class) @ConditionalOnClass({ org.eclipse.jetty.server.Server.class }) + @EnableConfigurationProperties(ServerProperties.class) static class EmbeddedJetty { @Bean @@ -109,10 +114,17 @@ JettyResourceFactory jettyServerResourceFactory() { return new JettyResourceFactory(); } + @Bean + @ConditionalOnMissingBean + JettyThreadPoolFactory jettyThreadPoolFactory(ServerProperties serverProperties) { + return new JettyConstrainedQueuedThreadPoolFactory(serverProperties); + } + @Bean JettyReactiveWebServerFactory jettyReactiveWebServerFactory(JettyResourceFactory resourceFactory, - ObjectProvider serverCustomizers) { + JettyThreadPoolFactory threadPoolFactory, ObjectProvider serverCustomizers) { JettyReactiveWebServerFactory serverFactory = new JettyReactiveWebServerFactory(); + serverFactory.setThreadPool(threadPoolFactory.create()); serverFactory.getServerCustomizers().addAll(serverCustomizers.orderedStream().collect(Collectors.toList())); serverFactory.setResourceFactory(resourceFactory); return serverFactory; diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryConfiguration.java index 03dbe4e92b01..fcf5fe44a46e 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryConfiguration.java @@ -32,8 +32,12 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.SearchStrategy; +import org.springframework.boot.autoconfigure.web.ServerProperties; +import org.springframework.boot.autoconfigure.web.embedded.JettyConstrainedQueuedThreadPoolFactory; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.web.embedded.jetty.JettyServerCustomizer; import org.springframework.boot.web.embedded.jetty.JettyServletWebServerFactory; +import org.springframework.boot.web.embedded.jetty.JettyThreadPoolFactory; import org.springframework.boot.web.embedded.tomcat.TomcatConnectorCustomizer; import org.springframework.boot.web.embedded.tomcat.TomcatContextCustomizer; import org.springframework.boot.web.embedded.tomcat.TomcatProtocolHandlerCustomizer; @@ -90,12 +94,20 @@ TomcatServletWebServerFactory tomcatServletWebServerFactory( @Configuration(proxyBeanMethods = false) @ConditionalOnClass({ Servlet.class, Server.class, Loader.class, WebAppContext.class }) @ConditionalOnMissingBean(value = ServletWebServerFactory.class, search = SearchStrategy.CURRENT) + @EnableConfigurationProperties(ServerProperties.class) static class EmbeddedJetty { @Bean - JettyServletWebServerFactory JettyServletWebServerFactory( + @ConditionalOnMissingBean + JettyThreadPoolFactory jettyThreadPoolFactory(ServerProperties serverProperties) { + return new JettyConstrainedQueuedThreadPoolFactory(serverProperties); + } + + @Bean + JettyServletWebServerFactory JettyServletWebServerFactory(JettyThreadPoolFactory threadPoolFactory, ObjectProvider serverCustomizers) { JettyServletWebServerFactory factory = new JettyServletWebServerFactory(); + factory.setThreadPool(threadPoolFactory.create()); factory.getServerCustomizers().addAll(serverCustomizers.orderedStream().collect(Collectors.toList())); return factory; } 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 96d91dd1720f..fdbcf3089499 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 @@ -76,6 +76,7 @@ * @author Andrew McGhie * @author HaiTao Zhang * @author Rafiullah Hamedy + * @author Chris Bono */ class ServerPropertiesTests { @@ -238,6 +239,12 @@ void testCustomizeJettyIdleTimeout() { assertThat(this.properties.getJetty().getThreadIdleTimeout()).isEqualTo(Duration.ofSeconds(10)); } + @Test + void testCustomizeJettyMaxQueueCapacity() { + bind("server.jetty.max-queue-capacity", "5150"); + assertThat(this.properties.getJetty().getMaxQueueCapacity()).isEqualTo(5150); + } + @Test void testCustomizeUndertowServerOption() { bind("server.undertow.options.server.ALWAYS_SET_KEEP_ALIVE", "true"); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/JettyConstrainedQueuedThreadPoolFactoryTest.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/JettyConstrainedQueuedThreadPoolFactoryTest.java new file mode 100644 index 000000000000..81b45c014757 --- /dev/null +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/JettyConstrainedQueuedThreadPoolFactoryTest.java @@ -0,0 +1,108 @@ +/* + * Copyright 2012-2019 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.boot.autoconfigure.web.embedded; + +import java.lang.reflect.Method; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.SynchronousQueue; + +import org.eclipse.jetty.util.BlockingArrayQueue; +import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.boot.autoconfigure.web.ServerProperties; +import org.springframework.boot.context.properties.bind.Bindable; +import org.springframework.boot.context.properties.bind.Binder; +import org.springframework.boot.context.properties.source.ConfigurationPropertySources; +import org.springframework.mock.env.MockEnvironment; +import org.springframework.test.context.support.TestPropertySourceUtils; +import org.springframework.util.ReflectionUtils; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link JettyConstrainedQueuedThreadPoolFactory}. + * + * @author Chris Bono + */ +class JettyConstrainedQueuedThreadPoolFactoryTest { + + private MockEnvironment environment; + + private ServerProperties serverProperties; + + private JettyConstrainedQueuedThreadPoolFactory factory; + + @BeforeEach + void setup() { + this.environment = new MockEnvironment(); + this.serverProperties = new ServerProperties(); + ConfigurationPropertySources.attach(this.environment); + this.factory = new JettyConstrainedQueuedThreadPoolFactory(this.serverProperties); + } + + @Test + void factoryReturnsNullWhenMaxCapacityNotSpecified() { + bind("server.jetty.max-queue-capacity="); + assertThat(this.factory.create()).isNull(); + } + + @Test + void factoryReturnsSynchronousQueueWhenMaxCapacityIsZero() { + bind("server.jetty.max-queue-capacity=0"); + QueuedThreadPool queuedThreadPool = this.factory.create(); + assertThat(getQueue(queuedThreadPool, SynchronousQueue.class)).isNotNull(); + } + + @Test + void factoryReturnsBlockingArrayQueueWithDefaultsWhenOnlyMaxCapacityIsSet() { + bind("server.jetty.max-queue-capacity=5150"); + QueuedThreadPool queuedThreadPool = this.factory.create(); + assertThat(queuedThreadPool.getMinThreads()).isEqualTo(8); + assertThat(queuedThreadPool.getMaxThreads()).isEqualTo(200); + assertThat(queuedThreadPool.getIdleTimeout()).isEqualTo(60000); + assertThat(getQueue(queuedThreadPool, BlockingArrayQueue.class).getMaxCapacity()).isEqualTo(5150); + } + + @Test + void factoryReturnsBlockingArrayQueueWithCustomValues() { + bind("server.jetty.max-queue-capacity=5150", "server.jetty.min-threads=200", "server.jetty.max-threads=1000", + "server.jetty.thread-idle-timeout=10000"); + QueuedThreadPool queuedThreadPool = this.factory.create(); + assertThat(queuedThreadPool.getMinThreads()).isEqualTo(200); + assertThat(queuedThreadPool.getMaxThreads()).isEqualTo(1000); + assertThat(queuedThreadPool.getIdleTimeout()).isEqualTo(10000); + assertThat(getQueue(queuedThreadPool, BlockingArrayQueue.class).getMaxCapacity()).isEqualTo(5150); + } + + private void bind(String... inlinedProperties) { + TestPropertySourceUtils.addInlinedPropertiesToEnvironment(this.environment, inlinedProperties); + new Binder(ConfigurationPropertySources.get(this.environment)).bind("server", + Bindable.ofInstance(this.serverProperties)); + } + + static > T getQueue(QueuedThreadPool queuedThreadPool, + Class expectedQueueClass) { + Method getQueue = ReflectionUtils.findMethod(QueuedThreadPool.class, "getQueue"); + ReflectionUtils.makeAccessible(getQueue); + Object obj = ReflectionUtils.invokeMethod(getQueue, queuedThreadPool); + assertThat(obj).isInstanceOf(expectedQueueClass); + return expectedQueueClass.cast(obj); + } + +} diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfigurationTests.java index 112c2efb4f80..92f0f1491b46 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfigurationTests.java @@ -22,14 +22,18 @@ import org.apache.catalina.connector.Connector; import org.apache.catalina.startup.Tomcat; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.eclipse.jetty.util.thread.ThreadPool; import org.junit.jupiter.api.Test; import reactor.netty.http.server.HttpServer; import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.autoconfigure.web.embedded.JettyConstrainedQueuedThreadPoolFactory; import org.springframework.boot.test.context.FilteredClassLoader; import org.springframework.boot.test.context.runner.ReactiveWebApplicationContextRunner; import org.springframework.boot.web.embedded.jetty.JettyReactiveWebServerFactory; import org.springframework.boot.web.embedded.jetty.JettyServerCustomizer; +import org.springframework.boot.web.embedded.jetty.JettyThreadPoolFactory; import org.springframework.boot.web.embedded.netty.NettyReactiveWebServerFactory; import org.springframework.boot.web.embedded.netty.NettyServerCustomizer; import org.springframework.boot.web.embedded.tomcat.TomcatConnectorCustomizer; @@ -53,6 +57,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; +import static org.mockito.BDDMockito.when; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -63,6 +68,7 @@ * @author Brian Clozel * @author Raheela Aslam * @author Madhura Bhave + * @author Chris Bono */ class ReactiveWebServerFactoryAutoConfigurationTests { @@ -242,6 +248,36 @@ void jettyServerCustomizerRegisteredAsBeanAndViaFactoryIsOnlyCalledOnce() { }); } + @Test + void jettyDefaultThreadPoolFactoryUsedToCreateThreadPoolOnWebServerFactory() { + new ReactiveWebApplicationContextRunner(AnnotationConfigReactiveWebServerApplicationContext::new) + .withConfiguration(AutoConfigurations.of(ReactiveWebServerFactoryAutoConfiguration.class)) + .withClassLoader(new FilteredClassLoader(Tomcat.class, HttpServer.class)) + .withUserConfiguration(DoubleRegistrationJettyServerCustomizerConfiguration.class, + HttpHandlerConfiguration.class) + .withPropertyValues("server.port=0", "server.jetty.max-queue-capacity:5150").run((context) -> { + assertThat(context.getBean(JettyThreadPoolFactory.class)) + .isInstanceOf(JettyConstrainedQueuedThreadPoolFactory.class); + JettyReactiveWebServerFactory factory = context.getBean(JettyReactiveWebServerFactory.class); + assertThat(factory.getThreadPool()).isNotNull(); + }); + } + + @Test + void jettyCustomThreadPoolFactoryUsedToCreateThreadPoolOnWebServerFactory() { + new ReactiveWebApplicationContextRunner(AnnotationConfigReactiveWebServerApplicationContext::new) + .withConfiguration(AutoConfigurations.of(ReactiveWebServerFactoryAutoConfiguration.class)) + .withClassLoader(new FilteredClassLoader(Tomcat.class, HttpServer.class)) + .withUserConfiguration(JettyCustomThreadPoolFactoryConfiguration.class, HttpHandlerConfiguration.class) + .withPropertyValues("server.port=0").run((context) -> { + JettyReactiveWebServerFactory factory = context.getBean(JettyReactiveWebServerFactory.class); + JettyThreadPoolFactory threadPoolFactory = context.getBean("jettyCustomThreadPoolFactory", + JettyThreadPoolFactory.class); + verify(threadPoolFactory, times(1)).create(); + assertThat(factory.getThreadPool()).isSameAs(JettyCustomThreadPoolFactoryConfiguration.threadPool); + }); + } + @Test void undertowBuilderCustomizerBeanIsAddedToFactory() { new ReactiveWebApplicationContextRunner(AnnotationConfigReactiveWebApplicationContext::new) @@ -468,6 +504,20 @@ WebServerFactoryCustomizer jettyCustomizer() { } + @Configuration(proxyBeanMethods = false) + static class JettyCustomThreadPoolFactoryConfiguration { + + static ThreadPool threadPool = new QueuedThreadPool(); + + @Bean + JettyThreadPoolFactory jettyCustomThreadPoolFactory() { + JettyThreadPoolFactory threadPoolFactory = mock(JettyThreadPoolFactory.class); + when(threadPoolFactory.create()).thenReturn(threadPool); + return threadPoolFactory; + } + + } + @Configuration(proxyBeanMethods = false) static class UndertowBuilderCustomizerConfiguration { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java index cb7306bb0c7c..6a292d0c1609 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java @@ -28,6 +28,8 @@ import org.apache.catalina.connector.Connector; import org.apache.catalina.startup.Tomcat; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.eclipse.jetty.util.thread.ThreadPool; import org.junit.jupiter.api.Test; import reactor.netty.http.server.HttpServer; @@ -35,12 +37,14 @@ import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression; +import org.springframework.boot.autoconfigure.web.embedded.JettyConstrainedQueuedThreadPoolFactory; import org.springframework.boot.test.context.FilteredClassLoader; import org.springframework.boot.test.context.assertj.AssertableWebApplicationContext; import org.springframework.boot.test.context.runner.ContextConsumer; import org.springframework.boot.test.context.runner.WebApplicationContextRunner; import org.springframework.boot.web.embedded.jetty.JettyServerCustomizer; import org.springframework.boot.web.embedded.jetty.JettyServletWebServerFactory; +import org.springframework.boot.web.embedded.jetty.JettyThreadPoolFactory; import org.springframework.boot.web.embedded.tomcat.TomcatConnectorCustomizer; import org.springframework.boot.web.embedded.tomcat.TomcatContextCustomizer; import org.springframework.boot.web.embedded.tomcat.TomcatProtocolHandlerCustomizer; @@ -64,6 +68,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.when; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -76,6 +81,7 @@ * @author Stephane Nicoll * @author Raheela Aslam * @author Madhura Bhave + * @author Chris Bono */ class ServletWebServerFactoryAutoConfigurationTests { @@ -181,6 +187,40 @@ void jettyServerCustomizerRegisteredAsBeanAndViaFactoryIsOnlyCalledOnce() { }); } + @Test + void jettyDefaultThreadPoolFactoryUsedToCreateThreadPoolOnWebServerFactory() { + WebApplicationContextRunner runner = new WebApplicationContextRunner( + AnnotationConfigServletWebServerApplicationContext::new) + .withClassLoader(new FilteredClassLoader(Tomcat.class, HttpServer.class)) + .withConfiguration(AutoConfigurations.of(ServletWebServerFactoryAutoConfiguration.class)) + .withPropertyValues("server.port:0", "server.jetty.max-queue-capacity:5150"); + runner.run((context) -> { + assertThat(context.getBean(JettyThreadPoolFactory.class)) + .isInstanceOf(JettyConstrainedQueuedThreadPoolFactory.class); + JettyServletWebServerFactory factory = context.getBean(JettyServletWebServerFactory.class); + assertThat(factory.getThreadPool()).isNotNull(); // its null by default so + // this verifies the + // factory was used + }); + } + + @Test + void jettyCustomThreadPoolFactoryUsedToCreateThreadPoolOnWebServerFactory() { + WebApplicationContextRunner runner = new WebApplicationContextRunner( + AnnotationConfigServletWebServerApplicationContext::new) + .withClassLoader(new FilteredClassLoader(Tomcat.class, HttpServer.class)) + .withConfiguration(AutoConfigurations.of(ServletWebServerFactoryAutoConfiguration.class)) + .withUserConfiguration(JettyCustomThreadPoolFactoryConfiguration.class) + .withPropertyValues("server.port:0"); + runner.run((context) -> { + JettyServletWebServerFactory factory = context.getBean(JettyServletWebServerFactory.class); + JettyThreadPoolFactory threadPoolFactory = context.getBean("jettyCustomThreadPoolFactory", + JettyThreadPoolFactory.class); + verify(threadPoolFactory, times(1)).create(); + assertThat(factory.getThreadPool()).isSameAs(JettyCustomThreadPoolFactoryConfiguration.threadPool); + }); + } + @Test void undertowDeploymentInfoCustomizerBeanIsAddedToFactory() { WebApplicationContextRunner runner = new WebApplicationContextRunner( @@ -579,6 +619,20 @@ WebServerFactoryCustomizer jettyCustomizer() { } + @Configuration(proxyBeanMethods = false) + static class JettyCustomThreadPoolFactoryConfiguration { + + static ThreadPool threadPool = new QueuedThreadPool(); + + @Bean + JettyThreadPoolFactory jettyCustomThreadPoolFactory() { + JettyThreadPoolFactory threadPoolFactory = mock(JettyThreadPoolFactory.class); + when(threadPoolFactory.create()).thenReturn(threadPool); + return threadPoolFactory; + } + + } + @Configuration(proxyBeanMethods = false) static class UndertowBuilderCustomizerConfiguration { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyThreadPoolFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyThreadPoolFactory.java new file mode 100644 index 000000000000..0bcc9365427b --- /dev/null +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyThreadPoolFactory.java @@ -0,0 +1,39 @@ +/* + * Copyright 2012-2019 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.boot.web.embedded.jetty; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.util.thread.ThreadPool; + +/** + * Factory to create a thread pool for use by a Jetty {@link Server}. + * + * @author Chris Bono + * @since 2.3.0 + * @param type of ThreadPool that the factory returns + */ +@FunctionalInterface +public interface JettyThreadPoolFactory { + + /** + * Creates a thread pool. + * @return a Jetty thread pool or null to use the default Jetty {@link Server} thread + * pool. + */ + T create(); + +} From f17a44230bb684b0d9defa9b2d68474702e2e71e Mon Sep 17 00:00:00 2001 From: cbono Date: Tue, 7 Jan 2020 22:07:09 -0600 Subject: [PATCH 2/2] Add configuration for embedded Undertow max queue capacity.] --- .../autoconfigure/web/ServerProperties.java | 32 +++++ ...vletWebServerFactoryAutoConfiguration.java | 7 ++ ...rtowServletWebServerFactoryCustomizer.java | 51 ++++++++ .../web/ServerPropertiesTests.java | 24 ++++ ...ebServerFactoryAutoConfigurationTests.java | 2 +- ...ervletWebServerFactoryCustomizerTests.java | 118 ++++++++++++++++++ 6 files changed, 233 insertions(+), 1 deletion(-) create mode 100644 spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/UndertowServletWebServerFactoryCustomizerTests.java 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 85bfb5fe2f64..eee0f865e0f8 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 @@ -29,6 +29,7 @@ import java.util.Map; import io.undertow.UndertowOptions; +import io.undertow.server.handlers.RequestLimitingHandler; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.DeprecatedConfigurationProperty; @@ -1307,6 +1308,21 @@ public static class Undertow { */ private Integer workerThreads; + /** + * Number of max concurrent requests. When specified will put a + * {@link RequestLimitingHandler} in place. The default is null. Can only be + * specified in Servlet environments. + */ + private Integer maxRequests; + + /** + * Maximum number of requests to queue up when running with a + * {@link RequestLimitingHandler} and the {@link #maxRequests} is reached. The + * default is -1 (unbounded). Can only be specified in Servlet environments and is + * only used when {@link #maxRequests} is specified. + */ + private Integer maxQueueCapacity; + /** * Whether to allocate buffers outside the Java heap. The default is derived from * the maximum amount of memory that is available to the JVM. @@ -1403,6 +1419,22 @@ public void setWorkerThreads(Integer workerThreads) { this.workerThreads = workerThreads; } + public Integer getMaxRequests() { + return this.maxRequests; + } + + public void setMaxRequests(Integer maxRequests) { + this.maxRequests = maxRequests; + } + + public Integer getMaxQueueCapacity() { + return this.maxQueueCapacity; + } + + public void setMaxQueueCapacity(Integer maxQueueCapacity) { + this.maxQueueCapacity = maxQueueCapacity; + } + public Boolean getDirectBuffers() { return this.directBuffers; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfiguration.java index 2d74032b6e05..7254b5ed81fd 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfiguration.java @@ -78,6 +78,13 @@ public TomcatServletWebServerFactoryCustomizer tomcatServletWebServerFactoryCust return new TomcatServletWebServerFactoryCustomizer(serverProperties); } + @Bean + @ConditionalOnClass(name = "io.undertow.Undertow") + public UndertowServletWebServerFactoryCustomizer undertowServletWebServerFactoryCustomizer( + ServerProperties serverProperties) { + return new UndertowServletWebServerFactoryCustomizer(serverProperties); + } + @Bean @ConditionalOnMissingFilterBean(ForwardedHeaderFilter.class) @ConditionalOnProperty(value = "server.forward-headers-strategy", havingValue = "framework") diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/UndertowServletWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/UndertowServletWebServerFactoryCustomizer.java index 86362316f5e3..dcf66eab9eab 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/UndertowServletWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/UndertowServletWebServerFactoryCustomizer.java @@ -16,6 +16,11 @@ package org.springframework.boot.autoconfigure.web.servlet; +import io.undertow.Handlers; +import io.undertow.server.HandlerWrapper; +import io.undertow.server.HttpHandler; +import io.undertow.server.handlers.RequestLimitingHandler; + import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.web.embedded.undertow.UndertowServletWebServerFactory; import org.springframework.boot.web.server.WebServerFactoryCustomizer; @@ -25,6 +30,7 @@ * Servlet web servers. * * @author Andy Wilkinson + * @author Chris Bono * @since 2.1.7 */ public class UndertowServletWebServerFactoryCustomizer @@ -36,10 +42,55 @@ public UndertowServletWebServerFactoryCustomizer(ServerProperties serverProperti this.serverProperties = serverProperties; } + // TODO why was this not being used before now? + @Override public void customize(UndertowServletWebServerFactory factory) { + factory.addDeploymentInfoCustomizers((deploymentInfo) -> deploymentInfo .setEagerFilterInit(this.serverProperties.getUndertow().isEagerFilterInit())); + + addRateLimiterIfMaxRequestsSet(factory); + } + + /** + * Installs a {@link RequestLimitingHandler} in the initial handler chain if + * {@link ServerProperties.Undertow#maxRequests} is set. If installed, it will use + * {@link ServerProperties.Undertow#maxQueueCapacity} to control the size of the queue + * in the rate limting handler. + * @param factory the factory to add the deployment info customizer on + */ + private void addRateLimiterIfMaxRequestsSet(UndertowServletWebServerFactory factory) { + Integer maxRequests = this.serverProperties.getUndertow().getMaxRequests(); + if (maxRequests == null) { + return; + } + Integer maxQueueCapacity = this.serverProperties.getUndertow().getMaxQueueCapacity(); + factory.addDeploymentInfoCustomizers((deploymentInfo) -> deploymentInfo.addInitialHandlerChainWrapper( + new RequestLimiterHandlerWrapper(maxRequests, (maxQueueCapacity != null) ? maxQueueCapacity : -1))); + } + + /** + * Explicit implementation of HandlerWrapper rather than Lambda to make it possible to + * verify the handler wrappers at runtime during tests. This could be instead written + * as a Lambda but then tests can not see what type of handler wrapper it is. + */ + static class RequestLimiterHandlerWrapper implements HandlerWrapper { + + Integer maxRequests; + + Integer maxQueueCapacity; + + RequestLimiterHandlerWrapper(Integer maxRequests, Integer maxQueueCapacity) { + this.maxRequests = maxRequests; + this.maxQueueCapacity = maxQueueCapacity; + } + + @Override + public HttpHandler wrap(HttpHandler handler) { + return Handlers.requestLimitingHandler(this.maxRequests, this.maxQueueCapacity, handler); + } + } } 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 fdbcf3089499..9af897c0cd8d 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 @@ -259,6 +259,30 @@ void testCustomizeUndertowSocketOption() { "true"); } + @Test + void testCustomizeUndertowWorkerThreads() { + bind("server.undertow.worker-threads", "150"); + assertThat(this.properties.getUndertow().getWorkerThreads()).isEqualTo(150); + } + + @Test + void testCustomizeUndertowIoThreads() { + bind("server.undertow.io-threads", "10"); + assertThat(this.properties.getUndertow().getIoThreads()).isEqualTo(10); + } + + @Test + void testCustomizeUndertowMaxRequests() { + bind("server.undertow.max-requests", "200"); + assertThat(this.properties.getUndertow().getMaxRequests()).isEqualTo(200); + } + + @Test + void testCustomizeUndertowMaxQueueCapacity() { + bind("server.undertow.max-queue-capacity", "100"); + assertThat(this.properties.getUndertow().getMaxQueueCapacity()).isEqualTo(100); + } + @Test void testCustomizeJettyAccessLog() { Map map = new HashMap<>(); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java index 6a292d0c1609..0b411e202de2 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java @@ -231,7 +231,7 @@ void undertowDeploymentInfoCustomizerBeanIsAddedToFactory() { .withPropertyValues("server.port:0"); runner.run((context) -> { UndertowServletWebServerFactory factory = context.getBean(UndertowServletWebServerFactory.class); - assertThat(factory.getDeploymentInfoCustomizers()).hasSize(1); + assertThat(factory.getDeploymentInfoCustomizers()).hasSize(2); }); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/UndertowServletWebServerFactoryCustomizerTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/UndertowServletWebServerFactoryCustomizerTests.java new file mode 100644 index 000000000000..d6577e504069 --- /dev/null +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/UndertowServletWebServerFactoryCustomizerTests.java @@ -0,0 +1,118 @@ +/* + * Copyright 2012-2019 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.boot.autoconfigure.web.servlet; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.boot.autoconfigure.web.ServerProperties; +import org.springframework.boot.autoconfigure.web.servlet.UndertowServletWebServerFactoryCustomizer.RequestLimiterHandlerWrapper; +import org.springframework.boot.context.properties.bind.Bindable; +import org.springframework.boot.context.properties.bind.Binder; +import org.springframework.boot.context.properties.source.ConfigurationPropertySources; +import org.springframework.boot.web.embedded.undertow.UndertowServletWebServer; +import org.springframework.boot.web.embedded.undertow.UndertowServletWebServerFactory; +import org.springframework.mock.env.MockEnvironment; +import org.springframework.test.context.support.TestPropertySourceUtils; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link UndertowServletWebServerFactoryCustomizer}. + * + * @author Chris Bono + */ +class UndertowServletWebServerFactoryCustomizerTests { + + private UndertowServletWebServerFactoryCustomizer customizer; + + private MockEnvironment environment; + + private ServerProperties serverProperties; + + @BeforeEach + void setup() { + this.environment = new MockEnvironment(); + this.serverProperties = new ServerProperties(); + ConfigurationPropertySources.attach(this.environment); + this.customizer = new UndertowServletWebServerFactoryCustomizer(this.serverProperties); + } + + @Test + void eagerFilterInitEnabled() { + bind("server.undertow.eager-filter-init=true"); + UndertowServletWebServer server = customizeAndGetServer(); + assertThat(server.getDeploymentManager().getDeployment().getDeploymentInfo().isEagerFilterInit()).isTrue(); + } + + @Test + void eagerFilterInitDisabled() { + bind("server.undertow.eager-filter-init=false"); + UndertowServletWebServer server = customizeAndGetServer(); + assertThat(server.getDeploymentManager().getDeployment().getDeploymentInfo().isEagerFilterInit()).isFalse(); + } + + @Test + void limiterNotAddedWhenMaxRequestsNotSet() { + bind("server.undertow.max-requests="); + UndertowServletWebServer server = customizeAndGetServer(); + assertThat(server.getDeploymentManager().getDeployment().getDeploymentInfo().getInitialHandlerChainWrappers() + .stream().anyMatch(RequestLimiterHandlerWrapper.class::isInstance)).isFalse(); + } + + @Test + void limiterAddedWhenMaxRequestSetWithDefaultMaxQueueCapacity() { + bind("server.undertow.max-requests=200"); + UndertowServletWebServer server = customizeAndGetServer(); + RequestLimiterHandlerWrapper requestLimiter = server.getDeploymentManager().getDeployment().getDeploymentInfo() + .getInitialHandlerChainWrappers().stream().filter(RequestLimiterHandlerWrapper.class::isInstance) + .findFirst().map(RequestLimiterHandlerWrapper.class::cast).orElse(null); + assertThat(requestLimiter).isNotNull(); + assertThat(requestLimiter.maxRequests).isEqualTo(200); + assertThat(requestLimiter.maxQueueCapacity).isEqualTo(-1); + } + + @Test + void limiterAddedWhenMaxRequestSetWithCustomMaxQueueCapacity() { + bind("server.undertow.max-requests=200", "server.undertow.max-queue-capacity=100"); + UndertowServletWebServer server = customizeAndGetServer(); + RequestLimiterHandlerWrapper requestLimiter = server.getDeploymentManager().getDeployment().getDeploymentInfo() + .getInitialHandlerChainWrappers().stream().filter(RequestLimiterHandlerWrapper.class::isInstance) + .findFirst().map(RequestLimiterHandlerWrapper.class::cast).orElse(null); + assertThat(requestLimiter).isNotNull(); + assertThat(requestLimiter.maxRequests).isEqualTo(200); + assertThat(requestLimiter.maxQueueCapacity).isEqualTo(100); + } + + private void bind(String... inlinedProperties) { + TestPropertySourceUtils.addInlinedPropertiesToEnvironment(this.environment, inlinedProperties); + new Binder(ConfigurationPropertySources.get(this.environment)).bind("server", + Bindable.ofInstance(this.serverProperties)); + } + + private UndertowServletWebServer customizeAndGetServer() { + UndertowServletWebServerFactory factory = customizeAndGetFactory(); + return (UndertowServletWebServer) factory.getWebServer(); + } + + private UndertowServletWebServerFactory customizeAndGetFactory() { + UndertowServletWebServerFactory factory = new UndertowServletWebServerFactory(0); + this.customizer.customize(factory); + return factory; + } + +}