`SQLException: Unable to enlist connection to existing transaction` when accessing multiple persistence units in the same transaction since 3.8.2 about quarkus

[mmusgrov]

@yrodiere I think the outcome of the discussions was that we continue to allow a single one-phase aware resource to participate in a JTA transaction, but never more than one, and that the risks will be adequately documented.

[turing85]

@yrodiere I think the outcome of the discussions was that we continue to allow a single one-phase aware resource to participate in a JTA transaction, but never more than one, and that the risks will be adequately documented.

uhm....

This is only half of the truth. As was highlighted multiple times: users rely on the LRCO, e.g. for azure-hosted MS SQL Databases which do not support XA. We are actively cutting off technologies that "worked" before (I know - some transactional properties were violated - but it seems that it was "good enough" or the compromise worth taking instead of "not using it at all").

OK fair enough, but let's make sure the risk is documented.

For me, this sounded like we continue with @zhfeng's proposal to implement a property, especially since @maxandersen already said that the flag should be added.

The change in and of itself seems easy enough. The challenge, however, is to find the correct location to set the property.

[lamemind-ud]

We have been on Quarkus for more than two years with a multi-service project.
We heavily use transactions on multiple PUs.

Preventing by design the use of multiple PUs transactions would be a very costly regression for us. Even though it is not the best practice, we are aware and conscious of the type of risks involved.

[anbonifacio]

We are blocked by this regression too. The only option at the moment is to stay on 3.8.1 for the foreseeable future. But it is totally unacceptable in the long term.

[mmusgrov]

Obviously I'll do my best to diagnose issues but all bets are off if the user runs with the proposed property. Since I'm overruled we'll have to go with Amos's suggestion of printing out a prominent warning if the application has the proposed property enabled.

[turing85]

The question still is: why was this change made? And should we really include it in a patch-level update of an LTS? Was something broken (as in "transactional properties were violated", not in "actual behaviour was different from documented behaviour") before?

[turing85]

@zhfeng we could add that property but it is transactionally unsafe so we I doubt we'd add it.

I am for adding this property. For quarkus 3.9.0 and onwards, the property can default to false, so users have to opt-in in order to use it. For 3.8.x, however, I think it should default to true to not break existing behaviour.

[zhfeng]

Thanks Amos, so the problem was always there but with that change the problem is now explicit.

@mmusgrov Yes, exactly - is there any concern from you to introduce such a config property to allow multiple last resources? Oberviously the default value is false and if users set it with true explictly, a WARN message should be printed out when the application starts up.

[jacopo-cavallarin]

As a side note: I found out while doing some tests with XA that currently the Dev Service for PostgreSQL does not automatically enable XA support by setting max_prepared_transactions on the DB when quarkus is configured to use XA. This causes problems with tests that deal with multiple datasources, and I haven't found a way to set max_prepared_transactions manually with some quarkus configuration property.

I think this issue is related: #36934

[maxandersen]

Clearly I'm overruled here but I'm not prepared to support users who use multiple one-phase aware resources.

but apparently, we did technically did allow it for years and it allows using things like azure hosted SQL databases.

When did we technically allow it, it's only ever been "allowed" in quarkus because of a bug in Agroal?

Yes, so due to that its been technically allowed for 4 years.

That ain't great; but makes it something we'll have to deal with and give some migration/update path.

[mjiderhamn]

This seems caused by #39072.

However setting quarkus.datasource.XYZ.jdbc.transactions=xa on the datasources involved as per the discussion there seems to cause other issues solve the issue.

[ketola]

I think this is an improvement, but this is a pretty big change, meaning that if you have not paid attention to the transaction handling when using multiple data sources you might need several code changes to fix this. And that's why I think it might be a good idea to revert the Agroal upgrade and introduce it in the next minor version upgrade. Because otherwise this might block some users from upgrading to patches containing security fixes.

Also this might be worth explaining in https://quarkus.io/guides/transaction as you might come across this issue the moment you add the second datasource in the application.

In my case I have several data sources in use and they are in separate modules and I consider them being independent and that's why I have tried to keep the transaction handling separated - event though I have noticed earlier that Quarkus has allowed me to do queries in separate datasources using the same transaction.
This change brought up couple of places where my transaction handling was still allowing the same transaction. In these places I was able to fix it either using the annotation:

@Transactional(Transactional.TxType.REQUIRES_NEW)

or if I needed to have separate transactions inside the same method I could use the programmatic approach:

return QuarkusTransaction.requiringNew().call(() -> {
            // fetch stuff
        });

I hope these tips could be helpful to someone.

[yrodiere]

At the very least this needs an entry in the migration guide, both 3.9 and 3.8... looks like we collectively skipped that, sorry @gastaldi .

[turing85]

To be honest, I think the change should be reverted for 3.8.x. This is a breaking change and very unexpected. Maybe someone could point out exactly what changed, and why?

There is still the open question whether we can set the agroal version back to 2.2 in our projects: #39283 (comment)

[mmusgrov]

Even allowing a single one-phase aware resource to join an XA transaction containing two-phase aware resources is transactionally unsafe. Beyond that, allowing two such resources is asking for trouble and we will get many users asking us why the integrity of their data is compromised. I can anticipate that it will end badly and give Quarkus a poor reputation.

[turing85]

So we just close this issue as "won't fix"?

[mmusgrov]

This is only half of the truth. As was highlighted multiple times: users rely on the LRCO, e.g. for azure-hosted MS SQL Databases which do not support XA. We are actively cutting off technologies that "worked" before (I know - some transactional properties were violated - but it seems that it was "good enough" or the compromise worth taking instead of "not using it at all").

OK fair enough, but let's make sure the risk is documented.

[gsmet]

@mmusgrov I understand your point and where you're coming from but not everybody wants XA. And some people are perfectly fine with not having the additional safety it provides. Otherwise, XA would be used a lot more.

I think we need to accept that. And also clarify in the documentation the problems that not using XA can cause.

[mjiderhamn]

@mmusgrov

Are the issues because the datasources you want to use are not XA capable?

The issue was PostgreSQL config (max_prepared_transactions). *blush*

So quarkus.datasource.XYZ.jdbc.transactions=xa resolved the issue for us.

[zhfeng]

Thanks a lot @mmusgrov - The system property works!

I'm sorry it is my bad and I test it only with allowMultipleLastResources before but it seem it's not enought. lastResourceOptimisationInterfaceClassName needs to be set as well.

