JBTM-4023 Implement parallel prepare using Virtual threads (JEP 444) #2445

mmusgrov · 2025-12-10T11:00:41Z

jbosstm/narayana-proposals#3
https://issues.redhat.com/browse/JBTM-4023

JDK17 JDK21 JDK25
CORE

The proposal explains what the PR is adding but briefly it adds support for:

JEP 444: Virtual Threads
JEP 238: Multi-Release Jar Files

and uses these features to re-implement narayana support for asynchronously preparing participants.

I've staged the implementation in three commits:

Isolates the thread operations for async operations needed by BasicAction to just TwoPhaseCommitThreadPool (this is to simplify ongoing support for Multi-Release Jars (ie it contains the only differences needed for java 17 and 21) and adds tests for the functionality being replaced in commit 2.
Modifies BasicAction to use this modified TwoPhaseCommitThreadPool (and removes AsyncPrepare and AsyncCommit since their functionality is replaced by two new methods added to TwoPhaseCommitThreadPool.
Adds the Multi-Release jar support and modifies the tests added in commit 1 to validate that virtual threads are being used.

marcosgopen · 2025-12-11T08:57:59Z

...aCore/arjuna/src/main/java21/com/arjuna/ats/arjuna/coordinator/TwoPhaseCommitThreadPool.java

+ * Copyright The Narayana Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package com.arjuna.ats.arjuna.coordinator;


Is the java21 folder intended for java 21+ use only? @mmusgrov

It's just a name for a source root but should be indicative that it's for use with at least java 21.

If later on we need specific code, for say JEP 505: Structured Concurrency (Fifth Preview), then we'd create a new source root such as src/main/java25/, which I'm sure we can find uses for :-)

I see, thanks for clarifying

ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/BasicAction.java

tomjenkinson · 2025-12-12T13:24:03Z

ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/BasicAction.java

     *
     * @throws Error JBTM-895 tests, byteman limitation
     */
+    protected final void explicitPhase2Commit (BasicAction theAction, boolean reportHeuristics)


IIUC from a different part of the discussion, it would perhaps be appropriate to not have this reportHeuristics argument (and the similar one in explicitPhase2Abort) and just force false on phase2Commit and phase2Abort like we do in the newly deprecated AsyncCommit?

So would we want to disallow a future usage where a feature wants to synchronously call doCommit with an explicit BasicAction which seems like a potentially useful thing to do?

But I guess such a user would be making changes to BasicAction anyway an could add it back in again if the need were ever to arise. I'll remove the argument.

I suppose looking at it from that point then the answer is possibly no? But that being said it's a bit more than synchronously calling doCommit. I looked if we might be able to change our existing use of phase2Commit to this new method but I suppose we shouldn't because it would be adding extra calls to handling ThreadActionData. So I think your question is more like would we want to disallow a future usage where a feature wants to synchronously call doCommit with an explicit BasicAction and where they want to manipulate the ThreadActionData stack at the same time - that feels a more narrow use case.

Although then it's not obvious why reportHeuristics is being forced to be false unless the reader analyses the usages of explicitPhase2Commit and explicitPhase2Abort.

The latest commit) removes the reportHeuristics arguments to explicitPhase2Commit and explicitPhase2Abort

tomjenkinson · 2025-12-12T14:23:51Z

ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/BasicAction.java

     *
     * @throws Error JBTM-895 tests, byteman limitation
     */
+    protected final void explicitPhase2Commit (BasicAction theAction, boolean reportHeuristics)


I suppose looking at it from that point then the answer is possibly no? But that being said it's a bit more than synchronously calling doCommit. I looked if we might be able to change our existing use of phase2Commit to this new method but I suppose we shouldn't because it would be adding extra calls to handling ThreadActionData. So I think your question is more like would we want to disallow a future usage where a feature wants to synchronously call doCommit with an explicit BasicAction and where they want to manipulate the ThreadActionData stack at the same time - that feels a more narrow use case.

mmusgrov · 2025-12-12T16:50:12Z

So I think your question is more like would we want to disallow a future usage where a feature wants to synchronously call doCommit with an explicit BasicAction and where they want to manipulate the ThreadActionData stack at the same time - that feels a more narrow use case.

I called it explicitPhase2Commit because the context (BasicAction) is being passed explicitly (like we do with models like REST-AT and JTS) and on that basis manipulating the ThreadActionData stack is the intent behind the method. But I don't mind removing it since a second version of explicitPhase2Commit could be added if such a use case was ever called for.

