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 transactions in MessageEndpoint #25

Closed gastaldi closed 1 year ago

gastaldi commented 1 year ago

/cc @tadamski @vsevel

vsevel commented 1 year ago

hello. I tested this morning with your branch transactional. I don't see a change in behavior. I am testing with:

    @Override
    @Transactional(Transactional.TxType.REQUIRES_NEW)
    public void onMessage(Message message) {
        try {
            log.info("Received message: " + message.getBody(String.class) + "; tx is active = "
                    + QuarkusTransaction.isActive());
            if (message.getJMSRedelivered()) {
                log.info("accepting message on the second try");
            } else {
                log.info("failing the transaction");
                QuarkusTransaction.setRollbackOnly();
                // throw new RuntimeException("oops");
            }
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
    }

when I fail the tx with QuarkusTransaction.setRollbackOnly(); messages get consumed normally. when I fail the text with throw new RuntimeException("oops"); messages stay pending then get redelivered.

this probably means that the delivery is not associated with the jta tx.

gastaldi commented 1 year ago

@vsevel does it work if you remove the Transactional.TxType.REQUIRES_NEW?

vsevel commented 1 year ago

I see issues with the producer use case as well. I added some persistence when sending the message:

    @Path("/sendcreate")
    @GET
    @Transactional
    public String sendcreate() {
        try (JMSContext context = factory.createContext()) {
            Queue myQueue = context.createQueue("jms.queue.X");
            JMSProducer producer = context.createProducer();
            String giftDescription = "hello" + new Date();
            santaClausService.createGift(giftDescription);
            producer.send(myQueue, giftDescription);
            return "sent and created " + giftDescription;
        }
    }

I can see the message being received and the row being persisted. I was expecting an error saying that you can't enlist 2 non-xa resource managers.

does it work if you remove the Transactional.TxType.REQUIRES_NEW?

    @Override
    // @Transactional(Transactional.TxType.REQUIRES_NEW)
    public void onMessage(Message message) {
        try {
            boolean active = QuarkusTransaction.isActive();
            log.info("Received message: " + message.getBody(String.class) + "; tx is active = " + active);
            // rollback(message);
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
    }

yes. is that what you wanted me to test?

vsevel commented 1 year ago

and this works as well:

    @Override
    // @Transactional(Transactional.TxType.REQUIRES_NEW)
    public void onMessage(Message message) {
        try {
            boolean active = QuarkusTransaction.isActive();
            log.info("Received message: " + message.getBody(String.class) + "; tx is active = " + active);
            rollback(message);
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
    }

    private void rollback(Message message) throws JMSException {
        if (message.getJMSRedelivered()) {
            log.info("accepting message on the second try");
        } else {
            log.info("failing the transaction");
            // QuarkusTransaction.setRollbackOnly();
            throw new RuntimeException("oops");
        }
    }

message gets rolled back, and reprocessed.

gastaldi commented 1 year ago

yes. is that what you wanted me to test?

I mean to keep the @Transactional annotation but not the REQUIRES_NEW (use REQUIRED which is the default)

gastaldi commented 1 year ago

hello. I tested this morning with your branch transactional. I don't see a change in behavior. I am testing with:

    @Override
    @Transactional(Transactional.TxType.REQUIRES_NEW)
    public void onMessage(Message message) {
        try {
            log.info("Received message: " + message.getBody(String.class) + "; tx is active = "
                    + QuarkusTransaction.isActive());
            if (message.getJMSRedelivered()) {
                log.info("accepting message on the second try");
            } else {
                log.info("failing the transaction");
                QuarkusTransaction.setRollbackOnly();
                // throw new RuntimeException("oops");
            }
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
    }

when I fail the tx with QuarkusTransaction.setRollbackOnly(); messages get consumed normally. when I fail the text with throw new RuntimeException("oops"); messages stay pending then get redelivered.

this probably means that the delivery is not associated with the jta tx.

You can't control the message delivery transaction using JTA if you are using REQUIRES_NEW. Use REQUIRED (preferred), SUPPORTS or MANDATORY

gastaldi commented 1 year ago

Also, because the delivery is managed by JTA, Artemis will always return a XASession, so it doesn't matter if you use JMSContext or XAJMSContext, as the behavior will be the same

vsevel commented 1 year ago

in the ejb spec only REQUIRED and NOT_SUPPORTED were allowed from what I remember. in my last tests I used REQUIRED , but that does not change the outcome. unless I missed something, it does not seem there is enlistment. do you see something else?

gastaldi commented 1 year ago

@vsevel yes, it works for me, as you can see in the TransactionTest

vsevel commented 1 year ago

I used REQUIRED on the consumer test, and that is OK. let me try again on the producer side.

vsevel commented 1 year ago

this should fail and it does not:

    @Path("/sendcreate")
    @GET
    @Transactional(Transactional.TxType.REQUIRES_NEW)
    public String sendcreate() {
        String giftDescription = "hello" + new Date();
        try (JMSContext context = factory.createContext()) {
            Queue myQueue = context.createQueue("jms.queue.com.lodh.jee.ArteTest.Bank.jms.bankref1.QueueB");
            JMSProducer producer = context.createProducer();
            producer.send(myQueue, giftDescription);
        }
        // QuarkusTransaction.setRollbackOnly();
        santaClausService.createGift(giftDescription);
        return "sent and created " + giftDescription;
    }

we should have an error on registering 2 non xa resource managers

vsevel commented 1 year ago

same with

    @Path("/sendcreate")
    @GET
    @Transactional
    public String sendcreate() {
        String giftDescription = "hello" + new Date();
        try (JMSContext context = factory.createContext()) {
            Queue myQueue = context.createQueue("jms.queue.com.lodh.jee.ArteTest.Bank.jms.bankref1.QueueB");
            JMSProducer producer = context.createProducer();
            producer.send(myQueue, giftDescription);
        }
        santaClausService.createGift(giftDescription);
        // QuarkusTransaction.setRollbackOnly();
        return "sent and created " + giftDescription;
    }

it passes and it should not

gastaldi commented 1 year ago

The ConnectionFactory is an XA resource (see https://github.com/quarkiverse/quarkus-ironjacamar/pull/25#issuecomment-1705573579) so it doesn't matter if you're using ConnectionFactory or XAConnectionFactory, they are the same object returned by the RA and enlisted in the JTA transaction. I need to test with JPA to check the behavior.

Is the message sent if you rollback the transaction (when you uncomment QuarkusTransaction.setRollbackOnly())?

vsevel commented 1 year ago

but there is an enlistement, for instance this does not push any message (which is correct):

   @Path("/sendcreate")
    @GET
    @Transactional
    public String sendcreate() {
        String giftDescription = "hello" + new Date();
        try (JMSContext context = factory.createContext()) {
            Queue myQueue = context.createQueue("jms.queue.com.lodh.jee.ArteTest.Bank.jms.bankref1.QueueB");
            JMSProducer producer = context.createProducer();
            producer.send(myQueue, giftDescription);
        }
        santaClausService.createGift(giftDescription);
        QuarkusTransaction.setRollbackOnly();
        return "sent and created " + giftDescription;
    }

I need to check what is happening on the db side, all for tonight. cheers.

gastaldi commented 1 year ago

@vsevel I improved the Artemis IT by adding a PostgreSQL DB and Panache. Tests look okay to me

vsevel commented 1 year ago

hello @gastaldi I did some more tests. I am assuming non xa behavior, and it behaves as if it was supporting it. specifically those tests should fail (and do not):

there should be an error saying that you can't enlist 2 non-xa resource managers in the same tx if there is no error, it is either:

it is probably one of the last 2 situations. if so we should have the tx logs somewhere (e.g. data\tx-object-store) and a recovery manager?? and that would make the application stateful. we need to investigate that. the default behavior for quarkus should assume the app to be stateless (12 factors, microservice, ...)

gastaldi commented 1 year ago

I'll merge this and investigate the need for XA in a separate issue