@turing85 I tried with your reproducer https://github.com/turing85/quarkus-camel-transactions.git
It works with the following command

mvn -DCoreEnvironmentBean.allowMultipleLastResources=true -DJTAEnvironmentBean.lastResourceOptimisationInterfaceClassName=io.agroal.narayana.LocalXAResource  clean quarkus:dev

I can see some WARN messges in the console as well

2024-04-24 06:48:05,726 INFO  [org.apa.cam.imp.eng.AbstractCamelContext] (Quarkus Main Thread) Apache Camel 4.4.1 (camel-1) started in 8ms (build:0ms init:0ms start:8ms)
2024-04-24 06:48:05,728 INFO  [io.quarkus] (Quarkus Main Thread) quarkus-camel-transactions 999-SNAPSHOT on JVM (powered by Quarkus 3.8.3) started in 17.743s. 
2024-04-24 06:48:05,728 INFO  [io.quarkus] (Quarkus Main Thread) Profile dev activated. Live Coding activated.
2024-04-24 06:48:05,728 INFO  [io.quarkus] (Quarkus Main Thread) Installed features: [agroal, camel-core, camel-jta, camel-scheduler, camel-sql, cdi, jdbc-postgresql, narayana-jta, smallrye-context-propagation]
2024-04-24 06:48:06,743 INFO  [scheduler -> db read -> db clean -> db write] (Camel (camel-1) thread #1 - scheduler://read-clean-write) reading...
2024-04-24 06:48:06,784 WARN  [com.arj.ats.arjuna] (Camel (camel-1) thread #1 - scheduler://read-clean-write) ARJUNA012142: You have chosen to enable multiple last resources in the transaction manager. This is transactionally unsafe and should not be relied upon.
2024-04-24 06:48:06,808 INFO  [scheduler -> db read -> db clean -> db write] (Camel (camel-1) thread #1 - scheduler://read-clean-write) done
2024-04-24 06:48:06,808 INFO  [scheduler -> db read -> db clean -> db write] (Camel (camel-1) thread #1 - scheduler://read-clean-write) 2 entries to transfer
2024-04-24 06:48:06,809 INFO  [scheduler -> db read -> db clean -> db write] (Camel (camel-1) thread #1 - scheduler://read-clean-write) deleting...
2024-04-24 06:48:06,810 INFO  [scheduler -> db read -> db clean -> db write] (Camel (camel-1) thread #1 - scheduler://read-clean-write) done
2024-04-24 06:48:06,810 INFO  [scheduler -> db read -> db clean -> db write] (Camel (camel-1) thread #1 - scheduler://read-clean-write) transferring...
2024-04-24 06:48:06,823 WARN  [com.arj.ats.arjuna] (Camel (camel-1) thread #1 - scheduler://read-clean-write) ARJUNA012141: Multiple last resources have been added to the current transaction. This is transactionally unsafe and should not be relied upon. Current resource is LastResourceRecord(XAOnePhaseResource(io.agroal.narayana.LocalXAResource@7fc8c1fb))
2024-04-24 06:48:06,825 INFO  [scheduler -> db read -> db clean -> db write] (Camel (camel-1) thread #1 - scheduler://read-clean-write) done
2024-04-24 06:48:06,827 WARN  [com.arj.ats.arjuna] (Camel (camel-1) thread #1 - scheduler://read-clean-write) ARJUNA012141: Multiple last resources have been added to the current transaction. This is transactionally unsafe and should not be relied upon. Current resource is LastResourceRecord(XAOnePhaseResource(io.agroal.narayana.LocalXAResource@2bce10d2))
2024-04-24 06:48:16,828 INFO  [scheduler -> db read -> db clean -> db write] (Camel (camel-1) thread #1 - scheduler://read-clean-write) reading...
2024-04-24 06:48:16,830 INFO  [scheduler -> db read -> db clean -> db write] (Camel (camel-1) thread #1 - scheduler://read-clean-write) done
2024-04-24 06:48:16,830 INFO  [scheduler -> db read -> db clean -> db write] (Camel (camel-1) thread #1 - scheduler://read-clean-write) No entries to transfer
2024-04-24 06:48:18,165 INFO  [org.apa.cam.imp.eng.AbstractCamelContext] (Shutdown thread) Apache Camel 4.4.1 (camel-1) is shutting down

[zhfeng]

@turing85 Yeah, I think so. We just need some documents on migration and transaction guides.

[zhfeng]

As a side note: I found out while doing some tests with XA that currently the Dev Service for PostgreSQL does not automatically enable XA support by setting max_prepared_transactions on the DB when quarkus is configured to use XA. This causes problems with tests that deal with multiple datasources, and I haven't found a way to set max_prepared_transactions manually with some quarkus configuration property.

@jacopo-cavallarin can you try quarkus.datasource.devservices.command = --max_prepared_transactions=100 in application.properties?

[maxandersen]

If we really need the updated deps we should add the flag.

[mmusgrov]

Can't I just tell you the system property and then then someone updates the docs?

[mmusgrov]

Can't I just tell you the system property and then then someone updates the docs?

If I undestand the comment from @zhfeng correctly, this is not sufficient.

There is no sufficient fix: LRCO is transactionally unsafe.

[anbonifacio]

Can't I just tell you the system property and then then someone updates the docs?

If I undestand the comment from @zhfeng correctly, this is not sufficient.

There is no sufficient fix: LRCO is transactionally unsafe.

Since there is no fix or property that can be exposed on Quarkus side to mitigate the problem, I think that at least the reproducer from @jacopo-cavallarin should be used as a starting point to write a Quarkus blog post about this issue, in order to illustrate how to restructure the code and fix the problematic transactions.

[mmusgrov]

The application should be using persistence units that are backed by XA datasources so I would argue that this is an education issue - the docs should highlight the requirement when adding multiple such units and they should indicate that even using a single non-XA datasource with XA datasources isn't recommended.

[turing85]

The application should be using persistence units that are backed by XA datasources so I would argue that this is an education issue - the docs should highlight the requirement when adding multiple such units and they should indicate that even using a single non-XA datasource with XA datasources isn't recommended.

This is only half of the truth. As was highlighted multiple times: users rely on the LRCO, e.g. for azure-hosted MS SQL Databases which do not support XA. We are actively cutting off technologies that "worked" before (I know - some transactional properties were violated - but it seems that it was "good enough" or the compromise worth taking instead of "not using it at all").

[yrodiere]

@maxandersen @barreiro I think this issue needs one of you.

[mmusgrov]

As someone mentioned above, downgrading to Agroal 2.2 resolves the bug so can we take a step back and identify which Agroal change introduced the new problematic behaviour (presumably something around marking connections with the LastResource marker) and see if there are possible fixes there.

[mmusgrov]

Thanks Amos, so the problem was always there but with that change the problem is now explicit.

@mjiderhamn you said

However setting quarkus.datasource.XYZ.jdbc.transactions=xa on the datasources involved as per the discussion there seems to cause other issues

Are the issues because the datasources you want to use are not XA capable?

[zhfeng]

Sorry @mmusgrov - I don't think system property could work because this property need to be used at build time.

[zhfeng]

@gsmet IIUC, we introduce this propery only to avoid the breaking behavior in LTS version. But it should be removed in the future release, right? since it is not the right way to use XA transaction.

[maxandersen]

Clearly I'm overruled here but I'm not prepared to support users who use multiple one-phase aware resources.

but apparently, we did technically did allow it for years and it allows using things like azure hosted SQL databases.

if we want to shut that off we need something more than just cut it off abruptly.

[maxandersen]

So what I'm hearing is we need:

A) introduce config to be lenient
B) Print Warning - is it possible to only print it when multiple datasource detected or you think we should always print it?
C) Add notes to migration guide
D) document in https://quarkus.io/guides/transaction how to do this right for the cases where this an option and document the case where not possible (i.e. non-XA azure ms SQL usecase) how to deal with it.

for 3.8 we need A,B,C as soon as possible and D can be done less urgent but should happen soon.

@mmusgrov @zhfeng can you run with this and make it happen/open PR?

[zhfeng]

That worked, thanks! I still think that should be done automatically, as dev services generally require little to no configuration to function properly.

Yeah, but I'm not sure if there is a easy way to do that since when setup the postgres container, the devservice know little about whether the datasource is config with XA or not. You can check PostgresqlDevServicesProcessor.java

[yrodiere]

I tried setting the properties at native-image build time by adding

quarkus.native.additional-build-args = \
  -DCoreEnvironmentBean.allowMultipleLastResources=true \
  -DJTAEnvironmentBean.lastResourceOptimisationInterfaceClassName=io.agroal.narayana.LocalXAResource

to application.properties. This did not change anything.

I think these arguments are passed to native-image, not to the JVM. At least that's what this part of the docs implies:

Maybe try -J-D instead?

quarkus.native.additional-build-args = \
  -J-DCoreEnvironmentBean.allowMultipleLastResources=true \
  -J-DJTAEnvironmentBean.lastResourceOptimisationInterfaceClassName=io.agroal.narayana.LocalXAResource

[zhfeng]

Then we can use DJTAEnvironmentBean.lastResourceOptimisationInterfaceClassName=io.agroal.narayana.LocalXAResource to set it at runtime?

With such subsitition like

@TargetClass(com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionImple.class)
final class Target_TransactionImple {

    @Alias
    @RecomputeFieldValue(kind = RecomputeFieldValue.Kind.Reset)
    static Class LAST_RESOURCE_OPTIMISATION_INTERFACE;
}

[maymaewa]

Hello! I have the same situation where I'm trying to perform select operations on two datasources in single transaction.
After updating Quarkus from version 3.8.1 to version 3.8.2 I get the following error:
java.sql.SQLException: Enlisted connection used without active transaction

Error stacktrace:

	at io.agroal.narayana.XAExceptionUtils.xaException(XAExceptionUtils.java:20)
	at io.agroal.narayana.XAExceptionUtils.xaException(XAExceptionUtils.java:8)
	at io.agroal.narayana.LocalXAResource.rollback(LocalXAResource.java:89)
	at com.arjuna.ats.internal.jta.resources.arjunacore.XAResourceRecord.topLevelAbort(XAResourceRecord.java:338)
	at com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionImple.enlistResource(TransactionImple.java:644)
	at com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionImple.enlistResource(TransactionImple.java:398)
	at io.agroal.narayana.NarayanaTransactionIntegration.associate(NarayanaTransactionIntegration.java:120)
	at io.agroal.pool.ConnectionPool.getConnection(ConnectionPool.java:257)
	at io.agroal.pool.DataSource.getConnection(DataSource.java:86)
	at com.company.cloud.core.quarkus.db.datasource.testcontainers.TransactionsTest.lambda$testConnectionIsNotSharedWithinTransaction$3(TransactionsTest.java:146)
	at io.quarkus.narayana.jta.TransactionRunnerImpl.lambda$run$0(TransactionRunnerImpl.java:27)
	at io.quarkus.narayana.jta.QuarkusTransactionImpl.callInOurTx(QuarkusTransactionImpl.java:136)
	at io.quarkus.narayana.jta.QuarkusTransactionImpl.callRequireNew(QuarkusTransactionImpl.java:106)
	at io.quarkus.narayana.jta.QuarkusTransactionImpl.call(QuarkusTransactionImpl.java:29)
	at io.quarkus.narayana.jta.TransactionRunnerImpl.run(TransactionRunnerImpl.java:26)
	at com.company.cloud.core.quarkus.db.datasource.testcontainers.TransactionsTest.testConnectionIsNotSharedWithinTransaction(TransactionsTest.java:139)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:1013)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:827)
	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103)
	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.lambda$invoke$0(InterceptingExecutableInvoker.java:93)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
	at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:156)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:147)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:86)
	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103)
	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.lambda$invoke$0(InterceptingExecutableInvoker.java:93)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:92)
	at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:86)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:218)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:214)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:139)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:69)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:198)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:169)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:93)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:58)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:141)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:57)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:103)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:85)
	at org.junit.platform.launcher.core.DelegatingLauncher.execute(DelegatingLauncher.java:47)
	at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:63)
	at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:57)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater$1.execute(IdeaTestRunner.java:38)
	at com.intellij.rt.execution.junit.TestsRepeater.repeat(TestsRepeater.java:11)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:35)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:235)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)
