Support JDBC ObjectStore in narayana-jta extension

[zhfeng]

These changes could fix

• Allow configuring the Narayana transaction log to be stored in a Datasource #31066

But we still need

• Upgrade narayana to 6.0.1.Final which includes the fix of JBTM-3754
• Add some test cases with JDBC ObjectStore
• Support table prefix

[rsvoboda]

@zhfeng please add documentation too

Are there any specifics per various DB vendors?

CC @pjgg

[mmusgrov]

There is a requirement to disable the automatic creation of the schema for the JDBCStore so we need to expose the following Narayana config properties (with false as the default for createTable):

ObjectStoreEnvironmentBean.createTable
ObjectStoreEnvironmentBean.dropTable
and we should also have
ObjectStoreEnvironmentBean.tablePrefix

and we also need to wait for the release of 6.0.1.Final

[zhfeng]

@mmusgrov The narayana-lra tests are failing, and I run them locally then find the following WARN messages

21:09:47 WARN  [io.na.lra] (executor-thread-1) LRA025014: reason 'WebApplicationException null: LRA025001: LRA created with an unexpected status code: 417, coordinator response 'LRA025028: Demanded API version '1.1' is not in the list of the supported versions '[Ljava.lang.String;@4797d38c'. Please, provide the right version for the API.'': container request for method 'org.acme.quickstart.lra.coordinator.TransactionalResource#startTx': lra: 'context'
21:09:47 WARN  [io.na.lra] (executor-thread-1) LRA025025: Unable to process LRA annotations: -3: StartFailed (null: LRA025001: LRA created with an unexpected status code: 417, coordinator response 'LRA025028: Demanded API version '1.1' is not in the list of the supported versions '[Ljava.lang.String;@4797d38c'. Please, provide the right version for the API.') ()'

narayana-lra 6.0.1.Final does not support LRA 1.1? I revert to 6.0.0.Final and tests are OK.

[mmusgrov]

@mmusgrov The narayana-lra tests are failing, and I run them locally then find the following WARN messages

21:09:47 WARN  [io.na.lra] (executor-thread-1) LRA025014: reason 'WebApplicationException null: LRA025001: LRA created with an unexpected status code: 417, coordinator response 'LRA025028: Demanded API version '1.1' is not in the list of the supported versions '[Ljava.lang.String;@4797d38c'. Please, provide the right version for the API.'': container request for method 'org.acme.quickstart.lra.coordinator.TransactionalResource#startTx': lra: 'context'
21:09:47 WARN  [io.na.lra] (executor-thread-1) LRA025025: Unable to process LRA annotations: -3: StartFailed (null: LRA025001: LRA created with an unexpected status code: 417, coordinator response 'LRA025028: Demanded API version '1.1' is not in the list of the supported versions '[Ljava.lang.String;@4797d38c'. Please, provide the right version for the API.') ()'

narayana-lra 6.0.1.Final does not support LRA 1.1? I revert to 6.0.0.Final and tests are OK.

The 6.0.0.Final version of the coordinator does not support later versions of the client due to JBTM-3767 (LRA API versioning scheme is too restrictive). We've got a PR to fix it so the next version of the coordinator will be more permissive with regards to API versioning.

[michalvavrik]

@zhfeng please can you update me on your plans considering Naryana 6.0.1.Final is out now? Also please kindly:

• document how to use the JDBCStore transaction log in Quarkus and in particular how to achieve recovery of inflight transactions
• add testes that comprehensively test feature as far as Quarkus is concerned, status request you have there seems insufficient. I this this is WIP, so if that's your plan anyway, please ignore this, thanks

[mmusgrov]

@michalvavrik @zhfeng I have delayed the announcement of 6.0.1.Final while I investigate what looks like a performance regression, when I have a better understanding of the issue I shall provide an update

[michalvavrik]

ok, thanks @mmusgrov

[zhfeng]

@michalvavrik I think we also need a new release of lra-coordinator-quarkus to make the lra tck tests work again with Narayana 6.0.1.Final. I've opened

• Upgrade Narayana to 6.0.1.Final and Quarkus to 3.0.0.Final jbosstm/lra-coordinator-quarkus#40

And there is a test plan for this feature https://github.com/quarkus-qe/quarkus-test-plans/blob/main/QUARKUS-2739.md

[mmusgrov]

I have raised quarkus pr #32922 for the upgrade to 6.0.1.Final

[zhfeng]

Thanks @mmusgrov and I just rebase this PR to see how CI is working.

[mmusgrov]

Thanks @zhfeng it is looking good so far.
I guess you will be updating the title of the PR if it passes.

[yrodiere]

LGTM, though I added a few suggestions below. You'll probably want to clear up whether transactions can be enabled on the datasource used to store the transaction log, in particular.

[quarkus-bot]

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

[zhfeng]

@mmusgrov is there anything you need to add to documentation?

@yrodiere I'd like to open a follow up issue for

• Checking datasource used for jdbc object store is not with transactions = XA
• Enable reflection only on the driver that will actually get used, and only if actually using JDBC for the transaction log

@geoand I think this PR is good for merging. Is it possible to target for quarkus 3.1?

[geoand]

Is it possible to target for quarkus 3.1?

If it gets merged soon, that will happen automatically

[mmusgrov]

@mmusgrov is there anything you need to add to documentation?

@zhfeng Ah, I assumed that your PR is already including it: https://github.com/quarkusio/quarkus/pull/31729/files#diff-b86c4389d1f3659cbf608fadee4fbad6863020687d3f102c24b44dd00028c10b

What more do you think we need to add?

[zhfeng]

@mmusgrov the documentation looks good to me right now.

Is there anyone can approve and merge it? Thanks!

[yrodiere]

@yrodiere I'd like to open a follow up issue for

* Checking datasource used for jdbc object store is not with `transactions = XA`

* Enable reflection only on the driver that will actually get used, and only if actually using JDBC for the transaction log

Ok, but please do open that issue now, otherwise we'll just forget :)

@geoand I think this PR is good for merging. Is it possible to target for quarkus 3.1?

It's good to merge as far as I'm concerned, but I don't know a thing about Narayana, so really my approval is not worth much. I'll defer to @mmusgrov on that front, and it seems he already approved, so... let's merge :)

[Sanne]

• Enable reflection only on the driver that will actually get used, and only if actually using JDBC for the transaction log

Just curious, what is this used for?

• JDBC driver extensions already do register the driver for reflective instantiation
• I thought we agreed the ObjectStore would use exclusively the connection pool (Datasource) ?

[yrodiere]

• Enable reflection only on the driver that will actually get used, and only if actually using JDBC for the transaction log

Just curious, what is this used for?

* JDBC driver extensions already do register the driver for reflective instantiation

* I thought we agreed the ObjectStore would use exclusively the connection pool (Datasource) ?

This is not a JDBC Driver, but a native concept of driver within Narayana. From what I understand it's the equivalent of Hibernate ORM's dialects.

[Sanne]

This is not a JDBC Driver, but a native concept of driver within Narayana. From what I understand it's the equivalent of Hibernate ORM's dialects.

Thanks, thas makes sense. Wondering now if we could get such things initialized at buid time, but that's most certainly an optimisation for another day :)

[zhfeng]

Thanks @yrodiere and I raise two follow up issues.

[ahus1]

I just found out the hard way that some properties changed. Instead in Quarkus 3.1 of

quarkus.transaction-manager.object-store-directory

it is now in Quarkus 3.2

quarkus.transaction-manager.object-store.directory

where the last - changed to a .. Might be a candidate for some release notes?

[geoand]

Indeed, that is a breaking change!

[geoand]

@yrodiere mind adding an entry to the wiki?

[yrodiere]

@geoand Done: https://github.com/quarkusio/quarkus/wiki/Migration-Guide-3.2#transaction-manager-narayana

[geoand]

Thanks!