Add support for virtual workflow threads #2297
Conversation
Quinn-With-Two-Ns
force-pushed
the
virtual-thread-support
branch
from
2d2ae19
to
123f261
Compare
Quinn-With-Two-Ns
force-pushed
the
virtual-thread-support
branch
from
36642e3
to
a968ea2
Compare
| // To test the virtual thread support we need to run a seprate test suite with Java 21 | ||
| testing { | ||
| suites { | ||
| virtualThreadTests(JvmTestSuite) { |
There was a problem hiding this comment.
I wonder if, in addition to this, it'd also be valuable in CI to run the whole existing test suite with virtual threads on for all workflow workers. If we have tests on thread interruption or deadlock detection, I think we want to confirm they still work.
There was a problem hiding this comment.
If we have tests on thread interruption or deadlock detection
Virtual threads don't behave any differently then platform threads here FWIW so I don't think it is providing a lot of value to double our CI time.
There was a problem hiding this comment.
We have a whole suite of different workflow logic/expectations, might as well confirm it works. I think we can run as a concurrent CI job to not double our time. I think the peace of mind is worth it. Should still have virtual-thread specific tests like these, but a whole CI run where virtual threads is enabled for everything (workflows and all 4 types of workers) would give some comfort.
Maybe even if it wasn't on in CI, even the ability to run it (e.g. some flag that changes all test workers and whatever supported plumbing needed) and confirmation that we did run it once and it works would be great.
There was a problem hiding this comment.
Yes that what this test suite is for if there is specific tests we can add them to this test site, but the differences between platforms and virtual threads are not enough to justify running all tests twice
There was a problem hiding this comment.
We can revisit this though when we mark virtual threads as non experimental
edit: revisit
There was a problem hiding this comment.
Maybe even if it wasn't on in CI, even the ability to run it (e.g. some flag that changes all test workers and whatever supported plumbing needed) and confirmation that we did run it once and it works would be great.
Yep I can do that
| * called for every thread created. | ||
| */ | ||
| @FunctionalInterface | ||
| public interface ThreadConfigurator { |
There was a problem hiding this comment.
Not sure you need a whole new interface for what could just be a Consumer<Thread> (and maybe then remove justification for a new package), but not a big deal
temporal-sdk/src/main/java/io/temporal/internal/worker/PollerOptions.java
Outdated
Show resolved
Hide resolved
| * <p>Default is false | ||
| */ | ||
| @Experimental | ||
| public Builder setUseVirtualWorkflowThreads(boolean useVirtualWorkflowThreads) { |
There was a problem hiding this comment.
Hrmm, useVirtualThreadsOnWorkflowWorker vs useVirtualWorkflowThreads is a tad confusing, but I can't think of a better way to represent the difference than what you've done
Add support for virtual workflow threads
Closes #1448