Caused by: java.sql.SQLException: Enlisted connection used without active transaction
	at io.agroal.pool.ConnectionHandler.verifyEnlistment(ConnectionHandler.java:381)
	at io.agroal.pool.ConnectionHandler.transactionRollback(ConnectionHandler.java:352)
	at io.agroal.narayana.LocalXAResource.rollback(LocalXAResource.java:86)
	... 85 more

This error is reproduced when trying to run the next test:

 @Test
    void testConnectionIsNotSharedWithinTransaction() throws SQLException {

        //Datasources creating and runing flyway scripts ...

        Assertions.assertNotEquals(firstDatasource, secondDatasource);
        firstDatasource.getConnection().createStatement().execute("INSERT into persons(first_name, last_name) VALUES ('first', 'person')");
        secondDatasource.getConnection().createStatement().execute("INSERT into persons(first_name, last_name) VALUES ('second', 'person')");

        QuarkusTransaction.requiringNew().run(() -> {
            try {
                ResultSet firstResultSet = firstDatasource.getConnection().createStatement().executeQuery("SELECT first_name from persons where last_name = 'person'");
                Assertions.assertTrue(firstResultSet.next());
                Assertions.assertEquals("first", firstResultSet.getString(1));
                Assertions.assertFalse(firstResultSet.next());

                ResultSet secondResultSet = secondDatasource.getConnection().createStatement().executeQuery("SELECT first_name from persons where last_name = 'person'");
                Assertions.assertTrue(secondResultSet.next());
                Assertions.assertEquals("second", secondResultSet.getString(1));
                Assertions.assertFalse(secondResultSet.next());
            } catch (SQLException e) {
                Assertions.fail(e);
            }
        });
    }

}

