https://issues.apache.org/jira/browse/AMQ-4621

Update the consumer / connection abort process such that when the
strategy is configured to abort the connection is only attempt to do so
once instead of once for every subscription in the map.  Also improve
logging to better indicate the subscription being aborted and the
destination that the subscription was on.

activemq-broker/src/main/java/org/apache/activemq/broker/region/policy/AbortSlowConsumerStrategy.java

@@ -16,7 +16,9 @@
  */
 package org.apache.activemq.broker.region.policy;
 
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.concurrent.ConcurrentHashMap;
@@ -106,38 +108,73 @@ public class AbortSlowConsumerStrategy implements SlowConsumerStrategy, Runnable
     }
 
     protected void abortSubscription(Map<Subscription, SlowConsumerEntry> toAbort, boolean abortSubscriberConnection) {
+
+        Map<Connection, List<Subscription>> abortMap = new HashMap<Connection, List<Subscription>>();
+
         for (final Entry<Subscription, SlowConsumerEntry> entry : toAbort.entrySet()) {
             ConnectionContext connectionContext = entry.getValue().context;
-            if (connectionContext!= null) {
+            if (connectionContext == null) {
-                try {
+                continue;
-                    LOG.info("aborting "
+            }
-                            + (abortSubscriberConnection ? "connection" : "consumer")
+
-                            + ", slow consumer: " + entry.getKey());
+            Connection connection = connectionContext.getConnection();
+            if (connection == null) {
+                LOG.debug("slowConsumer abort ignored, no connection in context:"  + connectionContext);
+            }
+
+            if (!abortMap.containsKey(connection)) {
+                abortMap.put(connection, new ArrayList<Subscription>());
+            }
+
+            abortMap.get(connection).add(entry.getKey());
+        }
+
+        for (Entry<Connection, List<Subscription>> entry : abortMap.entrySet()) {
+            final Connection connection = entry.getKey();
+            final List<Subscription> subscriptions = entry.getValue();
 
-                    final Connection connection = connectionContext.getConnection();
-                    if (connection != null) {
             if (abortSubscriberConnection) {
+
+                LOG.info("aborting connection:{} with {} slow consumers",
+                         connection.getConnectionId(), subscriptions.size());
+
+                if (LOG.isTraceEnabled()) {
+                    for (Subscription subscription : subscriptions) {
+                        LOG.trace("Connection {} being aborted because of slow consumer: {} on destination: {}",
+                                  new Object[] { connection.getConnectionId(),
+                                                 subscription.getConsumerInfo().getConsumerId(),
+                                                 subscription.getActiveMQDestination() });
+                    }
+                }
+
+                try {
                     scheduler.executeAfterDelay(new Runnable() {
                         @Override
                         public void run() {
-                                    connection.serviceException(new InactivityIOException("Consumer was slow too often (>"
+                            connection.serviceException(new InactivityIOException(
+                                    subscriptions.size() + " Consumers was slow too often (>"
                                     + maxSlowCount +  ") or too long (>"
-                                            + maxSlowDuration + "): " + entry.getKey().getConsumerInfo().getConsumerId()));
+                                    + maxSlowDuration + "): "));
                         }}, 0l);
+                } catch (Exception e) {
+                    LOG.info("exception on aborting connection {} with {} slow consumers",
+                             connection.getConnectionId(), subscriptions.size());
+                }
             } else {
-                            // just abort the consumer by telling it to stop
+                // just abort each consumer by telling it to stop
+                for (Subscription subscription : subscriptions) {
+                    LOG.info("aborting slow consumer: {} for destination:{}",
+                             subscription.getConsumerInfo().getConsumerId(),
+                             subscription.getActiveMQDestination());
+
+                    try {
                         ConsumerControl stopConsumer = new ConsumerControl();
-                            stopConsumer.setConsumerId(entry.getKey().getConsumerInfo().getConsumerId());
+                        stopConsumer.setConsumerId(subscription.getConsumerInfo().getConsumerId());
                         stopConsumer.setClose(true);
                         connection.dispatchAsync(stopConsumer);
-                        }
-                    } else {
-                        LOG.debug("slowConsumer abort ignored, no connection in context:"  + connectionContext);
-                    }
                     } catch (Exception e) {
-                    LOG.info("exception on stopping "
+                        LOG.info("exception on aborting slow consumer: {}", subscription.getConsumerInfo().getConsumerId());
-                            + (abortSubscriberConnection ? "connection" : "consumer")
+                    }
-                            + " to abort slow consumer: " + entry.getKey(), e);
                 }
             }
         }