paulrutter opened a new pull request, #560:
URL: https://github.com/apache/felix-dev/pull/560

   Fixes [FELIX-6859](https://issues.apache.org/jira/browse/FELIX-6859).
   
   Jetty 12 documents a [preferred virtual threads 
setup](https://jetty.org/docs/jetty/12/programming-guide/arch/threads.html#thread-pool-virtual-threads):
 a `QueuedThreadPool` whose virtual threads executor is a *bounded* 
`VirtualThreadPool`. Felix HTTP could not produce it. The two virtual thread 
options were the extremes — an unbounded number of concurrent tasks, or a 
standalone `VirtualThreadPool` that creates only virtual threads. A deployment 
that wants platform threads for the acceptors and the selectors *and* a bound 
on concurrent request tasks had no way to say so.
   
   ### Change
   
   Adds `org.apache.felix.http.jetty.virtualthreads.maxConcurrentTasks` to the 
jetty12 bundle. When virtual threads are enabled and this is set to a positive 
value, `createServer()` builds a `QueuedThreadPool`, sized by `threadpool.max` 
as usual, whose virtual threads executor is a `VirtualThreadPool` with 
`setMaxConcurrentTasks(<value>)`.
   
   | `virtualthreads.enable` | `threadpool.max` | 
`virtualthreads.maxConcurrentTasks` | Thread pool |
   |--|--|--|--|
   | `false` | unset | - | Jetty's default `QueuedThreadPool`, 200 platform 
threads |
   | `false` | set | - | `QueuedThreadPool` with `<max>` platform threads |
   | `true` | unset | unset | `QueuedThreadPool` + unbounded 
`newVirtualThreadPerTaskExecutor()` |
   | `true` | set | unset | standalone `VirtualThreadPool`, bounded by `<max>` |
   | `true` | any | **set** | **`QueuedThreadPool` + `VirtualThreadPool` 
bounded by `<value>`** (new) |
   
   The existing combinations are untouched, so this is backwards compatible.
   
   ### Two things worth a reviewer's attention
   
   **The `VirtualThreadPool` is added as a bean.** 
`QueuedThreadPool.setVirtualThreadsExecutor(Executor)` only stores the 
reference — it does not add the argument as a managed bean. `VirtualThreadPool` 
is a `ContainerLifeCycle` whose `doStart()` creates both the virtual executor 
and the bounding semaphore, and its `execute()` throws 
`RejectedExecutionException("not running")` while unstarted. So without 
`threadPool.addBean(virtualThreadPool)` every request would be rejected.
   
   **Non-positive values deliberately fall through** to the existing behaviour 
rather than selecting the new branch, because Jetty itself treats 
`maxConcurrentTasks <= 0` as unbounded (`doStart()` only creates the semaphore 
for a positive value). Selecting a "bounded" pool that isn't bounded would be 
worse than falling through — and for the common case of `threadpool.max` unset, 
the fallthrough lands on `QueuedThreadPool` + unbounded executor, which is what 
a value of `0` means anyway.
   
   ### Testing
   
   * `JettyConfigTest` — default of `-1`, plus `Integer` and `String` property 
values. 11/11 pass.
   * `JettyVirtualThreadsBoundedExecutorIT` — a new subclass of 
`JettyVirtualThreadsIT` with the new property set. Serving a request at all 
exercises the life cycle above, since an unstarted pool would reject the task.
   * All three `JettyVirtualThreads*IT` pass on Corretto 21.0.6.
   
   I confirmed the new branch is genuinely taken, rather than the test silently 
passing through an old path, with a temporary probe (removed before committing) 
that reported `pool=QueuedThreadPool vte=VirtualThreadPool maxTasks=50 
qtpMax=100` for a config of `threadpool.max=100` and `maxConcurrentTasks=50`.
   
   ### Notes
   
   * jetty12 only. The Jetty 11 bundle has no `virtualthreads.enable` at all.
   * The README row is added against the current table. 
[#559](https://github.com/apache/felix-dev/pull/559) rewrites this part of the 
README and adds a "Thread pool and virtual threads" section that spells out the 
combinations; whichever of the two lands second needs a trivial rebase, and 
that section gains the fifth row above.
   * Unrelated bug spotted next door, left alone: the `virtualthreads.enable` 
attribute definition passes `-1` to `AttributeDefinitionImpl`, which selects 
the `int` overload, so that boolean flag is declared as an INTEGER attribute 
defaulting to `-1`. Happy to fix it here, in #559, or in its own issue — say 
which.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to