[mjiderhamn]

Downgrading to Agroal 2.2 resolves the issue. So something in Agroal 2.3 broke this.

[luca-bassoricci]

Is it safe to downgrade agroal to 2.2 in pom.xml or better return to 3.8.1 and waiting for a fix?

[geoand]

@barreiro can you please have a look?

cc @gsmet @yrodiere

[maxandersen]

What is the fundemental change in agroal that caused this ?

[turing85]

I may have a reproducer involving camel-quarkus. It shows the exception mentioned in #39283 (comment).

https://github.com/turing85/quarkus-camel-transactions

I am still awaiting confirmation from the camel team that the application does (as per the camel specification) what I think it should do.

Side note: camel-quarkus was not updated from 3.8.1 to 3.8.2; it remained at version 3.8.0.

[turing85]

It seems that agroal/agroal@342ee87 is the commit that caused the issue to appear. However, this alone does not seem to be the root cause. The root cause seems to be agroal/agroal@ced9e8b. It feels like this throws is missing some additional checks.

The example I gave in my previous comment (#39283 (comment)) does not use XA at all. Thus, I do not understand why createXaResource(...) should even be called.

[zhfeng]

Thanks @turing85 for the reproducer!

I'm sorry that I have to say these codes were working before but not right. In your case, if you want to do the db clean in source database and db write in target database in a transation which means do them all or nothing, it definely needs XA. I highly recommend to config all the datasource with …​.jdbc.transactions = xa and also enable quarkus.transaction-manager.enable-recovery=true .Otherwise, it would be risk for the inconsistent data in two datasources.

The root case in agroal is to add LastResource interface in LocalXAResource. IIRC, narayana allows only one LastResource to enlist in a transaction @mmusgrov ? From the transaction perspective, this is right to make sure the non-XA resource could be involved in a XA transaction.

So I think the change in agroal makes sense but unfortuntaly, it breaks the applications which involve two or more non-XA resource in a XA transaction.

@maxandersen I think we definily need a document to describe these changes and impacts.

[turing85]

Thank you @zhfeng for the review. I stroke-through my reproducer.

[barreiro]

@zhfeng comment is spot on. Agroal added the LastResource to LocalXAResource to ensure that a single database is enlisted. Also, there was a change to throw an exception when the enlistment fails because that is a necessary step to manage the connection.

Unfortunately, the current exception does not describe the issue and is not very meaningful. I'll try to improve that.

[maxandersen]

@zhfeng can you clarify that last question ?

If we need to break Lts behaviour we need to have a reason.

Trying to grok what was actually happening in previous versions ? Sounds like commits was potentially happening outside a tx. For some users that might be tolerable (bad; I know) so is there a way for those users to get that old behaviour back or are you saying this is literally broken behaviour in all cases?

[zhfeng]

can you clarify that last question ?

Yeah, in previous version, if it involves multi non-XA datasources in a transaction, Narayana Transaction Manager CAN NOT guarantees that DO THEM ALL OR NOTHING. I think it could violate the Atomic property of Transaction. Also this does not work in the crash recovery secenario. And I understand that in some cases, users don't want such a Strong Consistent transaction behavior.

is there a way for those users to get that old behaviour back?

Yeah, I think there is a propery we can set in Narayana to allow multi LastResources like

arjPropertyManager.getCoreEnvironmentBean().setAllowMultipleLastResources(true);

but it should be set before creating the instance of TransactionManager. So I think we need to introduce a ConfigItem in quarkus-narayana-jta just like quarkus.transaction-manager.allow-multiple-last-resources=true. Do we have any plan to release 3.8.4 and I can try to add it?

@mmusgrov What do you think if we can introudce such a propery in quarkus-narayana-jta?

[mmusgrov]

@zhfeng we could add that property but it is transactionally unsafe so we I doubt we'd add it.

[mjiderhamn]

I can anticipate that it will end badly and give Quarkus a poor reputation.

Worse than a patch version of an LTS containing breaking changes...?

[mmusgrov]

Adding it to the management api/property config implies that Quarkus will support users who fall foul of the consequences of using this behaviour, I would even go so far as saying they'd be better of disabling transactions and winging it instead since that would be better than having naive users thinking that transactions are giving them protection - we already give them the option of disabling recovery, which is a bit odd, and allowing multiple last resources would only compound the problem of allowing transactionally unsafe usage of the extension.

I'd be agreeable to telling them that quarkus supports system properties that they can use to enable this behaviour but not for adding it directly to the extension config, ie it would become a workaround for a known defect.

Finally as I mentioned above LRCO is unsafe but there is a safe alternative, called Commit Markable Resource, but that only allows a single 1-phase aware resource to join an XA transaction.

[turing85]

@maxandersen , @mmusgrov So... how do we proceed now? What is the process?

[turing85]

Can't I just tell you the system property and then then someone updates the docs?

If I undestand the comment from @zhfeng correctly, this is not sufficient.

[turing85]

No opposition to this 🙂

[giovannimartelli]

following

[taneraruk]

is there any progress/decision about this issue?

[josearaujo]

any update?

[mmusgrov]

@yrodiere I think the outcome of the discussions was that we continue to allow a single one-phase aware resource to participate in a JTA transaction, but never more than one, and that the risks will be adequately documented.

uhm....

This is only half of the truth. As was highlighted multiple times: users rely on the LRCO, e.g. for azure-hosted MS SQL Databases which do not support XA. We are actively cutting off technologies that "worked" before (I know - some transactional properties were violated - but it seems that it was "good enough" or the compromise worth taking instead of "not using it at all").

OK fair enough, but let's make sure the risk is documented.

For me, this sounded like we continue with @zhfeng's proposal to implement a property. The change in and of itself seems easy enough. The challenge, however, is to find the correct location to set the property.

I was responding to your comment

This is only half of the truth. As was highlighted multiple times: users rely on the LRCO, ...

where I had been arguing that even a single LRCO was dangerous and you said that " users rely on the LRCO, e.g. for azure-hosted MS SQL Databases ..." so I was simply acknowledging that your point about there already being precedents for LRCO as being valid. On the other hand I doubt anyone would want to commit to supporting a system that allowed the enlistment of a second LRCO resource, there is no precedent in our industry of allowing two LRCO resources.

[mmusgrov]

Earlier I said,

I'd be agreeable to telling them that quarkus supports system properties that they can use to enable this behaviour but not for adding it directly to the extension config, ie it would become a workaround for a known defect.

This is the workaround for people that insist on enlisting more than one LRCO resource.

[lorenzopompili]

Following, we really need this bug fixed for our production too!

[turing85]

Following, we really need this bug fixed for our production too!

@lorenzopompili Can you please clarify what you mean? The root cause of this issue was a bugfix, which is now live. Do you need this fix? Or do you think that this "issue" is a bug (which is is not - it is a change in behaviour that was introduced by aforementioned bug fix)?

[lorenzopompili]

Actually we have some projects under 3.8.3, is this fixed in 3.8.4 and 3.9.4 so we can mobilitate to migrate our products?

[zhfeng]

@mmusgrov IIRC, the commit is here which adds LastResource marker on LocalXAResource

[mmusgrov]

Thanks Amos, so the problem was always there but with that change the problem is now explicit.

@mmusgrov Yes, exactly - is there any concern from you to introduce such a config property to allow multiple last resources? Oberviously the default value is false and if users set it with true explictly, a WARN message should be printed out when the application starts up.

I expressed my concern in most of my messages above: my current stance is to ask users to use the system property workaround and to add documentation telling users why only one LRCO resource is allowed and why even a single LRCO should be use with great caution.

[mmusgrov]

I mentioned the workaround (of using a system property) in two messages but no-one has responded to that.

[maxandersen]

@mmusgrov maybe I missed it but what system property is that? I didn't spot it in this discussion?

[mmusgrov]

And adding a config property to allow multiple LRCO resources is not really an option because it would imply that it is supported - ie the messaging would be that "we support you in writing transactionally unsafe code".

[mmusgrov]

@maxandersen

Yeah, I think there is a propery we can set in Narayana to allow multi LastResources like

arjPropertyManager.getCoreEnvironmentBean().setAllowMultipleLastResources(true);

but Amos says that system properties won't work because it's needed at build time.

[mmusgrov]

@mmusgrov I understand your point and where you're coming from but not everybody wants XA. And some people are perfectly fine with not having the additional safety it provides. Otherwise, XA would be used a lot more.

I think we need to accept that. And also clarify in the documentation the problems that not using XA can cause.

People who use multiple databases use XA otherwise there's not much point in using a transaction.

Clearly I'm overruled here but I'm not prepared to support users who use multiple one-phase aware resources.

[anbonifacio]

@mmusgrov I understand your point of view, however there are situations where one or more of the multiple databases in use does not support XA...
In our case we cannot change the underling database so XA may never be a viable option for us. 😞

[maxandersen]

Obviously I'll do my best to diagnose issues but all bets are off if the user runs with the proposed property. Since I'm overruled we'll have to go with Amos's suggestion of printing out a prominent warning if the application has the proposed property enabled.

Yes, I unfortunately don't see other option as the current prohibited usecase is not otherwise implementable afaics.

[mmusgrov]

@gsmet IIUC, we introduce this propery only to avoid the breaking behavior in LTS version. But it should be removed in the future release, right? since it is not the right way to use XA transaction.

It'll never get removed Amos because the use cases won't allow it - people are asking for it in order to avoid using XA capable resources and yet still be able to use multiple databases.

[mmusgrov]

Obviously I'll do my best to diagnose issues but all bets are off if the user runs with the proposed property. Since I'm overruled we'll have to go with Amos's suggestion of printing out a prominent warning if the application has the proposed property enabled.

Yes, I unfortunately don't see other option as the current prohibited usecase is not otherwise implementable afaics.

Do you mean the use case of having two non-XA aware resources, if so then yes I agree it isn't implementable.

[mmusgrov]

Sorry @mmusgrov - I don't think system property could work because this property need to be used at build time.

Why can't it be a build time decision, it would be the vendor of the application that is making the decision and who'd be accepting the risks on behalf of the users of that application.

[zhfeng]

Why can't it be a build time decision, it would be the vendor of the application that is making the decision and who'd be accepting the risks on behalf of the users of that application.

Hmm, I only checked these codes in NarayanaJtaProcessor.java before

        //we want to force Arjuna to init at static init time
        Properties defaultProperties = PropertiesFactory.getDefaultProperties();
        //we don't want to store the system properties here
        //we re-apply them at runtime
        for (Object i : System.getProperties().keySet()) {
            defaultProperties.remove(i);
        }

But it looks like now we init it at runtime. I will try a system property to see if it works. @mmusgrov do you know what the system property should be used here?

[mmusgrov]

I'll help with the documentation (C and D). @zhfeng if you are okay with A and B then we can sequence the PRs appropriately.

document the case where not possible (i.e. non-XA azure ms SQL usecase) how to deal with it.

@maxandersen when you say "how to deal with it" do you mean how to set this new config property since there isn't a way to deal with it?

[mmusgrov]

But it looks like now we init it at runtime. I will try a system property to see if it works. @mmusgrov do you know what the system property should be used here?

@zhfeng to enable it you would need to pass -DCoreEnvironmentBean.allowMultipleLastResources to the JVM and it's a boolean property.

If that works then I'd be a lot more comfortable documenting this "feature" in the migration guide and transactions manual.

[mmusgrov]

Clearly I'm overruled here but I'm not prepared to support users who use multiple one-phase aware resources.

but apparently, we did technically did allow it for years and it allows using things like azure hosted SQL databases.

When did we technically allow it, it's only ever been "allowed" in quarkus because of a bug in Agroal?

[turing85]

It works with the following command

mvn -DCoreEnvironmentBean.allowMultipleLastResources=true -DJTAEnvironmentBean.lastResourceOptimisationInterfaceClassName=io.agroal.narayana.LocalXAResource  clean quarkus:dev

@zhfeng does this mean we do not need any changes on the quarkus-side?

[turing85]

Have we tested whether this approach also works in a native image?

[zhfeng]

Hmm, not yet, I think it should work. Is there anyway to run your reporducer in native mode?

[turing85]

mh... yes. Give me a second. We just need a (containerized) database.

[turing85]

I have updated the reproducer, it now has two dockerized databases.

We can start the databases by executing

cd local-deployment
docker-compose up -d
cd ..

When we then build

./mvnw clean package

and run the application

java \
  -DCoreEnvironmentBean.allowMultipleLastResources=true \
  -DJTAEnvironmentBean.lastResourceOptimisationInterfaceClassName=io.agroal.narayana.LocalXAResource \
  -jar target/quarkus-app/quarkus-run.jar

everything works as expected.

However, when we reset the databse:

cd local-deployment
docker-compose down
docker-compose up -d
cd ..

build natively:

./mvnw --define native clean package

and run the native image:

target/quarkus-camel-transactions-999-SNAPSHOT-runner \
  -DCoreEnvironmentBean.allowMultipleLastResources=true \
  -DJTAEnvironmentBean.lastResourceOptimisationInterfaceClassName=io.agroal.narayana.LocalXAResource

It seems that the properties are ignored:

2024-04-24 09:31:21,109 WARN  [org.apa.cam.com.sch.SchedulerConsumer] (Camel (camel-1) thread #1 - scheduler://read-clean-write) Error processing exchange. Exchange[4E12785FE5C86F7-0000000000000000]. Caused by: [org.springframework.jdbc.CannotGetJdbcConnectionException - Failed to obtain JDBC Connection]: org.springframework.jdbc.CannotGetJdbcConnectionException: Failed to obtain JDBC Connection
	at org.springframework.jdbc.datasource.DataSourceUtils.getConnection(DataSourceUtils.java:84)
	at org.springframework.jdbc.core.JdbcTemplate.execute(JdbcTemplate.java:653)
	at org.springframework.jdbc.core.JdbcTemplate.execute(JdbcTemplate.java:695)
	at org.apache.camel.component.sql.SqlProducer.processInternal(SqlProducer.java:150)
	at org.apache.camel.component.sql.SqlProducer.process(SqlProducer.java:137)
	at org.apache.camel.support.AsyncProcessorConverterHelper$ProcessorToAsyncProcessorBridge.process(AsyncProcessorConverterHelper.java:65)
	at org.apache.camel.processor.SendProcessor.process(SendProcessor.java:210)
	at org.apache.camel.processor.errorhandler.RedeliveryErrorHandler$SimpleTask.handleFirst(RedeliveryErrorHandler.java:462)
	at org.apache.camel.processor.errorhandler.RedeliveryErrorHandler$SimpleTask.run(RedeliveryErrorHandler.java:438)
	at org.apache.camel.impl.engine.DefaultReactiveExecutor$Worker.executeFromQueue(DefaultReactiveExecutor.java:240)
	at org.apache.camel.impl.engine.DefaultReactiveExecutor.executeFromQueue(DefaultReactiveExecutor.java:77)
	at org.apache.camel.impl.engine.DefaultAsyncProcessorAwaitManager.await(DefaultAsyncProcessorAwaitManager.java:95)
	at org.apache.camel.impl.engine.DefaultAsyncProcessorAwaitManager.process(DefaultAsyncProcessorAwaitManager.java:84)
	at org.apache.camel.processor.errorhandler.RedeliveryErrorHandler.process(RedeliveryErrorHandler.java:200)
	at org.apache.camel.impl.engine.CamelInternalProcessor.processTransacted(CamelInternalProcessor.java:397)
	at org.apache.camel.impl.engine.CamelInternalProcessor.process(CamelInternalProcessor.java:327)
	at org.apache.camel.processor.Pipeline$PipelineTask.run(Pipeline.java:102)
	at org.apache.camel.impl.engine.DefaultReactiveExecutor$Worker.executeFromQueue(DefaultReactiveExecutor.java:240)
	at org.apache.camel.impl.engine.DefaultReactiveExecutor.executeFromQueue(DefaultReactiveExecutor.java:77)
	at org.apache.camel.impl.engine.DefaultAsyncProcessorAwaitManager.await(DefaultAsyncProcessorAwaitManager.java:95)
	at org.apache.camel.impl.engine.DefaultAsyncProcessorAwaitManager.process(DefaultAsyncProcessorAwaitManager.java:84)
	at org.apache.camel.support.AsyncProcessorSupport.process(AsyncProcessorSupport.java:32)
	at org.apache.camel.impl.engine.CamelInternalProcessor.processTransacted(CamelInternalProcessor.java:397)
	at org.apache.camel.impl.engine.CamelInternalProcessor.process(CamelInternalProcessor.java:327)
	at org.apache.camel.processor.Pipeline$PipelineTask.run(Pipeline.java:102)
	at org.apache.camel.impl.engine.DefaultReactiveExecutor$Worker.executeFromQueue(DefaultReactiveExecutor.java:240)
	at org.apache.camel.impl.engine.DefaultReactiveExecutor.executeFromQueue(DefaultReactiveExecutor.java:77)
	at org.apache.camel.impl.engine.DefaultAsyncProcessorAwaitManager.await(DefaultAsyncProcessorAwaitManager.java:95)
	at org.apache.camel.impl.engine.DefaultAsyncProcessorAwaitManager.process(DefaultAsyncProcessorAwaitManager.java:84)
	at org.apache.camel.support.AsyncProcessorSupport.process(AsyncProcessorSupport.java:32)
	at org.apache.camel.jta.TransactionErrorHandler.processByErrorHandler(TransactionErrorHandler.java:234)
	at org.apache.camel.jta.TransactionErrorHandler$1.run(TransactionErrorHandler.java:197)
	at org.apache.camel.quarkus.component.jta.TransactionalJtaTransactionPolicy.runWithTransaction(TransactionalJtaTransactionPolicy.java:47)
	at org.apache.camel.quarkus.component.jta.RequiredJtaTransactionPolicy.run(RequiredJtaTransactionPolicy.java:26)
	at org.apache.camel.jta.TransactionErrorHandler.doInTransactionTemplate(TransactionErrorHandler.java:187)
	at org.apache.camel.jta.TransactionErrorHandler.processInTransaction(TransactionErrorHandler.java:138)
	at org.apache.camel.jta.TransactionErrorHandler.process(TransactionErrorHandler.java:102)
	at org.apache.camel.jta.TransactionErrorHandler.process(TransactionErrorHandler.java:111)
	at org.apache.camel.processor.errorhandler.RedeliveryErrorHandler$SimpleTask.handleFirst(RedeliveryErrorHandler.java:462)
	at org.apache.camel.processor.errorhandler.RedeliveryErrorHandler$SimpleTask.run(RedeliveryErrorHandler.java:438)
	at org.apache.camel.impl.engine.DefaultReactiveExecutor$Worker.doRun(DefaultReactiveExecutor.java:199)
	at org.apache.camel.impl.engine.DefaultReactiveExecutor$Worker.executeReactiveWork(DefaultReactiveExecutor.java:189)
	at org.apache.camel.impl.engine.DefaultReactiveExecutor$Worker.tryExecuteReactiveWork(DefaultReactiveExecutor.java:166)
	at org.apache.camel.impl.engine.DefaultReactiveExecutor$Worker.schedule(DefaultReactiveExecutor.java:148)
	at org.apache.camel.impl.engine.DefaultReactiveExecutor.scheduleMain(DefaultReactiveExecutor.java:59)
	at org.apache.camel.processor.Pipeline.process(Pipeline.java:163)
	at org.apache.camel.impl.engine.CamelInternalProcessor.processNonTransacted(CamelInternalProcessor.java:354)
	at org.apache.camel.impl.engine.CamelInternalProcessor.process(CamelInternalProcessor.java:330)
	at org.apache.camel.component.scheduler.SchedulerConsumer.sendTimerExchange(SchedulerConsumer.java:70)
	at org.apache.camel.component.scheduler.SchedulerConsumer.poll(SchedulerConsumer.java:50)
	at org.apache.camel.support.ScheduledPollConsumer.doRun(ScheduledPollConsumer.java:204)
	at org.apache.camel.support.ScheduledPollConsumer.run(ScheduledPollConsumer.java:118)
	at [email protected]/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572)
	at [email protected]/java.util.concurrent.FutureTask.runAndReset(FutureTask.java:358)
	at [email protected]/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:305)
	at [email protected]/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
	at [email protected]/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
	at [email protected]/java.lang.Thread.runWith(Thread.java:1596)
	at [email protected]/java.lang.Thread.run(Thread.java:1583)
	at org.graalvm.nativeimage.builder/com.oracle.svm.core.thread.PlatformThreads.threadStartRoutine(PlatformThreads.java:833)
	at org.graalvm.nativeimage.builder/com.oracle.svm.core.posix.thread.PosixPlatformThreads.pthreadStartRoutine(PosixPlatformThreads.java:211)
