From 9680e24607d2bb3653fed614b66555b145fddf76 Mon Sep 17 00:00:00 2001 From: Justin Bertram Date: Tue, 18 Dec 2018 11:48:12 -0600 Subject: [PATCH] ARTEMIS-2221 avoid unnecessary Bindings instance creation When trying to get the bindings for an address the getBindingsForAddress method will create a Bindings instance if there are no bindings for the address. This is unnecessary in most circumstances so use the lookupBindingsForAddress method instead and check for null. --- .../protocol/openwire/OpenWireConnection.java | 20 +++++---- .../impl/ActiveMQServerControlImpl.java | 4 +- .../management/impl/AddressControlImpl.java | 42 ++++++++++++------- .../core/postoffice/impl/PostOfficeImpl.java | 14 ++++--- .../artemis/core/server/impl/QueueImpl.java | 8 ++-- .../core/server/impl/ScaleDownHandler.java | 16 +++---- 6 files changed, 60 insertions(+), 44 deletions(-) diff --git a/artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java b/artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java index ced463b619a..8964bcb1a04 100644 --- a/artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java +++ b/artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java @@ -1033,17 +1033,19 @@ public void removeDestination(ActiveMQDestination dest) throws Exception { } else { - Bindings bindings = server.getPostOffice().getBindingsForAddress(new SimpleString(dest.getPhysicalName())); + Bindings bindings = server.getPostOffice().lookupBindingsForAddress(new SimpleString(dest.getPhysicalName())); - for (Binding binding : bindings.getBindings()) { - Queue b = (Queue) binding.getBindable(); - if (b.getConsumerCount() > 0) { - throw new Exception("Destination still has an active subscription: " + dest.getPhysicalName()); - } - if (b.isDurable()) { - throw new Exception("Destination still has durable subscription: " + dest.getPhysicalName()); + if (bindings != null) { + for (Binding binding : bindings.getBindings()) { + Queue b = (Queue) binding.getBindable(); + if (b.getConsumerCount() > 0) { + throw new Exception("Destination still has an active subscription: " + dest.getPhysicalName()); + } + if (b.isDurable()) { + throw new Exception("Destination still has durable subscription: " + dest.getPhysicalName()); + } + b.deleteQueue(); } - b.deleteQueue(); } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java index 96f352e8fc3..a1273896fec 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java @@ -1069,8 +1069,8 @@ public String listBindingsForAddress(String address) throws Exception { clearIO(); try { - final Bindings bindings = server.getPostOffice().getBindingsForAddress(new SimpleString(address)); - return bindings.getBindings().stream().map(Binding::toManagementString).collect(Collectors.joining(",")); + final Bindings bindings = server.getPostOffice().lookupBindingsForAddress(new SimpleString(address)); + return bindings == null ? "" : bindings.getBindings().stream().map(Binding::toManagementString).collect(Collectors.joining(",")); } finally { blockOnIO(); } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/AddressControlImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/AddressControlImpl.java index b24c370dace..b7520b72374 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/AddressControlImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/AddressControlImpl.java @@ -130,14 +130,18 @@ public String getRoutingTypesAsJSON() throws Exception { public String[] getQueueNames() throws Exception { clearIO(); try { - Bindings bindings = postOffice.getBindingsForAddress(addressInfo.getName()); - List queueNames = new ArrayList<>(); - for (Binding binding : bindings.getBindings()) { - if (binding instanceof QueueBinding) { - queueNames.add(binding.getUniqueName().toString()); + Bindings bindings = postOffice.lookupBindingsForAddress(addressInfo.getName()); + if (bindings != null) { + List queueNames = new ArrayList<>(); + for (Binding binding : bindings.getBindings()) { + if (binding instanceof QueueBinding) { + queueNames.add(binding.getUniqueName().toString()); + } } + return queueNames.toArray(new String[queueNames.size()]); + } else { + return new String[0]; } - return queueNames.toArray(new String[queueNames.size()]); } catch (Throwable t) { throw new IllegalStateException(t.getMessage()); } finally { @@ -149,13 +153,17 @@ public String[] getQueueNames() throws Exception { public String[] getBindingNames() throws Exception { clearIO(); try { - Bindings bindings = postOffice.getBindingsForAddress(addressInfo.getName()); - String[] bindingNames = new String[bindings.getBindings().size()]; - int i = 0; - for (Binding binding : bindings.getBindings()) { - bindingNames[i++] = binding.getUniqueName().toString(); + Bindings bindings = postOffice.lookupBindingsForAddress(addressInfo.getName()); + if (bindings != null) { + String[] bindingNames = new String[bindings.getBindings().size()]; + int i = 0; + for (Binding binding : bindings.getBindings()) { + bindingNames[i++] = binding.getUniqueName().toString(); + } + return bindingNames; + } else { + return new String[0]; } - return bindingNames; } catch (Throwable t) { throw new IllegalStateException(t.getMessage()); } finally { @@ -234,10 +242,12 @@ public long getNumberOfMessages() throws Exception { clearIO(); long totalMsgs = 0; try { - Bindings bindings = postOffice.getBindingsForAddress(addressInfo.getName()); - for (Binding binding : bindings.getBindings()) { - if (binding instanceof QueueBinding) { - totalMsgs += ((QueueBinding) binding).getQueue().getMessageCount(); + Bindings bindings = postOffice.lookupBindingsForAddress(addressInfo.getName()); + if (bindings != null) { + for (Binding binding : bindings.getBindings()) { + if (binding instanceof QueueBinding) { + totalMsgs += ((QueueBinding) binding).getQueue().getMessageCount(); + } } } return totalMsgs; diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java index 5346d6c736a..3da4d0a2d91 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java @@ -630,12 +630,14 @@ public AddressInfo getAddressInfo(SimpleString addressName) { @Override public List listQueuesForAddress(SimpleString address) throws Exception { - Bindings bindingsForAddress = getBindingsForAddress(address); + Bindings bindingsForAddress = lookupBindingsForAddress(address); List queues = new ArrayList<>(); - for (Binding b : bindingsForAddress.getBindings()) { - if (b instanceof QueueBinding) { - Queue q = ((QueueBinding) b).getQueue(); - queues.add(q); + if (bindingsForAddress != null) { + for (Binding b : bindingsForAddress.getBindings()) { + if (b instanceof QueueBinding) { + Queue q = ((QueueBinding) b).getQueue(); + queues.add(q); + } } } return queues; @@ -770,7 +772,7 @@ private void deleteDuplicateCache(SimpleString address) throws Exception { @Override public boolean isAddressBound(final SimpleString address) throws Exception { - Bindings bindings = getBindingsForAddress(address); + Bindings bindings = lookupBindingsForAddress(address); return bindings != null && !bindings.getBindings().isEmpty(); } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java index 292cfc1e520..b24a010b8af 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java @@ -2988,9 +2988,9 @@ private void expire(final Transaction tx, final MessageReference ref) throws Exc SimpleString expiryAddress = addressSettingsRepository.getMatch(address.toString()).getExpiryAddress(); if (expiryAddress != null) { - Bindings bindingList = postOffice.getBindingsForAddress(expiryAddress); + Bindings bindingList = postOffice.lookupBindingsForAddress(expiryAddress); - if (bindingList.getBindings().isEmpty()) { + if (bindingList == null || bindingList.getBindings().isEmpty()) { ActiveMQServerLogger.LOGGER.errorExpiringReferencesNoBindings(expiryAddress); acknowledge(tx, ref, AckReason.EXPIRED, null); } else { @@ -3016,9 +3016,9 @@ private void sendToDeadLetterAddress(final Transaction tx, final MessageReference ref, final SimpleString deadLetterAddress) throws Exception { if (deadLetterAddress != null) { - Bindings bindingList = postOffice.getBindingsForAddress(deadLetterAddress); + Bindings bindingList = postOffice.lookupBindingsForAddress(deadLetterAddress); - if (bindingList.getBindings().isEmpty()) { + if (bindingList == null || bindingList.getBindings().isEmpty()) { ActiveMQServerLogger.LOGGER.messageExceededMaxDelivery(ref, deadLetterAddress); ref.acknowledge(tx, AckReason.KILLED, null); } else { diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ScaleDownHandler.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ScaleDownHandler.java index 7585165b315..db51dcc42a7 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ScaleDownHandler.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ScaleDownHandler.java @@ -115,16 +115,18 @@ public long scaleDownMessages(ClientSessionFactory sessionFactory, // perform a loop per address for (SimpleString address : postOffice.getAddresses()) { logger.debug("Scaling down address " + address); - Bindings bindings = postOffice.getBindingsForAddress(address); + Bindings bindings = postOffice.lookupBindingsForAddress(address); // It will get a list of queues on this address, ordered by the number of messages Set queues = new TreeSet<>(new OrderQueueByNumberOfReferencesComparator()); - for (Binding binding : bindings.getBindings()) { - if (binding instanceof LocalQueueBinding) { - Queue queue = ((LocalQueueBinding) binding).getQueue(); - // as part of scale down we will cancel any scheduled message and pass it to theWhile we scan for the queues we will also cancel any scheduled messages and deliver them right away - queue.deliverScheduledMessages(); - queues.add(queue); + if (bindings != null) { + for (Binding binding : bindings.getBindings()) { + if (binding instanceof LocalQueueBinding) { + Queue queue = ((LocalQueueBinding) binding).getQueue(); + // as part of scale down we will cancel any scheduled message and pass it to theWhile we scan for the queues we will also cancel any scheduled messages and deliver them right away + queue.deliverScheduledMessages(); + queues.add(queue); + } } }