quarkiverse / quarkus-ironjacamar

IronJacamar is an implementation of the Jakarta Connector Architecture specification
https://docs.quarkiverse.io/quarkus-ironjacamar/dev/index.html
Apache License 2.0
4 stars 2 forks source link

Support Transaction Manager Recovery #71

Closed gastaldi closed 11 months ago

gastaldi commented 1 year ago
gastaldi commented 1 year ago

@jhalliday can you please take a look if this makes sense?

gastaldi commented 1 year ago

@vsevel in case you have recovery enabled, can you test with this PR if the transaction is correctly recovered in case of a server crash?

vsevel commented 1 year ago

no. we do not have recovery enabled.

zhfeng commented 11 months ago

@gastaldi I add a crash recovery test https://github.com/zhfeng/jca-recovery-test. Only test it on JVM mode.

- run the application

java -jar target/quarkus-app/quarkus-run.jar

- send a `crash` message

curl -X POST http://localhost:8080/jca?name=crash

- restart the application and you can see such logs

2023-11-21 11:19:22,476 INFO [io.qua.iro.runtime] (jca-worker-pool--1) QIJ000001: Starting Resource Adapter : ActiveMQ Artemis 2.31.2 2023-11-21 11:19:22,492 INFO [org.acm.DummyXAResourceRecovery] (main) register DummyXAResourceRecovery 2023-11-21 11:19:22,496 INFO [org.apa.act.art.ra.ActiveMQRALogger] (jca-worker-pool--1) AMQ151007: Resource adaptor started 2023-11-21 11:19:22,534 INFO [io.quarkus] (main) code-with-quarkus 1.0.0-SNAPSHOT on JVM (powered by Quarkus 3.2.9.Final) started in 0.627s. Listening on: http://0.0.0.0:8080 2023-11-21 11:19:22,534 INFO [io.quarkus] (main) Profile prod activated. 2023-11-21 11:19:22,534 INFO [io.quarkus] (main) Installed features: [cdi, ironjacamar, narayana-jta, resteasy, smallrye-context-propagation, vertx] 2023-11-21 11:19:32,572 INFO [org.acm.DummyXAResourceRecovery] (Periodic Recovery) DummyXAResourceRecovery returning list of resources: [org.acme.DummyXAResource@17ad6ab0] 2023-11-21 11:19:32,577 INFO [org.acm.DummyXAResource] (Periodic Recovery) Committing DummyXAResource

- check the message

curl http://localhost:8080/jca


and you will get `Hello crash`.
gastaldi commented 11 months ago

I'll do a minor refactoring before merging this PR

zhfeng commented 11 months ago

@gastaldi I have some changes to add a property to disable passing XATerminator in some case. see https://github.com/zhfeng/quarkus-ironjacamar/commit/af88d23b1be05d84b1cd35324c31dd546e90247b

zhfeng commented 11 months ago

Is there any chance to add it?

gastaldi commented 11 months ago

@zhfeng Interesting, but it doesn't make much sense to me TBH. Can you elaborate on why this is needed?

zhfeng commented 11 months ago

Well, when we did a test with DTPRA (which is used to connect to the mainframe system), it would use XATerminator to run a inboundXids recovery in a seperate thread like DTPMAIN. And this causes the some failures confilcting with quarkus-agroal in narayana recovery thread due to https://issues.redhat.com/browse/AG-227 . Now the only way to disable its recovery thread in DTPRA is disabling passing XATerminator in BootStrapContext. Yeah, this looks like a workaround and we need to fix AG-227 later. Also it could re-consider with https://issues.redhat.com/browse/JBTM-3325 in narayana side.

gastaldi commented 11 months ago

I see. The best solution in this case IMHO is to change the DTPRA adapter to ignore that (maybe through a flag in the adapter), not the extension.

gastaldi commented 11 months ago

I'll do a minor refactoring before merging this PR

Refactoring done. @zhfeng can you review it before this gets merged?

gastaldi commented 11 months ago

I think we're now ready for a 1.1.0 release. @zhfeng Should we add something else?

zhfeng commented 11 months ago

Yeah, I think we are good for a new release. The only thing I think is to add a NOTE in docs to emphasis that if running in a XA transacction, it recommend to enable recovery by using quarkus.transaction-manager.enable-recovery=true also referring to Quarkus transaction guide . Anyway, we can do this after releasing.