Caused by: java.sql.SQLException: Exception in association of connection to existing transaction
	at io.agroal.narayana.NarayanaTransactionIntegration.associate(NarayanaTransactionIntegration.java:130)
	at io.agroal.pool.ConnectionPool.getConnection(ConnectionPool.java:257)
	at io.agroal.pool.DataSource.getConnection(DataSource.java:86)
	at io.agroal.api.AgroalDataSource_syYGL47zwCt2LQzSIsPp-v9wXhg_Synthetic_ClientProxy.getConnection(Unknown Source)
	at org.springframework.jdbc.datasource.DataSourceUtils.fetchConnection(DataSourceUtils.java:160)
	at org.springframework.jdbc.datasource.DataSourceUtils.doGetConnection(DataSourceUtils.java:118)
	at org.springframework.jdbc.datasource.DataSourceUtils.getConnection(DataSourceUtils.java:81)
	... 60 more
Caused by: java.sql.SQLException: Unable to enlist connection to existing transaction
	at io.agroal.narayana.NarayanaTransactionIntegration.associate(NarayanaTransactionIntegration.java:121)
	... 66 more

[turing85]

I tried setting the properties at native-image build time by adding

quarkus.native.additional-build-args = \
  -DCoreEnvironmentBean.allowMultipleLastResources=true \
  -DJTAEnvironmentBean.lastResourceOptimisationInterfaceClassName=io.agroal.narayana.LocalXAResource

