From 2fe565f378be904a352db71cf45ed32f704db747 Mon Sep 17 00:00:00 2001 From: Justin Bertram Date: Wed, 14 Aug 2019 09:37:11 +0100 Subject: [PATCH 1/2] ARTEMIS-2451 eliminate knownDestinations cache --- .../artemis/jms/client/ActiveMQConnection.java | 12 ------------ .../artemis/jms/client/ActiveMQDestination.java | 10 ++++++++++ .../activemq/artemis/jms/client/ActiveMQSession.java | 10 ++++++---- .../integration/amqp/QueueAutoCreationTest.java | 3 ++- .../client/AutoCreateJmsDestinationTest.java | 12 ++++++------ .../jms/client/TemporaryDestinationTest.java | 5 ++++- 6 files changed, 28 insertions(+), 24 deletions(-) diff --git a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnection.java b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnection.java index 2300a54ad7..9241ddf94b 100644 --- a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnection.java +++ b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnection.java @@ -97,8 +97,6 @@ public class ActiveMQConnection extends ActiveMQConnectionForContextImpl impleme private final Set tempQueues = new ConcurrentHashSet<>(); - private final Set knownDestinations = new ConcurrentHashSet<>(); - private volatile boolean hasNoLocal; private volatile ExceptionListener exceptionListener; @@ -543,20 +541,10 @@ public class ActiveMQConnection extends ActiveMQConnectionForContextImpl impleme public void addTemporaryQueue(final SimpleString queueAddress) { tempQueues.add(queueAddress); - knownDestinations.add(queueAddress); } public void removeTemporaryQueue(final SimpleString queueAddress) { tempQueues.remove(queueAddress); - knownDestinations.remove(queueAddress); - } - - public void addKnownDestination(final SimpleString address) { - knownDestinations.add(address); - } - - public boolean containsKnownDestination(final SimpleString address) { - return knownDestinations.contains(address); } public boolean containsTemporaryQueue(final SimpleString queueAddress) { diff --git a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQDestination.java b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQDestination.java index 3da3a5aac0..9554b64829 100644 --- a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQDestination.java +++ b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQDestination.java @@ -352,6 +352,7 @@ public class ActiveMQDestination extends JNDIStorable implements Destination, Se private final transient ActiveMQSession session; + private transient boolean created; // Constructors -------------------------------------------------- protected ActiveMQDestination(final String address, @@ -449,6 +450,7 @@ public class ActiveMQDestination extends JNDIStorable implements Destination, Se } else { sessionToUse.deleteTemporaryTopic(this); } + setCreated(false); } finally { if (openedHere) { sessionToUse.close(); @@ -483,6 +485,14 @@ public class ActiveMQDestination extends JNDIStorable implements Destination, Se return temporary; } + public boolean isCreated() { + return created; + } + + public void setCreated(boolean created) { + this.created = created; + } + public TYPE getType() { if (thetype == null) { if (temporary) { diff --git a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java index aea3d455ba..c61df988fd 100644 --- a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java +++ b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java @@ -383,7 +383,7 @@ public class ActiveMQSession implements QueueSession, TopicSession { void checkDestination(ActiveMQDestination destination) throws JMSException { SimpleString address = destination.getSimpleAddress(); // TODO: What to do with FQQN - if (!connection.containsKnownDestination(address)) { + if (!destination.isCreated()) { try { ClientSession.AddressQuery addressQuery = session.addressQuery(address); @@ -425,7 +425,7 @@ public class ActiveMQSession implements QueueSession, TopicSession { throw JMSExceptionHelper.convertFromActiveMQException(e); } // this is done at the end, if no exceptions are thrown - connection.addKnownDestination(address); + destination.setCreated(true); } } @@ -811,7 +811,7 @@ public class ActiveMQSession implements QueueSession, TopicSession { } } - connection.addKnownDestination(dest.getSimpleAddress()); + dest.setCreated(true); consumer = createClientConsumer(dest, null, coreFilterString); } else { @@ -825,7 +825,7 @@ public class ActiveMQSession implements QueueSession, TopicSession { } } - connection.addKnownDestination(dest.getSimpleAddress()); + dest.setCreated(true); SimpleString queueName; @@ -999,6 +999,8 @@ public class ActiveMQSession implements QueueSession, TopicSession { connection.addTemporaryQueue(simpleAddress); + queue.setCreated(true); + return queue; } catch (ActiveMQException e) { throw JMSExceptionHelper.convertFromActiveMQException(e); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/QueueAutoCreationTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/QueueAutoCreationTest.java index 5d87f11d4f..ed72104bcb 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/QueueAutoCreationTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/QueueAutoCreationTest.java @@ -25,6 +25,7 @@ import org.apache.activemq.artemis.core.server.JournalType; import org.apache.activemq.artemis.core.settings.impl.AddressSettings; import org.apache.activemq.artemis.jms.client.ActiveMQConnection; import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory; +import org.apache.activemq.artemis.jms.client.ActiveMQDestination; import org.apache.activemq.artemis.jms.client.ActiveMQQueue; import org.apache.activemq.artemis.jms.client.ActiveMQTopic; import org.apache.activemq.artemis.utils.UUIDGenerator; @@ -130,7 +131,7 @@ public class QueueAutoCreationTest extends JMSClientTestSupport { producer.send(session.createTextMessage("hello")); } - Assert.assertTrue(((ActiveMQConnection)connection).containsKnownDestination(addressName)); + Assert.assertTrue(((ActiveMQDestination) topic).isCreated()); } private void sendStringOfSize(int msgSize, boolean useCoreReceive) throws JMSException { diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/AutoCreateJmsDestinationTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/AutoCreateJmsDestinationTest.java index a2855434f2..1eafdb4a1f 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/AutoCreateJmsDestinationTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/AutoCreateJmsDestinationTest.java @@ -41,15 +41,15 @@ import org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl; import org.apache.activemq.artemis.core.security.Role; import org.apache.activemq.artemis.core.server.Queue; import org.apache.activemq.artemis.core.settings.impl.AddressSettings; -import org.apache.activemq.artemis.jms.client.ActiveMQConnection; import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory; +import org.apache.activemq.artemis.jms.client.ActiveMQDestination; import org.apache.activemq.artemis.jms.client.ActiveMQQueue; import org.apache.activemq.artemis.jms.client.ActiveMQTemporaryTopic; import org.apache.activemq.artemis.jms.client.ActiveMQTopic; -import org.apache.activemq.artemis.tests.util.Wait; import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager; import org.apache.activemq.artemis.tests.integration.IntegrationTestLogger; import org.apache.activemq.artemis.tests.util.JMSTestBase; +import org.apache.activemq.artemis.tests.util.Wait; import org.apache.activemq.artemis.utils.UUIDGenerator; import org.junit.After; import org.junit.Assert; @@ -300,7 +300,7 @@ public class AutoCreateJmsDestinationTest extends JMSTestBase { producer.send(session.createTextMessage("hello")); } - Assert.assertTrue(((ActiveMQConnection)connection).containsKnownDestination(addressName)); + Assert.assertTrue((((ActiveMQDestination) topic).isCreated())); } @Test (timeout = 30000) @@ -324,7 +324,7 @@ public class AutoCreateJmsDestinationTest extends JMSTestBase { Assert.fail("Expected to throw exception here"); } catch (JMSException expected) { } - Assert.assertFalse(((ActiveMQConnection)connection).containsKnownDestination(addressName)); + Assert.assertFalse(((ActiveMQDestination) queue).isCreated()); } } @@ -349,7 +349,7 @@ public class AutoCreateJmsDestinationTest extends JMSTestBase { } catch (JMSException expected) { } - Assert.assertFalse(((ActiveMQConnection)connection).containsKnownDestination(addressName)); + Assert.assertFalse(((ActiveMQDestination) queue).isCreated()); } @@ -370,7 +370,7 @@ public class AutoCreateJmsDestinationTest extends JMSTestBase { MessageProducer producer = session.createProducer(queue); Assert.assertNotNull(server.locateQueue(addressName)); - Assert.assertTrue(((ActiveMQConnection) connection).containsKnownDestination(addressName)); + Assert.assertTrue(((ActiveMQDestination) queue).isCreated()); } } diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/client/TemporaryDestinationTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/client/TemporaryDestinationTest.java index 01cde6305c..850359aaf0 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/client/TemporaryDestinationTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/client/TemporaryDestinationTest.java @@ -33,6 +33,7 @@ import org.apache.activemq.artemis.core.server.ServerSession; import org.apache.activemq.artemis.core.server.impl.ServerSessionImpl; import org.apache.activemq.artemis.core.settings.impl.AddressSettings; import org.apache.activemq.artemis.jms.client.ActiveMQConnection; +import org.apache.activemq.artemis.jms.client.ActiveMQDestination; import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager; import org.apache.activemq.artemis.tests.util.JMSTestBase; import org.junit.Before; @@ -79,9 +80,11 @@ public class TemporaryDestinationTest extends JMSTestBase { consumer.close(); + assertTrue(((ActiveMQDestination) tempQueue).isCreated()); + tempQueue.delete(); - assertFalse(conn.containsKnownDestination(SimpleString.toSimpleString(tempQueue.getQueueName()))); + assertFalse(((ActiveMQDestination) tempQueue).isCreated()); assertFalse(conn.containsTemporaryQueue(SimpleString.toSimpleString(tempQueue.getQueueName()))); } finally { From a48e2c9b6f2b54481acdcd26cdbe2caf50f715c1 Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Tue, 20 Aug 2019 10:49:39 -0400 Subject: [PATCH 2/2] ARTEMIS-2451 Some additional tests --- .../amqp/QueueAutoCreationTest.java | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/QueueAutoCreationTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/QueueAutoCreationTest.java index ed72104bcb..5de4fb460c 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/QueueAutoCreationTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/QueueAutoCreationTest.java @@ -134,6 +134,62 @@ public class QueueAutoCreationTest extends JMSClientTestSupport { Assert.assertTrue(((ActiveMQDestination) topic).isCreated()); } + @Test(timeout = 30000) + // QueueAutoCreationTest was created to validate auto-creation of queues + // and this test was added to validate a regression: https://issues.apache.org/jira/browse/ARTEMIS-2238 + public void testAutoCreateOnTopicManySends() throws Exception { + ConnectionFactory factory = new ActiveMQConnectionFactory("tcp://localhost:5672"); + Connection connection = factory.createConnection(); + SimpleString addressName = UUIDGenerator.getInstance().generateSimpleStringUUID(); + System.out.println("Address is " + addressName); + clientSession.createAddress(addressName, RoutingType.ANYCAST, false); + + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + for (int i = 0; i < 10; i++) { + Topic topic = new ActiveMQTopic(addressName.toString()); + MessageProducer producer = session.createProducer(topic); + producer.send(session.createTextMessage("hello")); + producer.close(); + Assert.assertTrue(((ActiveMQDestination) topic).isCreated()); + } + + } + + @Test(timeout = 30000) + // QueueAutoCreationTest was created to validate auto-creation of queues + // and this test was added to validate a regression: https://issues.apache.org/jira/browse/ARTEMIS-2238 + public void testAutoCreateOnTopicAndConsume() throws Exception { + ConnectionFactory factory = new ActiveMQConnectionFactory("tcp://localhost:5672"); + Connection connection = factory.createConnection(); + SimpleString addressName = UUIDGenerator.getInstance().generateSimpleStringUUID(); + System.out.println("Address is " + addressName); + clientSession.createAddress(addressName, RoutingType.ANYCAST, false); + + Connection recConnection = factory.createConnection(); + Session recSession = recConnection.createSession(Session.AUTO_ACKNOWLEDGE); + Topic topicConsumer = recSession.createTopic(addressName.toString()); + MessageConsumer consumer = recSession.createConsumer(topicConsumer); + recConnection.start(); + + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + for (int i = 0; i < 10; i++) { + Topic topic = session.createTopic(addressName.toString()); + MessageProducer producer = session.createProducer(topic); + producer.send(session.createTextMessage("hello")); + producer.close(); + Assert.assertTrue(((ActiveMQDestination) topic).isCreated()); + } + + for (int i = 0; i < 10; i++) { + TextMessage message = (TextMessage)consumer.receive(10_000); + Assert.assertNotNull(message); + Assert.assertEquals("hello", message.getText()); + } + + Assert.assertNull(consumer.receiveNoWait()); + + } + private void sendStringOfSize(int msgSize, boolean useCoreReceive) throws JMSException { Connection conn = this.createConnection();