-
Notifications
You must be signed in to change notification settings - Fork 2
Support Virtual Threads in the WorkManager #152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
good we are exploring the abilty of virtual threads - but I noticed the use of a global virtual thread on/off switch. Something we've avoided elsewhere (so far). Is having a global property the right way of doing this? anywhere else we explicitly require use of annotation at the entry point to make it very clear and explicit the execution model is different. so this is not -1'ing the idea; just checking this really is the best way to expose VT in JCA. |
@maxandersen thank you for your feedback. I thought about reading that annotation from the I'll experiment a bit before marking this PR as ready |
daa5e7b
to
fed9687
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces support for virtual threads in the WorkManager by adding a new virtual thread work manager and associated bootstrap context registration. The changes include modifications to the recorder to initialize and register virtual thread work managers, new constants for virtual thread identifiers, and updated build steps and dependencies to enable virtual thread support.
- Updated IronJacamarRecorder to add virtual thread work manager initialization and adjust bootstrap context creation in virtual thread scenarios.
- Added new constants for virtual thread work manager and bootstrap context names.
- Modified pom.xml and build steps in the deployment module and integration tests to include virtual thread support.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
runtime/src/main/java/io/quarkiverse/ironjacamar/runtime/IronJacamarRecorder.java | Added virtual thread work manager initialization and bootstrap context adjustments. |
runtime/src/main/java/io/quarkiverse/ironjacamar/Defaults.java | Introduced virtual thread related constant definitions. |
runtime/pom.xml | Added dependency for Quarkus virtual threads. |
integration-tests/artemis-common/src/main/java/io/quarkiverse/ironjacamar/artemis/ArtemisResourceAdapterFactory.java | Annotated resource adapter factory for virtual thread execution. |
deployment/src/main/java/io/quarkiverse/ironjacamar/deployment/IronJacamarProcessor.java | New build step for virtual thread work manager initialization. |
deployment/pom.xml | Added dependency for virtual threads in the deployment module. |
@@ -178,7 +208,13 @@ public RuntimeValue<Future<String>> initResourceAdapter( | |||
IronJacamarRuntimeConfig config) { | |||
Vertx vertx = vertxSupplier.get(); | |||
IronJacamarContainer ijContainer = beanContainer.beanInstance(IronJacamarContainer.class, Identifier.Literal.of(key)); | |||
CloneableBootstrapContext bootstrapContext = BootstrapContextCoordinator.getInstance().getDefaultBootstrapContext(); | |||
CloneableBootstrapContext bootstrapContext = BootstrapContextCoordinator.getInstance().createBootstrapContext(key); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider setting the bootstrap context name to Defaults.VIRTUAL_THREAD_BOOTSTRAP_CONTEXT_NAME when handling virtual thread work managers to maintain naming consistency and clarify its purpose.
Copilot uses AI. Check for mistakes.
workManager.setSecurityIntegration(securityIntegration); | ||
|
||
// Set the default executor | ||
ExecutorService defaultExecutor = VirtualThreadsRecorder.getCurrent(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment to document the expected behavior, lifecycle, and thread-safety guarantees of the executor returned by VirtualThreadsRecorder.getCurrent() to aid future maintainers.
Copilot uses AI. Check for mistakes.
Uh oh!
There was an error while loading. Please reload this page.