to application.properties. This did not change anything.

[zhfeng]

Thanks a lot @turing85 - I think we need a expert from Quarkus to check why these system properties do not work in native.

[jacopo-cavallarin]

As a side note: I found out while doing some tests with XA that currently the Dev Service for PostgreSQL does not automatically enable XA support by setting max_prepared_transactions on the DB when quarkus is configured to use XA. This causes problems with tests that deal with multiple datasources, and I haven't found a way to set max_prepared_transactions manually with some quarkus configuration property.

@jacopo-cavallarin can you try quarkus.datasource.devservices.command = --max_prepared_transactions=100 in application.properties?

That worked, thanks! I still think that should be done automatically, as dev services generally require little to no configuration to function properly.

[mmusgrov]

Thanks a lot @turing85 - I think we need a expert from Quarkus to check why these system properties do not work in native.

The guide implies it should work in native mode. @gsmet or @yrodiere can you suggest who might know about using system properties in native mode.

[turing85]

Maybe try -J-D instead?

quarkus.native.additional-build-args = \
  -J-DCoreEnvironmentBean.allowMultipleLastResources=true \
  -J-DJTAEnvironmentBean.lastResourceOptimisationInterfaceClassName=io.agroal.narayana.LocalXAResource

Same behaviour.

[yrodiere]

