spring-projects / spring-boot

Spring Boot helps you to create Spring-powered, production-grade applications and services with absolute minimum fuss.
https://spring.io/projects/spring-boot
Apache License 2.0
75.18k stars 40.68k forks source link

When virtual threads are enabled, configure Jetty to use them #35703

Closed wilkinsona closed 1 year ago

wilkinsona commented 1 year ago

We can use Jetty 10's org.eclipse.jetty.util.VirtualThreads.Configurable.setVirtualThreadsExecutor(Executor) API to configure it to use virtual threads when they're available. Both ExecutorThreadPool and QueuedThreadPool implement this API. We configure Jetty to use a QueuedThreadPool at the moment

mhalbritter commented 1 year ago

When using Jetty's recommended approach, there will be a pool using platform threads. These platform threads are used for NIO select/accept/destroy. When dispatching an action from AdaptiveExecutionStrategy which calls into user code, a virtual thread is used. This virtual thread does not count against the maximum size of the pool - that means that when using virtual threads there can be (much) more concurrent calls into user code.

We could think about deploying a ConnectionLimit which protects against unbounded calls on the connection level.

mhalbritter commented 1 year ago
new QueuedThreadPool(200, 0, 60000, 0, null, null, Thread.ofVirtual().name("jetty-", 0).factory());

Creates a pool which is only using virtual threads (200 max, 0 min, 60s idle time). But this pools virtual threads (which is not recommended) and may have other side effects on performance or otherwise. I think it's a good idea to stick to the Jetty programming guide.

mhalbritter commented 1 year ago

I've prototyped something here: https://github.com/mhalbritter/spring-boot/tree/mh/virtual-threads.

It adds a @ConditionalOnVirtualThreads, which enables if running on Java 21 and when spring.main.use-virtual-threads is set to true. With that in place, JettyVirtualThreadsWebServerFactoryCustomizer configures the ThreadPool on the Jetty server to use a QueuedThreadPool with a virtual executor.

To protect against DoS, I added the possibility to limit the maximum number of concurrent connections for Jetty, either by setting a property or by calling ConfigurableJettyWebServerFactory.setMaxConnections. The JettyVirtualThreadsWebServerFactoryCustomizer automatically limits the connections when using virtual threads to the max pool size, so that a virtual-thread running Jetty should behave roughly the same as a platform-thread running one.

mhalbritter commented 1 year ago

On a second thought, maybe it's not such a good idea to automatically limit the connections - with connection keep-alive, this could turn out quite bad.

philwebb commented 1 year ago

I'm not sure if we should add useVirtualThreads to SpringApplication or not. It feel like something that's not really tied to that class.

wilkinsona commented 1 year ago

That one was my suggestion. In terms of implementation details, lazy init's slightly different as it's SpringApplication that installs the LazyInitializationBeanFactoryPostProcessor whereas it wouldn't do anything (I don't think) for virtual threads. From a user's perspective, though, I think they're pretty similar as they're both configuration settings that have an effect across the whole app. It was thinking about it from this perspective that made me think it might be a good addition to SpringApplication. I don't feel strongly about it though.

philwebb commented 1 year ago

Looking at the changes in the branch, it seems that most of the updates are in auto-configuration which makes me a bit reluctant to change SpringApplication. With the existing prototype code, the SpringApplication class is adding a property source to make sure the property is available. That feels bit unusual to me and also made me wonder if we should reconsider where it's added.

We don't have too many properties like this. Perhaps CloudPlatform is pretty close but it's also a bit strange because it uses spring.main.cloud-platform as property but it's not actually bound to SpringApplication.

I wonder how folks will want to use virtual threads in practice. It's possible they may want to use them for only certain parts of the stack which will also make a global property too broad. Perhaps we might need fine-grain control and some kind of global default setting as well. E.g:

spring.application.threads.builder=platform
server.jetty.threads.builder=virtual
wilkinsona commented 1 year ago

It's possible they may want to use them for only certain parts of the stack which will also make a global property too broad

My expectation is that this won't be the case. There will be exceptions (like Jetty's acceptor threads) where using virtual threads does not make sense, but I expect those that want to use virtual threads to want them to be used everywhere where it makes sense for them to be used.

sergmain commented 1 year ago

I wonder how folks will want to use virtual threads in practice.

My application is heavily relied on events. I have 50+ events, dozen are bound to transactional context, others is @Async So, with fibers I expect better performance because there won't be potential limitation related to OS threads, i.e. threadPoolTaskScheduler.setPoolSize, AsyncConfigurer

About naming from other side - as a developer I, actually, don't care how you name it. As for me, most important is to have lesser work while migrating, For example - hibernate decided to 'help' with dialects. As a result I spent some time to find solution and then I've posted question on SO - https://stackoverflow.com/questions/76444941/springboot-3-hibernate-6-how-to-configure-version-of-mysql-dialect

ps. my app with events - https://github.com/sergmain/metaheuristic

vladimirfx commented 1 year ago

Sorry for the late comment.

The current Jetty behavior with the virtual thread pool configured is slightly strange - under load, it starts all max (200 by default) of platform threads but uses virtual threads for dispatching. So you have 200 useless platform threads that eat native memory.

When configuring virtual threads for Jetty based on QueuedThreadPool it makes sense to reduce max to available CPU cores.

See my experimental project - https://github.com/cit-consulting/boot-loom-jdbc

mhalbritter commented 1 year ago

Jetty uses platform threads to schedule some of the operations needed to accept a HTTP connection. The dispatch to user code is then done with virtual threads.

I don't think it's a good idea to lower the amount of max platform threads in the pool by default when using virtual threads. Here's the recommended approach of the Jetty team. It uses new QueuedThreadPool() which defaults to 200 max threads.

We trust Jetty to do the right thing here. If you don't want 200 threads doing (in your words) "useless" work, you can use the server properties to reduce the amount of max threads in the pool.

vladimirfx commented 1 year ago

I don't just think but rather profile - only 2-4 of 200 threads do actual work. This threads used to decode HTTP frames, so it used for CPU work only. So it can't be useful in amount > CPU count.

You are right - it is not Spring issue. I've file issue to Jetty.

mhalbritter commented 1 year ago

Please link back to this issue (or put the Jetty issue link in here), so we can track this. Thank you!