messaginghub / pooled-jms

A JMS Connection pool for messaging applications supporting JMS 1.1 and 2.0 and Jakarta JMS clients
Apache License 2.0
50 stars 24 forks source link

NotForMerging: failing testSendRecvRecvSendRecvWith{AMQP,Core} #8

Closed jirkadanek closed 6 years ago

jirkadanek commented 6 years ago

This is the test that is failing for me, even though I believe it should pass. If you change Artemis version in pom.xml to <artemis.version>2.4.0</artemis.version>, then the AMQP variant passes and Core variant fails. If you leave broker at 2.5.0 (or set the upcoming 2.6.0), then both tests fail. The line where assertion error is thrown if the test fails is marked by comment on the PR.

Can you please have a look, what is happening here?

tabish121 commented 6 years ago

Can you squash this into one commit to make it easier to digest?

tabish121 commented 6 years ago

After some digging and messing around with the test it appears to be due to some broker configuration defaults that changed between those Artemis releases that are causing the broker to purge the queue when there's no consumer attached to them so the message your test sends is getting lost. You need to create an address / queue and tell it not to purge itself etc and use that queue name in the test, see the patch below.

diff --git a/pooled-jms-interop-tests/pooled-jms-artemis-tests/src/test/java/org/messaginghub/pooled/jms/ArtemisJmsPoolTestSupport.java b/pooled-jms-interop-tests/pooled-jms-artemis-tests/src/test/java/org/messaginghub/pooled/jms/ArtemisJmsPoolTestSupport.java
index f20964e..d3501e6 100644
--- a/pooled-jms-interop-tests/pooled-jms-artemis-tests/src/test/java/org/messaginghub/pooled/jms/ArtemisJmsPoolTestSupport.java
+++ b/pooled-jms-interop-tests/pooled-jms-artemis-tests/src/test/java/org/messaginghub/pooled/jms/ArtemisJmsPoolTestSupport.java
@@ -16,6 +16,10 @@
  */
 package org.messaginghub.pooled.jms;

+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
 import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory;
 import org.apache.activemq.artemis.junit.EmbeddedJMSResource;
 import org.junit.After;
@@ -39,6 +43,12 @@
     public void setUp() throws Exception {
         LOG.info("========== started test: " + getTestName() + " ==========");

+        ActiveMQServer amqServer = server.getJmsServer().getActiveMQServer();
+
+        // Default Queue
+        amqServer.addAddressInfo(new AddressInfo(SimpleString.toSimpleString(getQueueName()), RoutingType.ANYCAST));
+        amqServer.createQueue(SimpleString.toSimpleString(getQueueName()), RoutingType.ANYCAST, SimpleString.toSimpleString(getQueueName()), null, true, false, -1, false, true);
+
         artemisJmsConnectionFactory = new ActiveMQConnectionFactory(server.getVmURL());

         cf = new JmsPoolConnectionFactory();
@@ -56,6 +66,10 @@
         LOG.info("========== completed test: " + getTestName() + " ==========");
     }

+    public String getQueueName() {
+        return getTestName();
+     }
+
     public String getTestName() {
         return name.getMethodName();
     }
jirkadanek commented 6 years ago

Thanks for figuring this out. I ran git bisect between 2.4.0 and 2.5.0 ActiveMQ Artemis tags to find out which commit changed the behavior for AMQP and it was

jdanek@nixos ~/Work/repos/activemq-artemis (git)-[16d9b19...|bisect] % git bisect good 1d1d6c8b4686f869df0ca5fc09c20128f8481cff is the first bad commit commit 1d1d6c8b4686f869df0ca5fc09c20128f8481cff Author: Clebert Suconic clebertsuconic@apache.org Date: Tue Nov 21 10:12:20 2017 -0500

ARTEMIS-1416 Implementing cache on queue and address querying

This will cache the last query, optimizing most of the cases
This won't optimize the case where you are sending producers with different address,
but this is not the one I'm after now.

:040000 040000 315357d1cfdfb9a71a081b8976ad54f5f5070ab3 50eb2ba0ae07fdb77b46575d9d7a5f84bd4f6483 M artemis-protocols

Since this does not seem intentional (perf optimization is changing behavior?) I will investigate a bit more and report this to Artemis.

tabish121 commented 6 years ago

Recommend closing this then as it is broker behavior unrelated to the JMS pool

jirkadanek commented 6 years ago

Artemis issue reported as https://issues.apache.org/jira/browse/ARTEMIS-1881 Queue autocreate together with multiple producers/consumers on the same session stopped working with AMQP protocol.