Well I just checked, that's definitely how you pass system properties during static init:

quarkus/core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/NativeImageBuildStep.java

Lines 825 to 827 in c84ac1d

	if (nativeConfig.headless()) {
	nativeImageArgs.add("-J-Djava.awt.headless=true");
	}

So the system property is likely passed to the JVM, but somehow ignored by Narayana. Logs could help confirm this easily, e.g. by dumping System.getProperties to stdout through custom code in your app.

As to why Narayana would ignore it... Beats me. Maybe it's taken into account at runtime only, and there's some problem there? Debugging the reproducer in JVM mode may offer you some insights, e.g. you would at least know when the properties are taken into account (static init or runtime).

[zhfeng]

Thanks @yrodiere - I take a deep look.

System properties pass to the application at runtime and they are all with the right value. The problem is in narayana which use these property as a static variable. like in ArjunaCore/arjuna/classes/com/arjuna/ats/internal/arjuna/abstractrecords/LastResourceRecord.java

private static final boolean ALLOW_MULTIPLE_LAST_RESOURCES = arjPropertyManager
            .getCoreEnvironmentBean().isAllowMultipleLastResources();

in ArjunaJTA/jta/classes/com/arjuna/ats/internal/jta/transaction/arjunacore/TransactionImple.java

private static final Class LAST_RESOURCE_OPTIMISATION_INTERFACE = jtaPropertyManager.getJTAEnvironmentBean()
            .getLastResourceOptimisationInterface();