Separately, thanks for approving but I may need a config option: while writing a benchmark I have found it necessary to add an option to enable/disable virtual threads in order to get the comparison (similar to what you asked about](https://github.com/jbosstm/narayana-proposals/pull/3/changes#r2580268195) in your review of the proposal but I need it at runtime. However, the change isn't safe while TwoPhaseCommitThreadPool is in use by BasicAction so I won't be able to commit it. But I will need the option to set it at boot time for benchmarking comparisons which would imply a change to the proposal.

mmusgrov · 2025-12-15T08:48:34Z

Separately, thanks for approving but I may need a config option: while writing a benchmark I have found it necessary to add an option to enable/disable virtual threads in order to get the comparison (similar to what you asked about](https://github.com/jbosstm/narayana-proposals/pull/3/changes#r2580268195) in your review of the proposal but I need it at runtime.

I added the config option.

mmusgrov · 2025-12-15T09:58:44Z

I noticed a sporadic failure of AtomicActionAsyncTest.testRegistrationDuringCompletion on my laptop so I added the Hold label while I investigate.

tomjenkinson · 2025-12-17T11:58:10Z

ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/TwoPhaseCommitThreadPool.java

+                arjPropertyManager.getCoordinatorEnvironmentBean().getMaxTwoPhaseCommitThreads());
+    }
+
+    public static boolean isUsingVirtualThreads() {


Are you definitely OK with this and restartExecutor being in the public API? They look like they are used by the test but maybe you anticipate a use case for business logic to need it?

Not really. I will create another package, matching the pools package name, for just the restart test .

The isUsingVirtualThreads call is harmless and potentially useful to third parties though.

And we do need the close and restart functionality for graceful shutdown and restart.

In the last push I made the access modifier for TwoPhaseCommitThreadPool.restartExecutor less permissive and used method handles to call it from the test, I also removed the isUsingVirtualThreads and getExecutorClassName methods from the same class.

And I've squashed the similar commits - the only other one that I would have like to squash was the one about not passing the heuristic reporting flag when running async, SHA b65a00f, it contained too many conflicts. The remaining commits contain mostly self-contained modifications which may be useful for future forensic activities. TwoPhaseCommitThreadPool is changed in more than one commit but it would be quite difficult
to pull just those out into a separate commit.

mmusgrov · 2025-12-17T12:26:55Z

FYI I tested the last commit using:

$ for i in $(seq 1 200); do echo "$i"; mvn test -Dtest=AtomicActionAsyncTest || break; done

which would fail quite early on in the loop without that last commit.

mmusgrov · 2025-12-18T10:21:13Z

ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/TwoPhaseCommitThreadPool.java

+        );
+    }
+
+    static void submitJob(java.util.function.Consumer<Boolean> func, boolean reportHeuristics) {


We no longer use this so I'll remove it.

tomjenkinson

If all you need to do is remove the the method that you have identified is not used then my approval still stands after that is removed

mmusgrov · 2025-12-18T14:50:29Z

I've added the Hold label while I investigate the java 25 failure (the test run isn't choosing the correct version of the TwoPhaseCommitThreadPool class).

… to TwoPhaseCommitThreadPool

…ith method references JBTM-4023 Deprecate AsyncCommit and AsyncPrepare and replace with method references. Pass the action context to resource prepare/abort/commit.

… an method to restart the thread pool

…nc since it is ignored (after review)

mmusgrov · 2026-01-11T18:30:28Z

@tomjenkinson The tests are passing now. The earlier failure on the jdk 25 axis was because the surefire plugin wasn't using the java 25 specific version of TwoPhaseCommitThreadPool. I fixed it using maven profiles by defining a separate maven surefire plugin configuration which is activated when testing with java 25, and similarly for java 21 and java 17. The details are in the last commit (SHA ab8af5e JBTM-4023 update poms for testing multi-release jars).

The tests on as_builder (17) passed but the builder failure happened on the "Save Narayana Maven repository" step:

Error: Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run

tomjenkinson · 2026-01-12T10:02:54Z

Very nice! Thank you!

tomjenkinson · 2026-01-12T10:03:27Z

I re-approved it but please note the JDK 17 failure

mmusgrov · 2026-01-12T10:11:24Z

I re-approved it but please note the JDK 17 failure

The tests on as_builder (17) passed but the builder failure happened on the "Save Narayana Maven repository" step:

Error: Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run

