Fixes #12171 - QoSHandler does not resume on a virtual thread. #12174
Conversation
| private void execute(Runnable task, boolean useVirtualThreads) | ||
| { | ||
| ThreadPool executor = getServer().getThreadPool(); | ||
| if (useVirtualThreads) | ||
| { | ||
| Executor virtualExecutor = VirtualThreads.getVirtualThreadsExecutor(executor); | ||
| if (virtualExecutor != null) | ||
| { | ||
| virtualExecutor.execute(task); | ||
| return; | ||
| } | ||
| } | ||
| executor.execute(task); | ||
| } |
There was a problem hiding this comment.
I don't think we should do specific QoSHandler resolution for this. See my comments on #12171.... but tl;dr; is that we can't be putting this logic everywhere in the code base that getThreadPool().execute(entry) is used. We need either to not care, or have a general solution.
jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java
Show resolved
Hide resolved
jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java
Show resolved
Hide resolved
|
Sorry @sbordet I just screwed this branch... let's see if I can fix it... |
gregw
force-pushed
the
fix/jetty-12.0.x/qos-handler-virtual-threads
branch
from
48e0e41
to
c0d5adf
Compare
OK I reverted my merge. I didn't see that this is for 12.0.x, and merged with 12.1.x to get the disabled tests from 5bbae27. |
|
@gregw the failures in HEAD were unrelated to this PR. It is not fixed yet by your revert, as it still shows 3000+ files changed. I'll force push my original work. |
sbordet
force-pushed
the
fix/jetty-12.0.x/qos-handler-virtual-threads
branch
from
c0d5adf
to
e58bc60
Compare
Now QoSHandler attempts to resume using a virtual thread if the request was handled with a virtual thread and then suspended. Removed warn() from VirtualThreads.isVirtualThread(), it was too verbose. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet
force-pushed
the
fix/jetty-12.0.x/qos-handler-virtual-threads
branch
from
e58bc60
to
e560b77
Compare
Yeah I know. But I thought this PR would have to pass those tests, so I thought they could be fixed here... but I didn't see the 12.0 vs 12.1 thing... oops
Dang! Sorry I thought I forced pushed it back. |
Now QoSHandler attempts to resume using a virtual thread if the request was handled with a virtual thread and then suspended.
Removed warn() from VirtualThreads.isVirtualThread(), it was too verbose.