So it need to recompute at runtime.

[mmusgrov]

The quarkus doc for native suggests initialising these at runtime so will the following get around the problem:

--initialize-at-run-time=com.arjuna.ats.internal.arjuna.abstractrecords.LastResourceRecord,com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionImple

[zhfeng]

Hmm, I think we need to move setDefaultProperties to a staic_init step like

    @BuildStep
    @Record(STATIC_INIT)
    public void setProperties(NarayanaJtaRecorder recorder) {
        //we want to force Arjuna to init at static init time
        Properties defaultProperties = PropertiesFactory.getDefaultProperties();
        //we don't want to store the system properties here
        //we re-apply them at runtime
        for (Object i : System.getProperties().keySet()) {
            defaultProperties.remove(i);
        }
        recorder.setDefaultProperties(defaultProperties);
    }

[zhfeng]

I will open a PR for these changes on narayana-jta and @turing85 can you try this fix?
And also the quarkus.native.additional-build-args should be sepearted by ,

quarkus.native.additional-build-args = \
  -J-DCoreEnvironmentBean.allowMultipleLastResources=true, \
  -J-DJTAEnvironmentBean.lastResourceOptimisationInterfaceClassName=io.agroal.narayana.LocalXAResource

[zhfeng]

I'm running your reproducer in native mode locally and it works as expected!

[zhfeng]

@mmusgrov I tried --initialize-at-run-time and the native image build is good but it throws the Exception at runtime.

2024-04-24 22:46:22,547 WARN  [com.arj.ats.common] (Camel (camel-1) thread #1 - scheduler://read-clean-write) ARJUNA048004: attempt to load com.arjuna.ats.jta.resources.LastResourceCommitOptimisation threw ClassNotFound. Wrong classloader?: java.lang.ClassNotFoundException: com.arjuna.ats.jta.resources.LastResourceCommitOptimisation
	at org.graalvm.nativeimage.builder/com.oracle.svm.core.hub.ClassForNameSupport.forName(ClassForNameSupport.java:122)
	at org.graalvm.nativeimage.builder/com.oracle.svm.core.hub.ClassForNameSupport.forName(ClassForNameSupport.java:86)
	at [email protected]/java.lang.Class.forName(DynamicHub.java:1356)
	at [email protected]/java.lang.Class.forName(DynamicHub.java:1319)
	at [email protected]/java.lang.Class.forName(DynamicHub.java:1312)
	at com.arjuna.common.internal.util.ClassloadingUtility.loadClass(ClassloadingUtility.java:42)
	at com.arjuna.ats.jta.common.JTAEnvironmentBean.getLastResourceOptimisationInterface(JTAEnvironmentBean.java:863)
	at com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionImple.<clinit>(TransactionImple.java:1707)
	at com.arjuna.ats.internal.jta.transaction.arjunacore.BaseTransaction.getStatus(BaseTransaction.java:142)
	at io.quarkus.narayana.jta.runtime.NotifyingTransactionManager.getStatus(NotifyingTransactionManager.java:106)

[yrodiere]

You can only initialize classes at runtime if they're not used during static init. Probably not your case here?

[zhfeng]

@yrodiere is there any way to re-compute the static variables? We have to move the assign statment to a static block?

like

private static final Class LAST_RESOURCE_OPTIMISATION_INTERFACE = jtaPropertyManager.getJTAEnvironmentBean()
            .getLastResourceOptimisationInterface();

change to

private static Class LAST_RESOURCE_OPTIMISATION_INTERFACE;

static {
        LAST_RESOURCE_OPTIMISATION_INTERFACE = jtaPropertyManager.getJTAEnvironmentBean().getLastResourceOptimisationInterface();
}

[mmusgrov]

@zhfeng the narayana-jta extension needs to know about the new class:

[mmusgrov@dev2 quarkus] (main) $ git diff extensions/narayana-jta/deployment/src/main/java/io/quarkus/narayana/jta/deployment/NarayanaJtaProcessor.java
diff --git a/extensions/narayana-jta/deployment/src/main/java/io/quarkus/narayana/jta/deployment/NarayanaJtaProcessor.java b/extensions/narayana-jta/deployment/src/main/java/io/quarkus/narayana/jta/deployment/NarayanaJtaProcessor.java
index 396279af870..253fa7810ac 100644
--- a/extensions/narayana-jta/deployment/src/main/java/io/quarkus/narayana/jta/deployment/NarayanaJtaProcessor.java
+++ b/extensions/narayana-jta/deployment/src/main/java/io/quarkus/narayana/jta/deployment/NarayanaJtaProcessor.java
@@ -99,6 +99,7 @@ public void build(NarayanaJtaRecorder recorder,
         additionalBeans.produce(new AdditionalBeanBuildItem(NarayanaJtaProducers.class));
         additionalBeans.produce(AdditionalBeanBuildItem.unremovableOf("io.quarkus.narayana.jta.RequestScopedTransaction"));
 
+        runtimeInit.produce(new RuntimeInitializedClassBuildItem(com.arjuna.ats.jta.resources.LastResourceCommitOptimisation.class.getName()));

Although it is a while since I looked at extension code so I will defer to your expertise.