I'd say it's a GH Action issue, do you have any thoughts about that @jmfinelli

tomjenkinson · 2026-01-12T11:22:37Z

The JDK double as_builder thing will hopefully be fixed by #2462

mmusgrov · 2026-01-12T13:29:04Z

The runs passed (https://github.com/jbosstm/narayana/actions/runs/20899006547/job/60095690267?pr=2445) so I'm merging.

marcosgopen reviewed Dec 11, 2025

View reviewed changes

mmusgrov force-pushed the JBTM-4023 branch 3 times, most recently from d0e9e8c to 645cd29 Compare December 11, 2025 16:26

tomjenkinson reviewed Dec 12, 2025

View reviewed changes

ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/BasicAction.java Outdated Show resolved Hide resolved

tomjenkinson reviewed Dec 12, 2025

View reviewed changes

ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/BasicAction.java Show resolved Hide resolved

tomjenkinson reviewed Dec 12, 2025

View reviewed changes

ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/BasicAction.java Show resolved Hide resolved

tomjenkinson reviewed Dec 12, 2025

View reviewed changes

tomjenkinson approved these changes Dec 12, 2025

View reviewed changes

mmusgrov added the Hold label Dec 15, 2025

mmusgrov removed the Hold label Dec 17, 2025

tomjenkinson reviewed Dec 17, 2025

View reviewed changes

mmusgrov force-pushed the JBTM-4023 branch from c11da6e to 06ab149 Compare December 17, 2025 19:02

mmusgrov commented Dec 18, 2025

View reviewed changes

tomjenkinson approved these changes Dec 18, 2025

View reviewed changes

mmusgrov added the Hold label Dec 18, 2025

mmusgrov added 2 commits January 10, 2026 19:40

JBTM-4023 add more tests for async test operations

4d62827

JBTM-4023 Multi-Release jar and add JEP 444, virtual threads, support…

157f9a7

… to TwoPhaseCommitThreadPool

mmusgrov force-pushed the JBTM-4023 branch from 59bc7dc to f97503f Compare January 10, 2026 19:50

mmusgrov added 6 commits January 11, 2026 17:19

JBTM-4023 Deprecate unused AsyncCommit and AsyncPrepare and replace w…

92857fa

…ith method references JBTM-4023 Deprecate AsyncCommit and AsyncPrepare and replace with method references. Pass the action context to resource prepare/abort/commit.

JBTM-4023 add config flag and test to control for virtual threads and…

66fed0b

… an method to restart the thread pool

JBTM-4023 don't pass in the heuristic reporting flag when running asy…

1defe25

…nc since it is ignored (after review)

JBTM-4023 protect the list of synchronizations against concurrent access

b0140c7

JBTM-4023 remove unused method added earlier

e78b31e

JBTM-4023 update poms for testing multi-release jars

ab8af5e

mmusgrov force-pushed the JBTM-4023 branch from f97503f to ab8af5e Compare January 11, 2026 17:22

mmusgrov removed the Hold label Jan 11, 2026

tomjenkinson approved these changes Jan 12, 2026

View reviewed changes

mmusgrov merged commit ccd7406 into jbosstm:main Jan 12, 2026
66 of 75 checks passed

JBTM-4023 Implement parallel prepare using Virtual threads (JEP 444) #2445

JBTM-4023 Implement parallel prepare using Virtual threads (JEP 444) #2445

Uh oh!

Conversation

mmusgrov commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmusgrov Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tomjenkinson Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmusgrov Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmusgrov commented Dec 12, 2025

Uh oh!

mmusgrov commented Dec 15, 2025

Uh oh!

mmusgrov commented Dec 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmusgrov commented Dec 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomjenkinson left a comment

Choose a reason for hiding this comment

Uh oh!

mmusgrov commented Dec 18, 2025

Uh oh!

mmusgrov commented Jan 11, 2026

Uh oh!

tomjenkinson commented Jan 12, 2026

Uh oh!

tomjenkinson commented Jan 12, 2026

Uh oh!

mmusgrov commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tomjenkinson commented Jan 12, 2026

Uh oh!

mmusgrov commented Jan 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

mmusgrov commented Dec 10, 2025 •

edited

Loading

mmusgrov Dec 11, 2025 •

edited

Loading

tomjenkinson Dec 12, 2025 •

edited

Loading

mmusgrov Dec 15, 2025 •

edited

Loading

mmusgrov commented Jan 12, 2026 •

edited

Loading