From 79eea6332f61dcba28b7312afe9c287ee7c788b9 Mon Sep 17 00:00:00 2001 From: Felix Schumacher Date: Sat, 6 Oct 2018 11:03:26 +0200 Subject: [PATCH 1/6] Use varargs for log parameters instead of `new Object[]` Some log statements where called with a combination of `new Object[] {...}` and an exception. That lead to a misinterpretation of those parameters. The first object array is considered to be the first argument and the exception the second parameter of the format string. It should have been interpreted as the members of the object array as the parameters for the format string and the exception as the exception itself. So for example: LOG.info("User {} has done {}", new Object[] {"A", "something"}, e); would log something like: User [A, something] has done java.lang.RuntimeException: OOps --- .../DemandForwardingBridgeSupport.java | 101 ++++++++---------- .../network/DiscoveryNetworkConnector.java | 4 +- .../network/jms/DestinationBridge.java | 2 +- .../activemq/network/jms/JmsConnector.java | 2 +- .../SimpleCachedLDAPAuthorizationMap.java | 16 +-- 5 files changed, 56 insertions(+), 69 deletions(-) diff --git a/activemq-broker/src/main/java/org/apache/activemq/network/DemandForwardingBridgeSupport.java b/activemq-broker/src/main/java/org/apache/activemq/network/DemandForwardingBridgeSupport.java index 6a8b66a576b..9db6d3eafb8 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/network/DemandForwardingBridgeSupport.java +++ b/activemq-broker/src/main/java/org/apache/activemq/network/DemandForwardingBridgeSupport.java @@ -281,9 +281,8 @@ public void stop() throws Exception { try { // local start complete if (startedLatch.getCount() < 2) { - LOG.trace("{} unregister bridge ({}) to {}", new Object[]{ - configuration.getBrokerName(), this, remoteBrokerName - }); + LOG.trace("{} unregister bridge ({}) to {}", + configuration.getBrokerName(), this, remoteBrokerName); brokerService.getBroker().removeBroker(null, remoteBrokerInfo); brokerService.getBroker().networkBridgeStopped(remoteBrokerInfo); } @@ -406,9 +405,8 @@ private void collectBrokerInfos() { // and if so just stop now before registering anything. remoteBrokerId = remoteBrokerInfo.getBrokerId(); if (localBrokerId.equals(remoteBrokerId)) { - LOG.trace("{} disconnecting remote loop back connector for: {}, with id: {}", new Object[]{ - configuration.getBrokerName(), remoteBrokerName, remoteBrokerId - }); + LOG.trace("{} disconnecting remote loop back connector for: {}, with id: {}", + configuration.getBrokerName(), remoteBrokerName, remoteBrokerId); ServiceSupport.dispose(localBroker); ServiceSupport.dispose(remoteBroker); // the bridge is left in a bit of limbo, but it won't get retried @@ -550,12 +548,10 @@ private void startLocalBridge() throws Throwable { // new peer broker (a consumer can work with remote broker also) brokerService.getBroker().addBroker(null, remoteBrokerInfo); - LOG.info("Network connection between {} and {} ({}) has been established.", new Object[]{ - localBroker, remoteBroker, remoteBrokerName - }); - LOG.trace("{} register bridge ({}) to {}", new Object[]{ - configuration.getBrokerName(), this, remoteBrokerName - }); + LOG.info("Network connection between {} and {} ({}) has been established.", + localBroker, remoteBroker, remoteBrokerName); + LOG.trace("{} register bridge ({}) to {}", + configuration.getBrokerName(), this, remoteBrokerName); } else { LOG.warn("Bridge was disposed before the startLocalBridge() method was fully executed."); } @@ -642,13 +638,11 @@ protected void startRemoteBridge() throws Exception { public void serviceRemoteException(Throwable error) { if (!disposed.get()) { if (error instanceof SecurityException || error instanceof GeneralSecurityException) { - LOG.error("Network connection between {} and {} shutdown due to a remote error: {}", new Object[]{ - localBroker, remoteBroker, error - }); + LOG.error("Network connection between {} and {} shutdown due to a remote error: {}", + localBroker, remoteBroker, error); } else { - LOG.warn("Network connection between {} and {} shutdown due to a remote error: {}", new Object[]{ - localBroker, remoteBroker, error - }); + LOG.warn("Network connection between {} and {} shutdown due to a remote error: {}", + localBroker, remoteBroker, error); } LOG.debug("The remote Exception was: {}", error, error); brokerService.getTaskRunnerFactory().execute(new Runnable() { @@ -947,25 +941,22 @@ private void serviceRemoteConsumerAdvisory(DataStructure data) throws IOExceptio } if (path != null && networkTTL > -1 && path.length >= networkTTL) { - LOG.debug("{} Ignoring sub from {}, restricted to {} network hops only: {}", new Object[]{ - configuration.getBrokerName(), remoteBrokerName, networkTTL, info - }); + LOG.debug("{} Ignoring sub from {}, restricted to {} network hops only: {}", + configuration.getBrokerName(), remoteBrokerName, networkTTL, info); return; } if (contains(path, localBrokerPath[0])) { // Ignore this consumer as it's a consumer we locally sent to the broker. - LOG.debug("{} Ignoring sub from {}, already routed through this broker once: {}", new Object[]{ - configuration.getBrokerName(), remoteBrokerName, info - }); + LOG.debug("{} Ignoring sub from {}, already routed through this broker once: {}", + configuration.getBrokerName(), remoteBrokerName, info); return; } if (!isPermissableDestination(info.getDestination())) { // ignore if not in the permitted or in the excluded list - LOG.debug("{} Ignoring sub from {}, destination {} is not permitted: {}", new Object[]{ - configuration.getBrokerName(), remoteBrokerName, info.getDestination(), info - }); + LOG.debug("{} Ignoring sub from {}, destination {} is not permitted: {}", + configuration.getBrokerName(), remoteBrokerName, info.getDestination(), info); return; } @@ -984,9 +975,8 @@ private void serviceRemoteConsumerAdvisory(DataStructure data) throws IOExceptio final DestinationInfo destInfo = (DestinationInfo) data; BrokerId[] path = destInfo.getBrokerPath(); if (path != null && networkTTL > -1 && path.length >= networkTTL) { - LOG.debug("{} Ignoring destination {} restricted to {} network hops only", new Object[]{ - configuration.getBrokerName(), destInfo, networkTTL - }); + LOG.debug("{} Ignoring destination {} restricted to {} network hops only", + configuration.getBrokerName(), destInfo, networkTTL); return; } if (contains(destInfo.getBrokerPath(), localBrokerPath[0])) { @@ -1000,9 +990,8 @@ private void serviceRemoteConsumerAdvisory(DataStructure data) throws IOExceptio tempDest.setConnectionId(localSessionInfo.getSessionId().getConnectionId()); } destInfo.setBrokerPath(appendToBrokerPath(destInfo.getBrokerPath(), getRemoteBrokerPath())); - LOG.trace("{} bridging {} destination on {} from {}, destination: {}", new Object[]{ - configuration.getBrokerName(), (destInfo.isAddOperation() ? "add" : "remove"), localBroker, remoteBrokerName, destInfo - }); + LOG.trace("{} bridging {} destination on {} from {}, destination: {}", + configuration.getBrokerName(), (destInfo.isAddOperation() ? "add" : "remove"), localBroker, remoteBrokerName, destInfo); if (destInfo.isRemoveOperation()) { // Serialize with removeSub operations such that all removeSub advisories // are generated @@ -1109,7 +1098,7 @@ public void serviceLocalException(MessageDispatch messageDispatch, Throwable err return; } - LOG.info("Network connection between {} and {} shutdown due to a local error: {}", new Object[]{localBroker, remoteBroker, error}); + LOG.info("Network connection between {} and {} shutdown due to a local error: {}", localBroker, remoteBroker, error); LOG.debug("The local Exception was: {}", error, error); brokerService.getTaskRunnerFactory().execute(new Runnable() { @@ -1158,7 +1147,8 @@ protected void addSubscription(DemandSubscription sub) throws IOException { protected void removeSubscription(final DemandSubscription sub) throws IOException { if (sub != null) { - LOG.trace("{} remove local subscription: {} for remote {}", new Object[]{configuration.getBrokerName(), sub.getLocalInfo().getConsumerId(), sub.getRemoteInfo().getConsumerId()}); + LOG.trace("{} remove local subscription: {} for remote {}", configuration.getBrokerName(), + sub.getLocalInfo().getConsumerId(), sub.getRemoteInfo().getConsumerId()); // ensure not available for conduit subs pending removal subscriptionMapByLocalId.remove(sub.getLocalInfo().getConsumerId()); @@ -1209,9 +1199,10 @@ protected void serviceLocalCommand(Command command) { if (sub != null && md.getMessage() != null && sub.incrementOutstandingResponses()) { if (suppressMessageDispatch(md, sub)) { - LOG.debug("{} message not forwarded to {} because message came from there or fails TTL, brokerPath: {}, message: {}", new Object[]{ - configuration.getBrokerName(), remoteBrokerName, Arrays.toString(md.getMessage().getBrokerPath()), md.getMessage() - }); + LOG.debug( + "{} message not forwarded to {} because message came from there or fails TTL, brokerPath: {}, message: {}", + configuration.getBrokerName(), remoteBrokerName, + Arrays.toString(md.getMessage().getBrokerPath()), md.getMessage()); // still ack as it may be durable try { localBroker.oneway(new MessageAck(md, MessageAck.INDIVIDUAL_ACK_TYPE, 1)); @@ -1222,9 +1213,10 @@ protected void serviceLocalCommand(Command command) { } Message message = configureMessage(md); - LOG.debug("bridging ({} -> {}), consumer: {}, destination: {}, brokerPath: {}, message: {}", new Object[]{ - configuration.getBrokerName(), remoteBrokerName, md.getConsumerId(), message.getDestination(), Arrays.toString(message.getBrokerPath()), (LOG.isTraceEnabled() ? message : message.getMessageId()) - }); + LOG.debug("bridging ({} -> {}), consumer: {}, destination: {}, brokerPath: {}, message: {}", + configuration.getBrokerName(), remoteBrokerName, md.getConsumerId(), + message.getDestination(), Arrays.toString(message.getBrokerPath()), + (LOG.isTraceEnabled() ? message : message.getMessageId())); if (isDuplex() && NetworkBridgeFilter.isAdvisoryInterpretedByNetworkBridge(message)) { try { // never request b/c they are eventually acked async @@ -1501,18 +1493,16 @@ private boolean hasLowerPriority(Subscription existingSub, ConsumerInfo candidat boolean suppress = false; if (existingSub.getConsumerInfo().getPriority() >= candidateInfo.getPriority()) { - LOG.debug("{} Ignoring duplicate subscription from {}, sub: {} is duplicate by network subscription with equal or higher network priority: {}, networkConsumerIds: {}", new Object[]{ - configuration.getBrokerName(), remoteBrokerName, candidateInfo, existingSub, existingSub.getConsumerInfo().getNetworkConsumerIds() - }); + LOG.debug("{} Ignoring duplicate subscription from {}, sub: {} is duplicate by network subscription with equal or higher network priority: {}, networkConsumerIds: {}", + configuration.getBrokerName(), remoteBrokerName, candidateInfo, existingSub, existingSub.getConsumerInfo().getNetworkConsumerIds()); suppress = true; } else { // remove the existing lower priority duplicate and allow this candidate try { removeDuplicateSubscription(existingSub); - LOG.debug("{} Replacing duplicate subscription {} with sub from {}, which has a higher priority, new sub: {}, networkConsumerIds: {}", new Object[]{ - configuration.getBrokerName(), existingSub.getConsumerInfo(), remoteBrokerName, candidateInfo, candidateInfo.getNetworkConsumerIds() - }); + LOG.debug("{} Replacing duplicate subscription {} with sub from {}, which has a higher priority, new sub: {}, networkConsumerIds: {}", + configuration.getBrokerName(), existingSub.getConsumerInfo(), remoteBrokerName, candidateInfo, candidateInfo.getNetworkConsumerIds()); } catch (IOException e) { LOG.error("Failed to remove duplicated sub as a result of sub with higher priority, sub: {}", existingSub, e); } @@ -1591,7 +1581,7 @@ protected DemandSubscription doCreateDemandSubscription(ConsumerInfo info) throw priority -= info.getBrokerPath().length + 1; } result.getLocalInfo().setPriority(priority); - LOG.debug("{} using priority: {} for subscription: {}", new Object[]{configuration.getBrokerName(), priority, info}); + LOG.debug("{} using priority: {} for subscription: {}", configuration.getBrokerName(), priority, info); } configureDemandSubscription(info, result); return result; @@ -1644,14 +1634,12 @@ protected void configureDemandSubscription(ConsumerInfo info, DemandSubscription protected void removeDemandSubscription(ConsumerId id) throws IOException { DemandSubscription sub = subscriptionMapByRemoteId.remove(id); - LOG.debug("{} remove request on {} from {}, consumer id: {}, matching sub: {}", new Object[]{ - configuration.getBrokerName(), localBroker, remoteBrokerName, id, sub - }); + LOG.debug("{} remove request on {} from {}, consumer id: {}, matching sub: {}", + configuration.getBrokerName(), localBroker, remoteBrokerName, id, sub); if (sub != null) { removeSubscription(sub); - LOG.debug("{} removed sub on {} from {}: {}", new Object[]{ - configuration.getBrokerName(), localBroker, remoteBrokerName, sub.getRemoteInfo() - }); + LOG.debug("{} removed sub on {} from {}: {}", + configuration.getBrokerName(), localBroker, remoteBrokerName, sub.getRemoteInfo()); } } @@ -1967,9 +1955,8 @@ protected boolean canDuplexDispatch(Message message) { long lastStoredForMessageProducer = getStoredSequenceIdForMessage(message.getMessageId()); if (producerSequenceId <= lastStoredForMessageProducer) { result = false; - LOG.debug("suppressing duplicate message send [{}] from network producer with producerSequence [{}] less than last stored: {}", new Object[]{ - (LOG.isTraceEnabled() ? message : message.getMessageId()), producerSequenceId, lastStoredForMessageProducer - }); + LOG.debug("suppressing duplicate message send [{}] from network producer with producerSequence [{}] less than last stored: {}", + (LOG.isTraceEnabled() ? message : message.getMessageId()), producerSequenceId, lastStoredForMessageProducer); } } return result; diff --git a/activemq-broker/src/main/java/org/apache/activemq/network/DiscoveryNetworkConnector.java b/activemq-broker/src/main/java/org/apache/activemq/network/DiscoveryNetworkConnector.java index a2c457c90e4..b3c8b7fc8af 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/network/DiscoveryNetworkConnector.java +++ b/activemq-broker/src/main/java/org/apache/activemq/network/DiscoveryNetworkConnector.java @@ -114,7 +114,7 @@ public void onServiceAdd(DiscoveryEvent event) { try { connectUri = URISupport.applyParameters(connectUri, parameters, DISCOVERED_OPTION_PREFIX); } catch (URISyntaxException e) { - LOG.warn("could not apply query parameters: {} to: {}", new Object[]{ parameters, connectUri }, e); + LOG.warn("could not apply query parameters: {} to: {}", parameters, connectUri, e); } LOG.info("Establishing network connection from {} to {}", localURI, connectUri); @@ -166,7 +166,7 @@ public void onServiceAdd(DiscoveryEvent event) { } catch (Exception e) { ServiceSupport.dispose(localTransport); ServiceSupport.dispose(remoteTransport); - LOG.warn("Could not start network bridge between: {} and: {} due to: {}", new Object[]{ localURI, uri, e.getMessage() }); + LOG.warn("Could not start network bridge between: {} and: {} due to: {}", localURI, uri, e.getMessage()); LOG.debug("Start failure exception: ", e); try { // Will remove bridge and active event. diff --git a/activemq-broker/src/main/java/org/apache/activemq/network/jms/DestinationBridge.java b/activemq-broker/src/main/java/org/apache/activemq/network/jms/DestinationBridge.java index 9595aee8239..502e4145348 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/network/jms/DestinationBridge.java +++ b/activemq-broker/src/main/java/org/apache/activemq/network/jms/DestinationBridge.java @@ -149,7 +149,7 @@ public void onMessage(Message message) { return; } catch (Exception e) { - LOG.info("failed to forward message on attempt: {} reason: {} message: {}", new Object[]{ attempt, e, message }, e); + LOG.info("failed to forward message on attempt: {} reason: {} message: {}", attempt, e, message, e); } } } diff --git a/activemq-broker/src/main/java/org/apache/activemq/network/jms/JmsConnector.java b/activemq-broker/src/main/java/org/apache/activemq/network/jms/JmsConnector.java index 6cb223a2cbc..b09b96a0872 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/network/jms/JmsConnector.java +++ b/activemq-broker/src/main/java/org/apache/activemq/network/jms/JmsConnector.java @@ -626,7 +626,7 @@ private void doInitializeConnection(boolean local) throws Exception { return; } catch(Exception e) { - LOG.debug("Failed to establish initial {} connection for JmsConnector [{}]", new Object[]{ (local ? "local" : "foreign"), attempt }, e); + LOG.debug("Failed to establish initial {} connection for JmsConnector [{}]", (local ? "local" : "foreign"), attempt, e); } finally { attempt++; } diff --git a/activemq-broker/src/main/java/org/apache/activemq/security/SimpleCachedLDAPAuthorizationMap.java b/activemq-broker/src/main/java/org/apache/activemq/security/SimpleCachedLDAPAuthorizationMap.java index 77cbb203dbf..87a606934e0 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/security/SimpleCachedLDAPAuthorizationMap.java +++ b/activemq-broker/src/main/java/org/apache/activemq/security/SimpleCachedLDAPAuthorizationMap.java @@ -237,7 +237,7 @@ protected synchronized void query() throws Exception { currentContext.search(queueSearchBase, getFilterForPermissionType(permissionType), constraints), DestinationType.QUEUE, permissionType); } catch (Exception e) { - LOG.error("Policy not applied!. Error processing policy under '{}' with filter '{}'", new Object[]{ queueSearchBase, getFilterForPermissionType(permissionType) }, e); + LOG.error("Policy not applied!. Error processing policy under '{}' with filter '{}'", queueSearchBase, getFilterForPermissionType(permissionType), e); } } @@ -247,7 +247,7 @@ protected synchronized void query() throws Exception { currentContext.search(topicSearchBase, getFilterForPermissionType(permissionType), constraints), DestinationType.TOPIC, permissionType); } catch (Exception e) { - LOG.error("Policy not applied!. Error processing policy under '{}' with filter '{}'", new Object[]{ topicSearchBase, getFilterForPermissionType(permissionType) }, e); + LOG.error("Policy not applied!. Error processing policy under '{}' with filter '{}'", topicSearchBase, getFilterForPermissionType(permissionType), e); } } @@ -257,7 +257,7 @@ protected synchronized void query() throws Exception { currentContext.search(tempSearchBase, getFilterForPermissionType(permissionType), constraints), DestinationType.TEMP, permissionType); } catch (Exception e) { - LOG.error("Policy not applied!. Error processing policy under '{}' with filter '{}'", new Object[]{ tempSearchBase, getFilterForPermissionType(permissionType) }, e); + LOG.error("Policy not applied!. Error processing policy under '{}' with filter '{}'", tempSearchBase, getFilterForPermissionType(permissionType), e); } } @@ -405,7 +405,7 @@ protected void applyACL(AuthorizationEntry entry, SearchResult result, Permissio try { memberAttributes = context.getAttributes(memberDn, new String[] { "objectClass", groupNameAttribute, userNameAttribute }); } catch (NamingException e) { - LOG.error("Policy not applied! Unknown member {} in policy entry {}", new Object[]{ memberDn, result.getNameInNamespace() }, e); + LOG.error("Policy not applied! Unknown member {} in policy entry {}", memberDn, result.getNameInNamespace(), e); continue; } @@ -419,7 +419,7 @@ protected void applyACL(AuthorizationEntry entry, SearchResult result, Permissio group = true; Attribute name = memberAttributes.get(groupNameAttribute); if (name == null) { - LOG.error("Policy not applied! Group {} does not have name attribute {} under entry {}", new Object[]{ memberDn, groupNameAttribute, result.getNameInNamespace() }); + LOG.error("Policy not applied! Group {} does not have name attribute {} under entry {}", memberDn, groupNameAttribute, result.getNameInNamespace()); break; } @@ -430,7 +430,7 @@ protected void applyACL(AuthorizationEntry entry, SearchResult result, Permissio user = true; Attribute name = memberAttributes.get(userNameAttribute); if (name == null) { - LOG.error("Policy not applied! User {} does not have name attribute {} under entry {}", new Object[]{ memberDn, userNameAttribute, result.getNameInNamespace() }); + LOG.error("Policy not applied! User {} does not have name attribute {} under entry {}", memberDn, userNameAttribute, result.getNameInNamespace()); break; } @@ -901,9 +901,9 @@ public void objectRenamed(NamingEvent namingEvent, DestinationType destinationTy } } } catch (InvalidNameException e) { - LOG.error("Policy not applied! Error parsing DN for object rename for rename of {} to {}", new Object[]{ oldBinding.getName(), newBinding.getName() }, e); + LOG.error("Policy not applied! Error parsing DN for object rename for rename of {} to {}", oldBinding.getName(), newBinding.getName(), e); } catch (Exception e) { - LOG.error("Policy not applied! Error processing object rename for rename of {} to {}", new Object[]{ oldBinding.getName(), newBinding.getName() }, e); + LOG.error("Policy not applied! Error processing object rename for rename of {} to {}", oldBinding.getName(), newBinding.getName(), e); } } From 9ad0a7d141666176a303301e910cd86a60e6c4f7 Mon Sep 17 00:00:00 2001 From: Felix Schumacher Date: Sat, 6 Oct 2018 13:43:26 +0200 Subject: [PATCH 2/6] More log messages with `new Object[]` as parameter. --- .../apache/activemq/broker/region/Queue.java | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/Queue.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/Queue.java index 86b8c6dccf0..0f9a3e2ccc4 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/Queue.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/Queue.java @@ -302,7 +302,9 @@ class BatchMessageRecoveryListener implements MessageRecoveryListener { public boolean recoverMessage(Message message) { recoveredAccumulator++; if ((recoveredAccumulator % 10000) == 0) { - LOG.info("cursor for {} has recovered {} messages. {}% complete", new Object[]{ getActiveMQDestination().getQualifiedName(), recoveredAccumulator, new Integer((int) (recoveredAccumulator * 100 / totalMessageCount))}); + LOG.info("cursor for {} has recovered {} messages. {}% complete", + getActiveMQDestination().getQualifiedName(), recoveredAccumulator, + (int) (recoveredAccumulator * 100 / totalMessageCount)); } // Message could have expired while it was being // loaded.. @@ -439,7 +441,10 @@ public QueueBrowserSubscription getBrowser() { @Override public void addSubscription(ConnectionContext context, Subscription sub) throws Exception { - LOG.debug("{} add sub: {}, dequeues: {}, dispatched: {}, inflight: {}", new Object[]{ getActiveMQDestination().getQualifiedName(), sub, getDestinationStatistics().getDequeues().getCount(), getDestinationStatistics().getDispatched().getCount(), getDestinationStatistics().getInflight().getCount() }); + LOG.debug("{} add sub: {}, dequeues: {}, dispatched: {}, inflight: {}", + getActiveMQDestination().getQualifiedName(), sub, getDestinationStatistics().getDequeues().getCount(), + getDestinationStatistics().getDispatched().getCount(), + getDestinationStatistics().getInflight().getCount()); super.addSubscription(context, sub); // synchronize with dispatch method so that no new messages are sent @@ -510,15 +515,14 @@ public void removeSubscription(ConnectionContext context, Subscription sub, long // while removing up a subscription. pagedInPendingDispatchLock.writeLock().lock(); try { - LOG.debug("{} remove sub: {}, lastDeliveredSeqId: {}, dequeues: {}, dispatched: {}, inflight: {}, groups: {}", new Object[]{ + LOG.debug("{} remove sub: {}, lastDeliveredSeqId: {}, dequeues: {}, dispatched: {}, inflight: {}, groups: {}", getActiveMQDestination().getQualifiedName(), sub, lastDeliveredSequenceId, getDestinationStatistics().getDequeues().getCount(), getDestinationStatistics().getDispatched().getCount(), getDestinationStatistics().getInflight().getCount(), - sub.getConsumerInfo().getAssignedGroupCount(destination) - }); + sub.getConsumerInfo().getAssignedGroupCount(destination)); consumersLock.writeLock().lock(); try { removeFromConsumerList(sub); @@ -1252,7 +1256,8 @@ private boolean shouldPageInMoreForBrowse(int max) { messagesLock.readLock().unlock(); } - LOG.trace("max {}, alreadyPagedIn {}, messagesCount {}, memoryUsage {}%", new Object[]{max, alreadyPagedIn, messagesInQueue, memoryUsage.getPercentUsage()}); + LOG.trace("max {}, alreadyPagedIn {}, messagesCount {}, memoryUsage {}%", max, alreadyPagedIn, messagesInQueue, + memoryUsage.getPercentUsage()); return (alreadyPagedIn == 0 || (alreadyPagedIn < max) && (alreadyPagedIn < messagesInQueue) && messages.hasSpace()); @@ -1971,7 +1976,7 @@ final void messageSent(final ConnectionContext context, final Message msg) throw }finally { consumersLock.readLock().unlock(); } - LOG.debug("{} Message {} sent to {}", new Object[]{ broker.getBrokerName(), msg.getMessageId(), this.destination }); + LOG.debug("{} Message {} sent to {}", broker.getBrokerName(), msg.getMessageId(), this.destination); wakeup(); } @@ -2042,19 +2047,18 @@ private PendingList doPageInForDispatch(boolean force, boolean processExpired, i } if (LOG.isDebugEnabled()) { - LOG.debug("{} toPageIn: {}, force:{}, Inflight: {}, pagedInMessages.size {}, pagedInPendingDispatch.size {}, enqueueCount: {}, dequeueCount: {}, memUsage:{}, maxPageSize:{}", - new Object[]{ - this, - toPageIn, - force, - destinationStatistics.getInflight().getCount(), - pagedInMessages.size(), - pagedInPendingSize, - destinationStatistics.getEnqueues().getCount(), - destinationStatistics.getDequeues().getCount(), - getMemoryUsage().getUsage(), - maxPageSize - }); + LOG.debug( + "{} toPageIn: {}, force:{}, Inflight: {}, pagedInMessages.size {}, pagedInPendingDispatch.size {}, enqueueCount: {}, dequeueCount: {}, memUsage:{}, maxPageSize:{}", + this, + toPageIn, + force, + destinationStatistics.getInflight().getCount(), + pagedInMessages.size(), + pagedInPendingSize, + destinationStatistics.getEnqueues().getCount(), + destinationStatistics.getDequeues().getCount(), + getMemoryUsage().getUsage(), + maxPageSize); } if (toPageIn > 0 && (force || (haveRealConsumer() && pagedInPendingSize < maxPageSize))) { From 27742034106716f47befe93ad15839de23b250f3 Mon Sep 17 00:00:00 2001 From: Felix Schumacher Date: Sat, 6 Oct 2018 14:08:05 +0200 Subject: [PATCH 3/6] More conversions from object array params to varargs when using logger --- .../apache/activemq/broker/BrokerService.java | 15 +++++++-------- .../activemq/broker/ProducerBrokerExchange.java | 15 ++++++--------- .../activemq/broker/TransportConnection.java | 2 +- .../policy/AbortSlowConsumerStrategy.java | 6 +++--- .../broker/scheduler/SchedulerBroker.java | 2 +- .../broker/util/LoggingBrokerPlugin.java | 2 +- .../activemq/broker/util/RedeliveryPlugin.java | 5 ++--- .../broker/util/TimeStampingBrokerPlugin.java | 2 +- .../ConditionalNetworkBridgeFilterFactory.java | 17 +++++++---------- .../apache/activemq/network/ConduitBridge.java | 15 ++++++--------- .../activemq/network/LdapNetworkConnector.java | 6 +++--- 11 files changed, 38 insertions(+), 49 deletions(-) diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java b/activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java index c959a6652d9..45855eb392c 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java @@ -743,7 +743,7 @@ private void doStartBroker() throws Exception { brokerId = broker.getBrokerId(); // need to log this after creating the broker so we have its id and name - LOG.info("Apache ActiveMQ {} ({}, {}) is starting", new Object[]{ getBrokerVersion(), getBrokerName(), brokerId }); + LOG.info("Apache ActiveMQ {} ({}, {}) is starting", getBrokerVersion(), getBrokerName(), brokerId); broker.start(); if (isUseJmx()) { @@ -770,7 +770,7 @@ private void doStartBroker() throws Exception { startAllConnectors(); - LOG.info("Apache ActiveMQ {} ({}, {}) started", new Object[]{ getBrokerVersion(), getBrokerName(), brokerId}); + LOG.info("Apache ActiveMQ {} ({}, {}) started", getBrokerVersion(), getBrokerName(), brokerId); LOG.info("For help or more information please see: http://activemq.apache.org"); getBroker().brokerServiceStarted(); @@ -833,7 +833,7 @@ public void run() { }.start(); } - LOG.info("Apache ActiveMQ {} ({}, {}) is shutting down", new Object[]{ getBrokerVersion(), getBrokerName(), brokerId} ); + LOG.info("Apache ActiveMQ {} ({}, {}) is shutting down", getBrokerVersion(), getBrokerName(), brokerId); removeShutdownHook(); if (this.scheduler != null) { @@ -893,9 +893,9 @@ public void run() { this.destinationFactory = null; if (startDate != null) { - LOG.info("Apache ActiveMQ {} ({}, {}) uptime {}", new Object[]{ getBrokerVersion(), getBrokerName(), brokerId, getUptime()}); + LOG.info("Apache ActiveMQ {} ({}, {}) uptime {}", getBrokerVersion(), getBrokerName(), brokerId, getUptime()); } - LOG.info("Apache ActiveMQ {} ({}, {}) is shutdown", new Object[]{ getBrokerVersion(), getBrokerName(), brokerId}); + LOG.info("Apache ActiveMQ {} ({}, {}) is shutdown", getBrokerVersion(), getBrokerName(), brokerId); synchronized (shutdownHooks) { for (Runnable hook : shutdownHooks) { @@ -957,9 +957,8 @@ public void stopGracefully(String connectorName, String queueName, long timeout, if (pollInterval <= 0) { pollInterval = 30; } - LOG.info("Stop gracefully with connectorName: {} queueName: {} timeout: {} pollInterval: {}", new Object[]{ - connectorName, queueName, timeout, pollInterval - }); + LOG.info("Stop gracefully with connectorName: {} queueName: {} timeout: {} pollInterval: {}", + connectorName, queueName, timeout, pollInterval); TransportConnector connector; for (int i = 0; i < transportConnectors.size(); i++) { connector = transportConnectors.get(i); diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/ProducerBrokerExchange.java b/activemq-broker/src/main/java/org/apache/activemq/broker/ProducerBrokerExchange.java index 99b766b4666..a216c440720 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/ProducerBrokerExchange.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/ProducerBrokerExchange.java @@ -144,20 +144,17 @@ public boolean canDispatch(Message messageSend) { long lastStoredForMessageProducer = getStoredSequenceIdForMessage(messageSend.getMessageId()); if (producerSequenceId <= lastStoredForMessageProducer) { canDispatch = false; - LOG.warn("suppressing duplicate message send [{}] from network producer with producerSequence [{}] less than last stored: {}", new Object[]{ - (LOG.isTraceEnabled() ? messageSend : messageSend.getMessageId()), producerSequenceId, lastStoredForMessageProducer - }); + LOG.warn("suppressing duplicate message send [{}] from network producer with producerSequence [{}] less than last stored: {}", + (LOG.isTraceEnabled() ? messageSend : messageSend.getMessageId()), producerSequenceId, lastStoredForMessageProducer); } } else if (producerSequenceId <= lastSendSequenceNumber.get()) { canDispatch = false; if (messageSend.isInTransaction()) { - LOG.warn("suppressing duplicated message send [{}] with producerSequenceId [{}] <= last stored: {}", new Object[]{ - (LOG.isTraceEnabled() ? messageSend : messageSend.getMessageId()), producerSequenceId, lastSendSequenceNumber - }); + LOG.warn("suppressing duplicated message send [{}] with producerSequenceId [{}] <= last stored: {}", + (LOG.isTraceEnabled() ? messageSend : messageSend.getMessageId()), producerSequenceId, lastSendSequenceNumber); } else { - LOG.debug("suppressing duplicated message send [{}] with producerSequenceId [{}] <= last stored: {}", new Object[]{ - (LOG.isTraceEnabled() ? messageSend : messageSend.getMessageId()), producerSequenceId, lastSendSequenceNumber - }); + LOG.debug("suppressing duplicated message send [{}] with producerSequenceId [{}] <= last stored: {}", + (LOG.isTraceEnabled() ? messageSend : messageSend.getMessageId()), producerSequenceId, lastSendSequenceNumber); } } else { diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnection.java b/activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnection.java index c064b183125..f4d9a0f0637 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnection.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnection.java @@ -814,7 +814,7 @@ public Response processAddConnection(ConnectionInfo info) throws Exception { } } registerConnectionState(info.getConnectionId(), state); - LOG.debug("Setting up new connection id: {}, address: {}, info: {}", new Object[]{ info.getConnectionId(), getRemoteAddress(), info }); + LOG.debug("Setting up new connection id: {}, address: {}, info: {}", info.getConnectionId(), getRemoteAddress(), info); this.faultTolerantConnection = info.isFaultTolerant(); // Setup the context. String clientId = info.getClientId(); diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/policy/AbortSlowConsumerStrategy.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/policy/AbortSlowConsumerStrategy.java index 62d583f5018..73cb9e82a42 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/policy/AbortSlowConsumerStrategy.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/policy/AbortSlowConsumerStrategy.java @@ -152,9 +152,9 @@ protected void abortSubscription(Map toAbort, b 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() }); + connection.getConnectionId(), + subscription.getConsumerInfo().getConsumerId(), + subscription.getActiveMQDestination()); } } diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/scheduler/SchedulerBroker.java b/activemq-broker/src/main/java/org/apache/activemq/broker/scheduler/SchedulerBroker.java index d8574160a5d..52ad7f9cf9f 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/scheduler/SchedulerBroker.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/scheduler/SchedulerBroker.java @@ -392,7 +392,7 @@ public void scheduledJob(String id, ByteSequence job) { messageSend.setExpiration(expiration); } messageSend.setTimestamp(newTimeStamp); - LOG.debug("Set message {} timestamp from {} to {}", new Object[]{ messageSend.getMessageId(), oldTimestamp, newTimeStamp }); + LOG.debug("Set message {} timestamp from {} to {}", messageSend.getMessageId(), oldTimestamp, newTimeStamp); } } diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/util/LoggingBrokerPlugin.java b/activemq-broker/src/main/java/org/apache/activemq/broker/util/LoggingBrokerPlugin.java index 4f93bcd434a..aacc32f0352 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/util/LoggingBrokerPlugin.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/util/LoggingBrokerPlugin.java @@ -156,7 +156,7 @@ public void acknowledge(ConsumerBrokerExchange consumerExchange, MessageAck ack) if (isLogAll() || isLogConsumerEvents()) { LOG.info("Acknowledging message for client ID: {}{}", consumerExchange.getConnectionContext().getClientId(), (ack.getMessageCount() == 1 ? ", " + ack.getLastMessageId() : "")); if (ack.getMessageCount() > 1) { - LOG.trace("Message count: {}, First Message Id: {}, Last Message Id: {}", new Object[]{ ack.getMessageCount(), ack.getFirstMessageId(), ack.getLastMessageId() }); + LOG.trace("Message count: {}, First Message Id: {}, Last Message Id: {}", ack.getMessageCount(), ack.getFirstMessageId(), ack.getLastMessageId()); } } super.acknowledge(consumerExchange, ack); diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/util/RedeliveryPlugin.java b/activemq-broker/src/main/java/org/apache/activemq/broker/util/RedeliveryPlugin.java index 8f66890eb18..9e3b1a6ca22 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/util/RedeliveryPlugin.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/util/RedeliveryPlugin.java @@ -172,9 +172,8 @@ public boolean sendToDeadLetterQueue(ConnectionContext context, MessageReference private void scheduleRedelivery(ConnectionContext context, MessageReference messageReference, long delay, int redeliveryCount) throws Exception { if (LOG.isTraceEnabled()) { Destination regionDestination = (Destination) messageReference.getRegionDestination(); - LOG.trace("redelivery #{} of: {} with delay: {}, dest: {}", new Object[]{ - redeliveryCount, messageReference.getMessageId(), delay, regionDestination.getActiveMQDestination() - }); + LOG.trace("redelivery #{} of: {} with delay: {}, dest: {}", + redeliveryCount, messageReference.getMessageId(), delay, regionDestination.getActiveMQDestination()); } final Message old = messageReference.getMessage(); Message message = old.copy(); diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/util/TimeStampingBrokerPlugin.java b/activemq-broker/src/main/java/org/apache/activemq/broker/util/TimeStampingBrokerPlugin.java index af6128f647d..b4bdf286a45 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/util/TimeStampingBrokerPlugin.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/util/TimeStampingBrokerPlugin.java @@ -125,7 +125,7 @@ public void send(ProducerBrokerExchange producerExchange, Message message) throw message.setExpiration(expiration); } message.setTimestamp(newTimeStamp); - LOG.debug("Set message {} timestamp from {} to {}", new Object[]{ message.getMessageId(), oldTimestamp, newTimeStamp }); + LOG.debug("Set message {} timestamp from {} to {}", message.getMessageId(), oldTimestamp, newTimeStamp); } } super.send(producerExchange, message); diff --git a/activemq-broker/src/main/java/org/apache/activemq/network/ConditionalNetworkBridgeFilterFactory.java b/activemq-broker/src/main/java/org/apache/activemq/network/ConditionalNetworkBridgeFilterFactory.java index 70c73430103..41be4088662 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/network/ConditionalNetworkBridgeFilterFactory.java +++ b/activemq-broker/src/main/java/org/apache/activemq/network/ConditionalNetworkBridgeFilterFactory.java @@ -120,7 +120,7 @@ protected boolean matchesForwardingFilter(Message message, final MessageEvaluati if (match) { LOG.trace("Replaying [{}] for [{}] back to origin in the absence of a local consumer", message.getMessageId(), message.getDestination()); } else { - LOG.trace("Suppressing replay of [{}] for [{}] back to origin {}", new Object[]{ message.getMessageId(), message.getDestination(), Arrays.asList(message.getBrokerPath())} ); + LOG.trace("Suppressing replay of [{}] for [{}] back to origin {}", message.getMessageId(), message.getDestination(), Arrays.asList(message.getBrokerPath())); } } else { @@ -129,9 +129,8 @@ protected boolean matchesForwardingFilter(Message message, final MessageEvaluati } if (match && rateLimitExceeded()) { - LOG.trace("Throttled network consumer rejecting [{}] for [{}] {}>{}/{}", new Object[]{ - message.getMessageId(), message.getDestination(), matchCount, rateLimit, rateDuration - }); + LOG.trace("Throttled network consumer rejecting [{}] for [{}] {}>{}/{}", + message.getMessageId(), message.getDestination(), matchCount, rateLimit, rateDuration); match = false; } @@ -149,17 +148,15 @@ private boolean hasNoLocalConsumers(final Message message, final MessageEvaluati if (!sub.getConsumerInfo().isNetworkSubscription() && !sub.getConsumerInfo().isBrowser()) { if (!isSelectorAware()) { - LOG.trace("Not replaying [{}] for [{}] to origin due to existing local consumer: {}", new Object[]{ - message.getMessageId(), message.getDestination(), sub.getConsumerInfo() - }); + LOG.trace("Not replaying [{}] for [{}] to origin due to existing local consumer: {}", + message.getMessageId(), message.getDestination(), sub.getConsumerInfo()); return false; } else { try { if (sub.matches(message, mec)) { - LOG.trace("Not replaying [{}] for [{}] to origin due to existing selector matching local consumer: {}", new Object[]{ - message.getMessageId(), message.getDestination(), sub.getConsumerInfo() - }); + LOG.trace("Not replaying [{}] for [{}] to origin due to existing selector matching local consumer: {}", + message.getMessageId(), message.getDestination(), sub.getConsumerInfo()); return false; } } catch (Exception ignored) {} diff --git a/activemq-broker/src/main/java/org/apache/activemq/network/ConduitBridge.java b/activemq-broker/src/main/java/org/apache/activemq/network/ConduitBridge.java index a4b5072a5c5..d08ae534b2f 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/network/ConduitBridge.java +++ b/activemq-broker/src/main/java/org/apache/activemq/network/ConduitBridge.java @@ -70,9 +70,8 @@ protected boolean addToAlreadyInterestedConsumers(ConsumerInfo info, boolean isF for (DemandSubscription ds : subscriptionMapByLocalId.values()) { DestinationFilter filter = DestinationFilter.parseFilter(ds.getLocalInfo().getDestination()); if (canConduit(ds) && filter.matches(info.getDestination())) { - LOG.debug("{} {} with ids {} matched (add interest) {}", new Object[]{ - configuration.getBrokerName(), info, info.getNetworkConsumerIds(), ds - }); + LOG.debug("{} {} with ids {} matched (add interest) {}", + configuration.getBrokerName(), info, info.getNetworkConsumerIds(), ds); // add the interest in the subscription if (!info.isDurable()) { ds.add(info.getConsumerId()); @@ -118,9 +117,8 @@ protected void removeDemandSubscription(ConsumerId id) throws IOException { for (DemandSubscription ds : subscriptionMapByLocalId.values()) { if (ds.remove(id)) { - LOG.debug("{} on {} from {} removed interest for: {} from {}", new Object[]{ - configuration.getBrokerName(), localBroker, remoteBrokerName, id, ds - }); + LOG.debug("{} on {} from {} removed interest for: {} from {}", + configuration.getBrokerName(), localBroker, remoteBrokerName, id, ds); } if (ds.isEmpty()) { tmpList.add(ds); @@ -129,9 +127,8 @@ protected void removeDemandSubscription(ConsumerId id) throws IOException { for (DemandSubscription ds : tmpList) { removeSubscription(ds); - LOG.debug("{} on {} from {} removed {}", new Object[]{ - configuration.getBrokerName(), localBroker, remoteBrokerName, ds - }); + LOG.debug("{} on {} from {} removed {}", + configuration.getBrokerName(), localBroker, remoteBrokerName, ds); } } } diff --git a/activemq-broker/src/main/java/org/apache/activemq/network/LdapNetworkConnector.java b/activemq-broker/src/main/java/org/apache/activemq/network/LdapNetworkConnector.java index 341ea30f9ba..39cbb8c4999 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/network/LdapNetworkConnector.java +++ b/activemq-broker/src/main/java/org/apache/activemq/network/LdapNetworkConnector.java @@ -297,7 +297,7 @@ protected synchronized void addConnector(SearchResult result) throws Exception { URI connectorURI = toURI(result); if (connectorMap.containsKey(connectorURI)) { int referenceCount = referenceMap.get(connectorURI) + 1; - LOG.warn("connector reference added for URI [{}], UUID [{}], total reference(s) [{}]", new Object[]{ connectorURI, uuid, referenceCount }); + LOG.warn("connector reference added for URI [{}], UUID [{}], total reference(s) [{}]", connectorURI, uuid, referenceCount); referenceMap.put(connectorURI, referenceCount); uuidMap.put(uuid, connectorURI); return; @@ -357,7 +357,7 @@ protected synchronized void removeConnector(SearchResult result) throws Exceptio int referenceCount = referenceMap.get(connectorURI) - 1; referenceMap.put(connectorURI, referenceCount); uuidMap.remove(uuid); - LOG.debug("connector referenced removed for URI [{}], UUID[{}], remaining reference(s) [{}]", new Object[]{ connectorURI, uuid, referenceCount }); + LOG.debug("connector referenced removed for URI [{}], UUID[{}], remaining reference(s) [{}]", connectorURI, uuid, referenceCount); if (referenceCount > 0) { return; @@ -434,7 +434,7 @@ public void objectRenamed(NamingEvent event) { String uuidNew = event.getNewBinding().getName(); URI connectorURI = uuidMap.remove(uuidOld); uuidMap.put(uuidNew, connectorURI); - LOG.debug("connector reference renamed for URI [{}], Old UUID [{}], New UUID [{}]", new Object[]{ connectorURI, uuidOld, uuidNew }); + LOG.debug("connector reference renamed for URI [{}], Old UUID [{}], New UUID [{}]", connectorURI, uuidOld, uuidNew); } /** From 380798c09631792b93e1e32ea010e85f7c41984e Mon Sep 17 00:00:00 2001 From: Felix Schumacher Date: Sat, 6 Oct 2018 14:20:22 +0200 Subject: [PATCH 4/6] Convert object array to varargs for log parameters and add a missing param The first log format string in this commit has more placeholder than parameters. I added one that I think is the correct one, but please check before committing this one. --- .../region/policy/AbortSlowAckConsumerStrategy.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/policy/AbortSlowAckConsumerStrategy.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/policy/AbortSlowAckConsumerStrategy.java index edb0cef9561..ae151fcf779 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/policy/AbortSlowAckConsumerStrategy.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/policy/AbortSlowAckConsumerStrategy.java @@ -163,16 +163,19 @@ private void abortAllQualifiedSlowConsumers() { if (getMaxSlowDuration() > 0 && (entry.getValue().markCount * getCheckPeriod() >= getMaxSlowDuration()) || getMaxSlowCount() > 0 && entry.getValue().slowCount >= getMaxSlowCount()) { - LOG.trace("Transferring consumer{} to the abort list: {} slow duration = {}, slow count = {}", - new Object[]{ entry.getKey().getConsumerInfo().getConsumerId(), + LOG.trace("Transferring consumer {} to the abort list: {} slow duration = {}, slow count = {}", + entry.getKey().getConsumerInfo().getConsumerId(), + toAbort, entry.getValue().markCount * getCheckPeriod(), - entry.getValue().getSlowCount() }); + entry.getValue().getSlowCount()); toAbort.put(entry.getKey(), entry.getValue()); slowConsumers.remove(entry.getKey()); } else { - LOG.trace("Not yet time to abort consumer {}: slow duration = {}, slow count = {}", new Object[]{ entry.getKey().getConsumerInfo().getConsumerId(), entry.getValue().markCount * getCheckPeriod(), entry.getValue().slowCount }); + LOG.trace("Not yet time to abort consumer {}: slow duration = {}, slow count = {}", + entry.getKey().getConsumerInfo().getConsumerId(), entry.getValue().markCount * getCheckPeriod(), + entry.getValue().slowCount); } } From 8f6f76b9082a8bcba05902f07e94f36cc7283b3d Mon Sep 17 00:00:00 2001 From: Felix Schumacher Date: Sat, 6 Oct 2018 14:38:34 +0200 Subject: [PATCH 5/6] More conversions from object array params to varargs when using logger --- .../activemq/broker/region/AbstractRegion.java | 6 +++--- .../broker/region/PrefetchSubscription.java | 8 ++++++-- .../activemq/broker/region/RegionBroker.java | 4 ++-- .../apache/activemq/broker/region/TempQueue.java | 3 ++- .../activemq/broker/region/TopicSubscription.java | 15 ++++++--------- .../region/cursors/FilePendingMessageCursor.java | 7 ++++--- .../cursors/StoreDurableSubscriberCursor.java | 5 ++++- .../policy/PriorityNetworkDispatchPolicy.java | 12 ++++++------ 8 files changed, 33 insertions(+), 27 deletions(-) diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/AbstractRegion.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/AbstractRegion.java index 8dcb76ca1c1..57307cf4935 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/AbstractRegion.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/AbstractRegion.java @@ -338,7 +338,7 @@ public Map getDestinationMap() { @Override @SuppressWarnings("unchecked") public Subscription addConsumer(ConnectionContext context, ConsumerInfo info) throws Exception { - LOG.debug("{} adding consumer: {} for destination: {}", new Object[]{ broker.getBrokerName(), info.getConsumerId(), info.getDestination() }); + LOG.debug("{} adding consumer: {} for destination: {}", broker.getBrokerName(), info.getConsumerId(), info.getDestination()); ActiveMQDestination destination = info.getDestination(); if (destination != null && !destination.isPattern() && !destination.isComposite()) { // lets auto-create the destination @@ -460,7 +460,7 @@ protected Set getInactiveDestinations() { @Override @SuppressWarnings("unchecked") public void removeConsumer(ConnectionContext context, ConsumerInfo info) throws Exception { - LOG.debug("{} removing consumer: {} for destination: {}", new Object[]{ broker.getBrokerName(), info.getConsumerId(), info.getDestination() }); + LOG.debug("{} removing consumer: {} for destination: {}", broker.getBrokerName(), info.getConsumerId(), info.getDestination()); Subscription sub = subscriptions.remove(info.getConsumerId()); // The sub could be removed elsewhere - see ConnectionSplitBroker @@ -685,7 +685,7 @@ public void processConsumerControl(ConsumerBrokerExchange consumerExchange, Cons entry.configurePrefetch(sub); } } - LOG.debug("setting prefetch: {}, on subscription: {}; resulting value: {}", new Object[]{ control.getPrefetch(), control.getConsumerId(), sub.getConsumerInfo().getPrefetchSize()}); + LOG.debug("setting prefetch: {}, on subscription: {}; resulting value: {}", control.getPrefetch(), control.getConsumerId(), sub.getConsumerInfo().getPrefetchSize()); try { lookup(consumerExchange.getConnectionContext(), control.getDestination(),false).wakeup(); } catch (Exception e) { diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/PrefetchSubscription.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/PrefetchSubscription.java index f4799238807..ca48cbcab2b 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/PrefetchSubscription.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/PrefetchSubscription.java @@ -738,7 +738,9 @@ public void onFailure() { if (node != QueueMessageReference.NULL_MESSAGE) { nodeDest.getDestinationStatistics().getDispatched().increment(); nodeDest.getDestinationStatistics().getInflight().increment(); - LOG.trace("{} failed to dispatch: {} - {}, dispatched: {}, inflight: {}", new Object[]{ info.getConsumerId(), message.getMessageId(), message.getDestination(), getSubscriptionStatistics().getDispatched().getCount(), dispatched.size() }); + LOG.trace("{} failed to dispatch: {} - {}, dispatched: {}, inflight: {}", + info.getConsumerId(), message.getMessageId(), message.getDestination(), + getSubscriptionStatistics().getDispatched().getCount(), dispatched.size()); } } if (node instanceof QueueMessageReference) { @@ -760,7 +762,9 @@ protected void onDispatch(final MessageReference node, final Message message) { if (node != QueueMessageReference.NULL_MESSAGE) { nodeDest.getDestinationStatistics().getDispatched().increment(); nodeDest.getDestinationStatistics().getInflight().increment(); - LOG.trace("{} dispatched: {} - {}, dispatched: {}, inflight: {}", new Object[]{ info.getConsumerId(), message.getMessageId(), message.getDestination(), getSubscriptionStatistics().getDispatched().getCount(), dispatched.size() }); + LOG.trace("{} dispatched: {} - {}, dispatched: {}, inflight: {}", info.getConsumerId(), + message.getMessageId(), message.getDestination(), + getSubscriptionStatistics().getDispatched().getCount(), dispatched.size()); } } diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/RegionBroker.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/RegionBroker.java index caf31f3b3c3..95c706d03db 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/RegionBroker.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/RegionBroker.java @@ -595,7 +595,7 @@ public synchronized void addBroker(Connection connection, BrokerInfo info) { brokerInfos.put(info.getBrokerId(), existing); } existing.incrementRefCount(); - LOG.debug("{} addBroker: {} brokerInfo size: {}", new Object[]{ getBrokerName(), info.getBrokerName(), brokerInfos.size() }); + LOG.debug("{} addBroker: {} brokerInfo size: {}", getBrokerName(), info.getBrokerName(), brokerInfos.size()); addBrokerInClusterUpdate(info); } @@ -606,7 +606,7 @@ public synchronized void removeBroker(Connection connection, BrokerInfo info) { if (existing != null && existing.decrementRefCount() == 0) { brokerInfos.remove(info.getBrokerId()); } - LOG.debug("{} removeBroker: {} brokerInfo size: {}", new Object[]{ getBrokerName(), info.getBrokerName(), brokerInfos.size()}); + LOG.debug("{} removeBroker: {} brokerInfo size: {}", getBrokerName(), info.getBrokerName(), brokerInfos.size()); // When stopping don't send cluster updates since we are the one's tearing down // our own bridges. if (!brokerService.isStopping()) { diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/TempQueue.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/TempQueue.java index 813827797df..90797da44dd 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/TempQueue.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/TempQueue.java @@ -84,7 +84,8 @@ public void addSubscription(ConnectionContext context, Subscription sub) throws @Override public void dispose(ConnectionContext context) throws IOException { if (this.destinationStatistics.getMessages().getCount() > 0) { - LOG.info("{} on dispose, purge of {} pending messages: {}", new Object[]{ getActiveMQDestination().getQualifiedName(), this.destinationStatistics.getMessages().getCount(), messages }); + LOG.info("{} on dispose, purge of {} pending messages: {}", getActiveMQDestination().getQualifiedName(), + this.destinationStatistics.getMessages().getCount(), messages); // we may want to capture these message ids in an advisory } try { diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/TopicSubscription.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/TopicSubscription.java index 0bf1c4e19e9..554a4ba6319 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/TopicSubscription.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/TopicSubscription.java @@ -134,12 +134,10 @@ public void add(MessageReference node) throws Exception { } if (!warnedAboutWait) { LOG.info("{}: Pending message cursor [{}] is full, temp usag ({}%) or memory usage ({}%) limit reached, blocking message add() pending the release of resources.", - new Object[]{ - toString(), - matched, - matched.getSystemUsage().getTempUsage().getPercentUsage(), - matched.getSystemUsage().getMemoryUsage().getPercentUsage() - }); + toString(), + matched, + matched.getSystemUsage().getTempUsage().getPercentUsage(), + matched.getSystemUsage().getMemoryUsage().getPercentUsage()); warnedAboutWait = true; } matchedListMutex.wait(20); @@ -185,9 +183,8 @@ public void add(MessageReference node) throws Exception { // lets avoid an infinite loop if we are given a bad eviction strategy // for a bad strategy lets just not evict if (messagesToEvict == 0) { - LOG.warn("No messages to evict returned for {} from eviction strategy: {} out of {} candidates", new Object[]{ - destination, messageEvictionStrategy, list.size() - }); + LOG.warn("No messages to evict returned for {} from eviction strategy: {} out of {} candidates", + destination, messageEvictionStrategy, list.size()); break; } } diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/FilePendingMessageCursor.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/FilePendingMessageCursor.java index f23d8172b47..ff93e50b8a3 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/FilePendingMessageCursor.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/FilePendingMessageCursor.java @@ -443,8 +443,8 @@ protected synchronized void flushToDisk() { long start = 0; if (LOG.isTraceEnabled()) { start = System.currentTimeMillis(); - LOG.trace("{}, flushToDisk() mem list size: {} {}", new Object[] { name, memoryList.size(), - (systemUsage != null ? systemUsage.getMemoryUsage() : "") }); + LOG.trace("{}, flushToDisk() mem list size: {} {}", name, memoryList.size(), + (systemUsage != null ? systemUsage.getMemoryUsage() : "")); } for (Iterator iterator = memoryList.iterator(); iterator.hasNext();) { MessageReference node = iterator.next(); @@ -461,7 +461,8 @@ protected synchronized void flushToDisk() { } memoryList.clear(); setCacheEnabled(false); - LOG.trace("{}, flushToDisk() done - {} ms {}", new Object[]{ name, (System.currentTimeMillis() - start), (systemUsage != null ? systemUsage.getMemoryUsage() : "") }); + LOG.trace("{}, flushToDisk() done - {} ms {}", name, (System.currentTimeMillis() - start), + (systemUsage != null ? systemUsage.getMemoryUsage() : "")); } } diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/StoreDurableSubscriberCursor.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/StoreDurableSubscriberCursor.java index 78645564d98..b701f6bbbde 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/StoreDurableSubscriberCursor.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/StoreDurableSubscriberCursor.java @@ -199,7 +199,10 @@ public synchronized boolean tryAddMessageLast(MessageReference node, long wait) if (prioritizedMessages && immediatePriorityDispatch && tsp.isPaging()) { if (msg.getPriority() > tsp.getLastRecoveredPriority()) { tsp.recoverMessage(node.getMessage(), true); - LOG.trace("cached high priority ({} message: {}, current paged batch priority: {}, cache size: {}", new Object[]{ msg.getPriority(), msg.getMessageId(), tsp.getLastRecoveredPriority(), tsp.batchList.size()}); + LOG.trace( + "cached high priority ({} message: {}, current paged batch priority: {}, cache size: {}", + msg.getPriority(), msg.getMessageId(), tsp.getLastRecoveredPriority(), + tsp.batchList.size()); } } } diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/policy/PriorityNetworkDispatchPolicy.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/policy/PriorityNetworkDispatchPolicy.java index a71825c3f90..f032958e0d1 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/policy/PriorityNetworkDispatchPolicy.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/policy/PriorityNetworkDispatchPolicy.java @@ -57,12 +57,12 @@ public boolean dispatch(MessageReference node, // higher priority matching sub exists highestPrioritySub = false; LOG.debug("ignoring lower priority: {} [{}, {}] in favour of: {} [{}, {}]", - new Object[]{ candidate, - candidate.getConsumerInfo().getNetworkConsumerIds(), - candidate.getConsumerInfo().getNetworkConsumerIds(), - sub, - sub.getConsumerInfo().getNetworkConsumerIds(), - sub.getConsumerInfo().getNetworkConsumerIds() }); + candidate, + candidate.getConsumerInfo().getNetworkConsumerIds(), + candidate.getConsumerInfo().getNetworkConsumerIds(), + sub, + sub.getConsumerInfo().getNetworkConsumerIds(), + sub.getConsumerInfo().getNetworkConsumerIds()); } } } From 4ce6bfe9d3ba990a39c1deb37103e58ad7eff2fa Mon Sep 17 00:00:00 2001 From: Felix Schumacher Date: Sat, 6 Oct 2018 14:38:52 +0200 Subject: [PATCH 6/6] Convert from object array params to varargs when using logger and rescue exception This was again a case, where the object array would put the exception parameter into a place where it would be taken as the second parameter and not used as intended. --- .../main/java/org/apache/activemq/broker/region/Topic.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/Topic.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/Topic.java index 66e586cd4d3..2837e90dbc1 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/Topic.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/Topic.java @@ -894,10 +894,8 @@ private void clearPendingAndDispatch(DurableTopicSubscription durableTopicSubscr try { durableTopicSubscription.dispatchPending(); } catch (IOException exception) { - LOG.warn("After clear of pending, failed to dispatch to: {}, for: {}, pending: {}", new Object[]{ - durableTopicSubscription, - destination, - durableTopicSubscription.pending }, exception); + LOG.warn("After clear of pending, failed to dispatch to: {}, for: {}, pending: {}", + durableTopicSubscription, destination, durableTopicSubscription.pending, exception); } } }