diff --git a/activemq-all/pom.xml b/activemq-all/pom.xml index 54031088465..fa111f42d68 100644 --- a/activemq-all/pom.xml +++ b/activemq-all/pom.xml @@ -14,7 +14,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-all diff --git a/activemq-amqp/pom.xml b/activemq-amqp/pom.xml index 287cdb4bdd7..82f8299094d 100644 --- a/activemq-amqp/pom.xml +++ b/activemq-amqp/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-amqp diff --git a/activemq-blueprint/pom.xml b/activemq-blueprint/pom.xml index bca01ca3f30..3d0939e6c46 100644 --- a/activemq-blueprint/pom.xml +++ b/activemq-blueprint/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-blueprint diff --git a/activemq-broker/pom.xml b/activemq-broker/pom.xml index 2c26ebc2500..5d65a728031 100644 --- a/activemq-broker/pom.xml +++ b/activemq-broker/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-broker diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/DurableTopicSubscription.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/DurableTopicSubscription.java index 13106cd0f34..bcdb4948136 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/DurableTopicSubscription.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/DurableTopicSubscription.java @@ -298,6 +298,16 @@ public void removePending(MessageReference node) throws IOException { pending.remove(node); } + @Override + protected void processExpiredAck(ConnectionContext context, Destination dest, + MessageReference node) { + + // Each subscription needs to expire both on the store and + // decrement the reference count + super.processExpiredAck(context, dest, node); + node.decrementReferenceCount(); + } + @Override protected void doAddRecoveredMessage(MessageReference message) throws Exception { synchronized (pending) { 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 93b3b2ae576..6bf17af6f4a 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 @@ -298,9 +298,8 @@ public final void acknowledge(final ConnectionContext context,final MessageAck a inAckRange = true; } if (inAckRange) { - Destination regionDestination = nodeDest; if (broker.isExpired(node)) { - regionDestination.messageExpired(context, this, node); + processExpiredAck(context, nodeDest, node); } iter.remove(); decrementPrefetchCounter(node); @@ -396,6 +395,11 @@ public final void acknowledge(final ConnectionContext context,final MessageAck a } } + protected void processExpiredAck(final ConnectionContext context, final Destination dest, + final MessageReference node) { + dest.messageExpired(context, this, node); + } + private void registerRemoveSync(ConnectionContext context, final MessageReference node) { // setup a Synchronization to remove nodes from the // dispatched list. 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 92e50c04bf4..e921c36dd48 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 @@ -17,10 +17,12 @@ package org.apache.activemq.broker.region; import java.io.IOException; +import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.concurrent.CancellationException; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -29,6 +31,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.stream.Collectors; import org.apache.activemq.advisory.AdvisorySupport; import org.apache.activemq.broker.BrokerService; import org.apache.activemq.broker.ConnectionContext; @@ -51,8 +54,8 @@ import org.apache.activemq.command.SubscriptionInfo; import org.apache.activemq.filter.MessageEvaluationContext; import org.apache.activemq.filter.NonCachedMessageEvaluationContext; -import org.apache.activemq.management.MessageFlowStats; import org.apache.activemq.store.MessageRecoveryListener; +import org.apache.activemq.store.MessageStore.StoreType; import org.apache.activemq.store.NoLocalSubscriptionAware; import org.apache.activemq.store.PersistenceAdapter; import org.apache.activemq.store.TopicMessageStore; @@ -702,6 +705,11 @@ public boolean isDuplicate(MessageId id) { for (DurableTopicSubscription sub : durableSubscribers.values()) { if (!sub.isActive() || sub.isEnableMessageExpirationOnActiveDurableSubs()) { message.setRegionDestination(this); + // AMQ-9721 - Remove message from the cursor if it exists after + // loading from the store. Store recoverExpired() does not inc + // the ref count so we don't need to decrement here, but if + // the cursor finds its own copy in memory it will dec that ref. + sub.removePending(message); messageExpired(connectionContext, sub, message); } } @@ -825,15 +833,99 @@ protected void dispatch(final ConnectionContext context, Message message) throws } } - private final AtomicBoolean expiryTaskInProgress = new AtomicBoolean(false); - private final Runnable expireMessagesWork = new Runnable() { + + /** + * Simple recovery listener that will check if the topic memory usage is full + * when hasSpace() is called. This could be enhanced in the future if needed. + */ + private final MessageRecoveryListener expiryListener = new MessageRecoveryListener() { + @Override - public void run() { - List browsedMessages = new InsertionCountList(); - doBrowse(browsedMessages, getMaxExpirePageSize()); + public boolean recoverMessage(Message message) { + return true; + } + + @Override + public boolean recoverMessageReference(MessageId ref) { + return true; + } + + @Override + public boolean hasSpace() { + return !Topic.this.memoryUsage.isFull(); + } + + @Override + public boolean isDuplicate(MessageId ref) { + return false; + } + }; + + private final AtomicBoolean expiryTaskInProgress = new AtomicBoolean(false); + private final Runnable expireMessagesWork = () -> { + try { + final TopicMessageStore store = Topic.this.topicStore; + if (store != null && store.getType() == StoreType.KAHADB) { + if (store.getMessageCount() == 0) { + LOG.debug("Skipping topic expiration check for {}, store size is 0", destination); + return; + } + + // get the sub keys that should be checked for expired messages + final var subs = durableSubscribers.entrySet().stream() + .filter(entry -> isEligibleForExpiration(entry.getValue())) + .map(Entry::getKey).collect(Collectors.toSet()); + + if (subs.isEmpty()) { + LOG.debug("Skipping topic expiration check for {}, no eligible subscriptions to check", destination); + return; + } + + // For each eligible subscription, return the messages in the store that are expired + // The same message refs are shared between subs if duplicated so this is efficient + var expired = store.recoverExpired(subs, getMaxExpirePageSize(), + expiryListener); + + final ConnectionContext connectionContext = createConnectionContext(); + // Go through any expired messages and remove for each sub + for (Entry> entry : expired.entrySet()) { + DurableTopicSubscription sub = durableSubscribers.get(entry.getKey()); + List expiredMessages = entry.getValue(); + + // If the sub still exists and there are expired messages then process + if (sub != null && !expiredMessages.isEmpty()) { + // There's a small race condition here if the sub comes online, + // but it's not a big deal as at worst there maybe be duplicate acks for + // the expired message but the store can handle it + if (isEligibleForExpiration(sub)) { + expiredMessages.forEach(message -> { + message.setRegionDestination(Topic.this); + try { + // AMQ-9721 - Remove message from the cursor if it exists after + // loading from the store. Store recoverExpired() does not inc + // the ref count so we don't need to decrement here, but if + // the cursor finds its own copy in memory it will dec that ref. + sub.removePending(message); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + messageExpired(connectionContext, sub, message); + }); + } + } + } + } else { + // If not KahaDB, fall back to the legacy browse method because + // the recoverExpired() method is not supported + doBrowse(new InsertionCountList<>(), getMaxExpirePageSize()); + } + } catch (Throwable e) { + LOG.warn("Failed to expire messages on Topic: {}", getActiveMQDestination().getPhysicalName(), e); + } finally { expiryTaskInProgress.set(false); } }; + private final Runnable expireMessagesTask = new Runnable() { @Override public void run() { @@ -855,9 +947,6 @@ public void messageExpired(ConnectionContext context, Subscription subs, Message ack.setDestination(destination); ack.setMessageID(reference.getMessageId()); try { - if (subs instanceof DurableTopicSubscription) { - ((DurableTopicSubscription)subs).removePending(reference); - } acknowledge(context, subs, ack, reference); } catch (Exception e) { LOG.error("Failed to remove expired Message from the store ", e); @@ -927,6 +1016,10 @@ private void clearPendingAndDispatch(DurableTopicSubscription durableTopicSubscr } } + private static boolean isEligibleForExpiration(DurableTopicSubscription sub) { + return sub.isEnableMessageExpirationOnActiveDurableSubs() || !sub.isActive(); + } + public Map getDurableTopicSubs() { return durableSubscribers; } 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 73a51137651..a6cb27f9273 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 @@ -188,6 +188,9 @@ public void add(MessageReference node) throws Exception { messagesToEvict = oldMessages.length; for (int i = 0; i < messagesToEvict; i++) { MessageReference oldMessage = oldMessages[i]; + // AMQ-9721 - discard no longer removes from matched so remove here + oldMessage.decrementReferenceCount(); + matched.remove(oldMessage); //Expired here is false as we are discarding due to the messageEvictingStrategy discard(oldMessage, false); } @@ -751,8 +754,6 @@ public void onFailure() { private void discard(MessageReference message, boolean expired) { discarding = true; try { - message.decrementReferenceCount(); - matched.remove(message); if (destination != null) { destination.getDestinationStatistics().getDequeues().increment(); if(destination.isAdvancedNetworkStatisticsEnabled() && getContext() != null && getContext().isNetworkConnection()) { diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/AbstractStoreCursor.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/AbstractStoreCursor.java index 30089e35515..8eec87e09fa 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/AbstractStoreCursor.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/AbstractStoreCursor.java @@ -114,7 +114,7 @@ public synchronized boolean recoverMessage(Message message, boolean cached) thro } } message.incrementReferenceCount(); - batchList.addMessageLast(message); + batchList.addMessageLast(createBatchListRef(message)); clearIterator(true); recovered = true; } else if (!cached) { @@ -136,6 +136,10 @@ public synchronized boolean recoverMessage(Message message, boolean cached) thro return recovered; } + protected MessageReference createBatchListRef(Message message) { + return message; + } + protected boolean duplicateFromStoreExcepted(Message message) { // expected for messages pending acks with kahadb.concurrentStoreAndDispatchQueues=true for // which this existing unused flag has been repurposed @@ -448,13 +452,15 @@ public final synchronized void remove() { @Override public final synchronized void remove(MessageReference node) { - if (batchList.remove(node) != null) { + final PendingNode message = batchList.remove(node); + if (message != null) { size--; setCacheEnabled(false); + // decrement reference count if removed from batchList + message.getMessage().decrementReferenceCount(); } } - @Override public final synchronized void clear() { gc(); 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 801208c7983..ed51ce8881d 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 @@ -162,8 +162,7 @@ public synchronized void release() { @Override public synchronized void destroy() throws Exception { stop(); - for (Iterator i = memoryList.iterator(); i.hasNext();) { - MessageReference node = i.next(); + for (MessageReference node : memoryList) { node.decrementReferenceCount(); } memoryList.clear(); @@ -365,11 +364,19 @@ public synchronized long messageSize() { */ @Override public synchronized void clear() { + // AMQ-9726 - Iterate over all nodes to decrement the ref count + // to decrement the memory usage tracker + for (MessageReference node : memoryList) { + node.decrementReferenceCount(); + } memoryList.clear(); if (!isDiskListEmpty()) { try { - getDiskList().destroy(); - } catch (IOException e) { + // AMQ-9726 - This method will destroy the list and + // set the reference to null so it will be reset + // for future writes + destroyDiskList(); + } catch (Exception e) { throw new RuntimeException(e); } } 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 55a77550e1e..510412b7944 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 @@ -28,6 +28,7 @@ import org.apache.activemq.broker.region.Destination; import org.apache.activemq.broker.region.DurableTopicSubscription; import org.apache.activemq.broker.region.MessageReference; +import org.apache.activemq.broker.region.NullMessageReference; import org.apache.activemq.broker.region.Topic; import org.apache.activemq.command.Message; import org.apache.activemq.usage.SystemUsage; @@ -274,8 +275,27 @@ public synchronized void remove() { @Override public synchronized void remove(MessageReference node) { - for (PendingMessageCursor tsp : storePrefetches) { - tsp.remove(node); + // AMQ-9721 - Check if message is persistent or non-persistent. + // Removing from the non-persistent cursor requires searching the + // entire list if it's paged onto disk which is quite slow, + // so it doesn't make sense to try and remove as it will never + // exist if it's persistent. + + // MessageReference can be a null reference if called from DurableSubscriptionView + // so we do not know if it's persistent and just need to search everything. + if (node instanceof NullMessageReference) { + for (PendingMessageCursor tsp : storePrefetches) { + tsp.remove(node); + } + } else if (node.isPersistent()) { + for (PendingMessageCursor tsp : storePrefetches) { + if (tsp.equals(nonPersistent)) { + continue; + } + tsp.remove(node); + } + } else { + nonPersistent.remove(node); } } diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/TopicStorePrefetch.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/TopicStorePrefetch.java index a97b1e8b504..6b75806cc18 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/TopicStorePrefetch.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/TopicStorePrefetch.java @@ -16,6 +16,8 @@ */ package org.apache.activemq.broker.region.cursors; +import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.activemq.broker.region.IndirectMessageReference; import org.apache.activemq.broker.region.MessageReference; import org.apache.activemq.broker.region.Subscription; import org.apache.activemq.broker.region.Topic; @@ -151,6 +153,11 @@ protected void doFillBatch() throws Exception { } } + @Override + protected MessageReference createBatchListRef(Message message) { + return new TopicStoreMessageReference(message); + } + public byte getLastRecoveredPriority() { return lastRecoveredPriority; } @@ -168,4 +175,24 @@ public Subscription getSubscription() { public String toString() { return "TopicStorePrefetch(" + clientId + "," + subscriberName + ",storeHasMessages=" + this.storeHasMessages +") " + this.subscription.getConsumerInfo().getConsumerId() + " - " + super.toString(); } + + // This extends IndirectMessageReference to allow expiring messages for multiple + // durable subscriptions. Each durable subscription needs to ack the message in the store so + // each durable sub will now get their own reference so that the subscription can expire + // correctly and not just the first subscription. + static class TopicStoreMessageReference extends IndirectMessageReference { + private final AtomicBoolean processAsExpired = new AtomicBoolean(false); + + public TopicStoreMessageReference(Message message) { + super(message); + } + + @Override + public boolean canProcessAsExpired() { + // mark original message ref as expired, this won't be used + // by this class but someone may get the original message and check it + super.canProcessAsExpired(); + return processAsExpired.compareAndSet(false, true); + } + } } diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/region/virtual/VirtualTopic.java b/activemq-broker/src/main/java/org/apache/activemq/broker/region/virtual/VirtualTopic.java index c57d80a16f4..7b6bd71ad27 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/region/virtual/VirtualTopic.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/region/virtual/VirtualTopic.java @@ -67,7 +67,7 @@ public ActiveMQDestination getMappedDestinations() { public Destination interceptMappedDestination(Destination destination) { // do a reverse map from destination to get actual virtual destination final String physicalName = destination.getActiveMQDestination().getPhysicalName(); - final Pattern pattern = Pattern.compile(getRegex(prefix) + "(.*)" + getRegex(postfix)); + final Pattern pattern = Pattern.compile(getRegex(prefix) + "(.+)" + getRegex(postfix)); final Matcher matcher = pattern.matcher(physicalName); if (matcher.matches()) { final String virtualName = matcher.group(1); 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 57afc85d11e..8d16445fb96 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 @@ -113,7 +113,7 @@ */ public abstract class DemandForwardingBridgeSupport implements NetworkBridge, BrokerServiceAware { private static final Logger LOG = LoggerFactory.getLogger(DemandForwardingBridgeSupport.class); - protected static final String DURABLE_SUB_PREFIX = "NC-DS_"; + protected static final String DURABLE_SUB_PREFIX = NetworkBridgeConfiguration.DURABLE_SUB_PREFIX; protected final Transport localBroker; protected final Transport remoteBroker; protected IdGenerator idGenerator = new IdGenerator(); @@ -664,23 +664,8 @@ public void run() { } } - /** - * Checks whether or not this consumer is a direct bridge network subscription - * @param info - * @return - */ - protected boolean isDirectBridgeConsumer(ConsumerInfo info) { - return (info.getSubscriptionName() != null && info.getSubscriptionName().startsWith(DURABLE_SUB_PREFIX)) && - (info.getClientId() == null || info.getClientId().startsWith(configuration.getName())); - } - protected boolean isProxyBridgeSubscription(String clientId, String subName) { - if (subName != null && clientId != null) { - if (subName.startsWith(DURABLE_SUB_PREFIX) && !clientId.startsWith(configuration.getName())) { - return true; - } - } - return false; + return NetworkBridgeUtils.isProxyBridgeSubscription(configuration, clientId, subName); } /** @@ -750,49 +735,61 @@ protected void serviceRemoteCommand(Command command) { } else if (command instanceof BrokerSubscriptionInfo) { final BrokerSubscriptionInfo brokerSubscriptionInfo = (BrokerSubscriptionInfo) command; - //Start in a new thread so we don't block the transport waiting for staticDestinations - syncExecutor.execute(new Runnable() { - - @Override - public void run() { - try { - staticDestinationsLatch.await(); - //Make sure after the countDown of staticDestinationsLatch we aren't stopping - if (!disposed.get()) { - BrokerSubscriptionInfo subInfo = brokerSubscriptionInfo; - LOG.debug("Received Remote BrokerSubscriptionInfo on {} from {}", - brokerService.getBrokerName(), subInfo.getBrokerName()); - - if (configuration.isSyncDurableSubs() && configuration.isConduitSubscriptions() - && !configuration.isDynamicOnly()) { - if (started.get()) { - if (subInfo.getSubscriptionInfos() != null) { - for (ConsumerInfo info : subInfo.getSubscriptionInfos()) { - //re-add any process any non-NC consumers that match the - //dynamicallyIncludedDestinations list - //Also re-add network consumers that are not part of this direct - //bridge (proxy of proxy bridges) - if((info.getSubscriptionName() == null || !isDirectBridgeConsumer(info)) && - NetworkBridgeUtils.matchesDestinations(dynamicallyIncludedDestinations, info.getDestination())) { - serviceRemoteConsumerAdvisory(info); - } - } - } + // Skip the durable sync if any of the following are true: + // 1) if the flag is set to false. + // 2) If dynamicOnly is true, this means means to only activate when the real + // consumers come back so we need to skip. This mode is useful espeically when + // setting TTL > 1 as the TTL info is tied to consumers + // 3) If conduit subscriptions is disable we also skip, for the same reason we + // skip when dynamicOnly is true, that we need to let consumers entirely drive + // the creation/removal of subscriptions as each consumer gets their own + if (!configuration.isSyncDurableSubs() || !configuration.isConduitSubscriptions() + || configuration.isDynamicOnly()) { + return; + } - //After re-added, clean up any empty durables - for (Iterator i = subscriptionMapByLocalId.values().iterator(); i.hasNext(); ) { - DemandSubscription ds = i.next(); - if (NetworkBridgeUtils.matchesDestinations(dynamicallyIncludedDestinations, ds.getLocalInfo().getDestination())) { - cleanupDurableSub(ds, i); - } - } + //Start in a new thread so we don't block the transport waiting for staticDestinations + syncExecutor.execute(() -> { + try { + staticDestinationsLatch.await(); + + //Make sure after the countDown of staticDestinationsLatch we aren't stopping + if (!disposed.get() && started.get()) { + final BrokerSubscriptionInfo subInfo = brokerSubscriptionInfo; + LOG.debug("Received Remote BrokerSubscriptionInfo on {} from {}", + brokerService.getBrokerName(), subInfo.getBrokerName()); + + // Go through and subs sent and see if we can add demand + if (subInfo.getSubscriptionInfos() != null) { + // Re-add and process subscriptions on the remote broker to add demand + for (ConsumerInfo info : subInfo.getSubscriptionInfos()) { + // Brokers filter what is sent, but the filtering logic has changed between + // versions, plus some durables sent are only processed for removes so we + // need to filter what to process for adding demand + if (NetworkBridgeUtils.matchesConfigForDurableSync(configuration, + info.getClientId(), info.getSubscriptionName(), info.getDestination())) { + serviceRemoteConsumerAdvisory(info); } } } - } catch (Exception e) { - LOG.warn("Error processing BrokerSubscriptionInfo: {}", e.getMessage(), e); - LOG.debug(e.getMessage(), e); + + //After processing demand to add, clean up any empty durables + for (Iterator i = subscriptionMapByLocalId.values().iterator(); i.hasNext(); ) { + DemandSubscription ds = i.next(); + // This filters on destinations to see if we should process possible removal + // based on the bridge configuration (included dests, TTL, etc). + if (NetworkBridgeUtils.matchesDestinations(configuration.getDynamicallyIncludedDestinations(), + ds.getLocalInfo().getDestination())) { + // Note that this method will further check that there are no remote + // demand that was previously added or associated. If there are remote + // subscriptions tied to the DS, then it will not be removed. + cleanupDurableSub(ds, i); + } + } } + } catch (Exception e) { + LOG.warn("Error processing BrokerSubscriptionInfo: {}", e.getMessage(), e); + LOG.debug(e.getMessage(), e); } }); @@ -1427,7 +1424,7 @@ protected void setupStaticDestinations() { if (dests != null) { for (ActiveMQDestination dest : dests) { if (isPermissableDestination(dest)) { - DemandSubscription sub = createDemandSubscription(dest, null); + DemandSubscription sub = createDemandSubscription(dest, null, null); if (sub != null) { sub.setStaticallyIncluded(true); try { @@ -1684,7 +1681,8 @@ protected DemandSubscription doCreateDemandSubscription(ConsumerInfo info) throw return result; } - final protected DemandSubscription createDemandSubscription(ActiveMQDestination destination, final String subscriptionName) { + final protected DemandSubscription createDemandSubscription(ActiveMQDestination destination, final String subscriptionName, + BrokerId[] brokerPath) { ConsumerInfo info = new ConsumerInfo(); info.setNetworkSubscription(true); info.setDestination(destination); @@ -1694,7 +1692,16 @@ final protected DemandSubscription createDemandSubscription(ActiveMQDestination } // Indicate that this subscription is being made on behalf of the remote broker. - info.setBrokerPath(new BrokerId[]{remoteBrokerId}); + // If we have existing brokerPath info then use it, this is important to + // preserve TTL information + if (brokerPath == null || brokerPath.length == 0) { + info.setBrokerPath(new BrokerId[]{remoteBrokerId}); + } else { + info.setBrokerPath(brokerPath); + if (!contains(brokerPath, remoteBrokerId)) { + addRemoteBrokerToBrokerPath(info); + } + } // the remote info held by the DemandSubscription holds the original // consumerId, the local info get's overwritten @@ -1778,7 +1785,7 @@ protected NetworkBridgeFilter createNetworkBridgeFilter(ConsumerInfo info) throw return filterFactory.create(info, getRemoteBrokerPath(), configuration.getMessageTTL(), configuration.getConsumerTTL()); } - protected void addRemoteBrokerToBrokerPath(ConsumerInfo info) throws IOException { + protected void addRemoteBrokerToBrokerPath(ConsumerInfo info) { info.setBrokerPath(appendToBrokerPath(info.getBrokerPath(), getRemoteBrokerPath())); } diff --git a/activemq-broker/src/main/java/org/apache/activemq/network/DurableConduitBridge.java b/activemq-broker/src/main/java/org/apache/activemq/network/DurableConduitBridge.java index 8d14f7492fe..9860bd13e15 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/network/DurableConduitBridge.java +++ b/activemq-broker/src/main/java/org/apache/activemq/network/DurableConduitBridge.java @@ -74,10 +74,14 @@ protected void setupStaticDestinations() { String candidateSubName = getSubscriberName(dest); for (Subscription subscription : topicRegion.getDurableSubscriptions().values()) { - String subName = subscription.getConsumerInfo().getSubscriptionName(); + ConsumerInfo subInfo = subscription.getConsumerInfo(); + String subName = subInfo.getSubscriptionName(); String clientId = subscription.getContext().getClientId(); if (subName != null && subName.equals(candidateSubName) && clientId.startsWith(configuration.getName())) { - DemandSubscription sub = createDemandSubscription(dest, subName); + // Include the brokerPath if it exists so that we can handle TTL more correctly + // This only works if the consumers are online as offline consumers are missing TTL + // For TTL > 1 configurations setting dynamicOnly to true may make more sense + DemandSubscription sub = createDemandSubscription(dest, subName, subInfo.getBrokerPath()); if (sub != null) { sub.getLocalInfo().setSubscriptionName(getSubscriberName(dest)); sub.setStaticallyIncluded(true); diff --git a/activemq-broker/src/main/java/org/apache/activemq/network/NetworkBridgeConfiguration.java b/activemq-broker/src/main/java/org/apache/activemq/network/NetworkBridgeConfiguration.java index 87ad022606b..911fb0bbe4e 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/network/NetworkBridgeConfiguration.java +++ b/activemq-broker/src/main/java/org/apache/activemq/network/NetworkBridgeConfiguration.java @@ -28,6 +28,7 @@ * Configuration for a NetworkBridge */ public class NetworkBridgeConfiguration { + public static final String DURABLE_SUB_PREFIX = "NC-DS_"; private boolean conduitSubscriptions = true; /** diff --git a/activemq-broker/src/main/java/org/apache/activemq/store/MessageStore.java b/activemq-broker/src/main/java/org/apache/activemq/store/MessageStore.java index aa0e85e0ec3..0339d72d986 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/store/MessageStore.java +++ b/activemq-broker/src/main/java/org/apache/activemq/store/MessageStore.java @@ -215,4 +215,12 @@ default void recoverMessages(final MessageRecoveryContext messageRecoveryContext void registerIndexListener(IndexListener indexListener); + StoreType getType(); + + enum StoreType { + MEMORY, JDBC, KAHADB, TEMP_KAHADB, + // deprecated, removed in 6.x + JOURNAL + } + } diff --git a/activemq-broker/src/main/java/org/apache/activemq/store/ProxyMessageStore.java b/activemq-broker/src/main/java/org/apache/activemq/store/ProxyMessageStore.java index d2f953aef5b..dfb369bd18f 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/store/ProxyMessageStore.java +++ b/activemq-broker/src/main/java/org/apache/activemq/store/ProxyMessageStore.java @@ -185,4 +185,8 @@ public MessageStoreStatistics getMessageStoreStatistics() { return delegate.getMessageStoreStatistics(); } + @Override + public StoreType getType() { + return delegate.getType(); + } } diff --git a/activemq-broker/src/main/java/org/apache/activemq/store/ProxyTopicMessageStore.java b/activemq-broker/src/main/java/org/apache/activemq/store/ProxyTopicMessageStore.java index 09b6529fa9e..d9b92500c01 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/store/ProxyTopicMessageStore.java +++ b/activemq-broker/src/main/java/org/apache/activemq/store/ProxyTopicMessageStore.java @@ -18,6 +18,9 @@ import java.io.IOException; +import java.util.List; +import java.util.Map; +import java.util.Set; import org.apache.activemq.broker.ConnectionContext; import org.apache.activemq.command.ActiveMQDestination; import org.apache.activemq.command.Message; @@ -25,6 +28,7 @@ import org.apache.activemq.command.MessageId; import org.apache.activemq.command.SubscriptionInfo; import org.apache.activemq.usage.MemoryUsage; +import org.apache.activemq.util.SubscriptionKey; /** * A simple proxy that delegates to another MessageStore. @@ -235,4 +239,10 @@ public long getMessageSize(String clientId, String subscriberName) public MessageStoreSubscriptionStatistics getMessageStoreSubStatistics() { return ((TopicMessageStore)delegate).getMessageStoreSubStatistics(); } + + @Override + public Map> recoverExpired(Set subs, int max, + MessageRecoveryListener listener) throws Exception { + return ((TopicMessageStore)delegate).recoverExpired(subs, max, listener); + } } diff --git a/activemq-broker/src/main/java/org/apache/activemq/store/TopicMessageStore.java b/activemq-broker/src/main/java/org/apache/activemq/store/TopicMessageStore.java index 95d5b72ed54..05f48539dc7 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/store/TopicMessageStore.java +++ b/activemq-broker/src/main/java/org/apache/activemq/store/TopicMessageStore.java @@ -20,10 +20,15 @@ import javax.jms.JMSException; +import java.util.List; +import java.util.Map; +import java.util.Set; import org.apache.activemq.broker.ConnectionContext; +import org.apache.activemq.command.Message; import org.apache.activemq.command.MessageAck; import org.apache.activemq.command.MessageId; import org.apache.activemq.command.SubscriptionInfo; +import org.apache.activemq.util.SubscriptionKey; /** * A MessageStore for durable topic subscriptions @@ -154,4 +159,23 @@ public interface TopicMessageStore extends MessageStore { * @throws IOException */ void addSubscription(SubscriptionInfo subscriptionInfo, boolean retroactive) throws IOException; + + /** + * Iterates over the pending messages in a topic and recovers any expired messages found for each + * of the subscriptions up to the maximum number of messages to search. Only subscriptions that + * have at least 1 expired message will be returned in the map. + *
+ * The expiry listener is only used to verify if there is space. Messages that are expired + * and will be added to 1 or more subscription in the returned map will be passed to + * the callback. The callback will only be called once per each unique message. + * + * @param subs The subscription keys to check for expired messages + * @param maxBrowse The maximum number of messages to check + * @param listener + * + * @return Expired messages for each subscription + */ + Map> recoverExpired(Set subs, int maxBrowse, + MessageRecoveryListener listener) throws Exception; + } diff --git a/activemq-broker/src/main/java/org/apache/activemq/store/memory/MemoryMessageStore.java b/activemq-broker/src/main/java/org/apache/activemq/store/memory/MemoryMessageStore.java index 7a0f69b04ea..3b3128649cc 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/store/memory/MemoryMessageStore.java +++ b/activemq-broker/src/main/java/org/apache/activemq/store/memory/MemoryMessageStore.java @@ -177,6 +177,11 @@ public void recoverMessageStoreStatistics() throws IOException { } } + @Override + public StoreType getType() { + return StoreType.MEMORY; + } + protected static final void incMessageStoreStatistics(final MessageStoreStatistics stats, final Message message) { if (stats != null && message != null) { stats.getMessageCount().increment(); diff --git a/activemq-broker/src/main/java/org/apache/activemq/store/memory/MemoryTopicMessageStore.java b/activemq-broker/src/main/java/org/apache/activemq/store/memory/MemoryTopicMessageStore.java index dd8be2be8ea..0aa6dc6ce07 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/store/memory/MemoryTopicMessageStore.java +++ b/activemq-broker/src/main/java/org/apache/activemq/store/memory/MemoryTopicMessageStore.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; +import java.util.Set; import org.apache.activemq.broker.ConnectionContext; import org.apache.activemq.command.ActiveMQDestination; import org.apache.activemq.command.Message; @@ -180,6 +181,12 @@ public void resetBatching(String clientId, String subscriptionName) { } } + @Override + public Map> recoverExpired(Set subs, int max, + MessageRecoveryListener listener) { + throw new UnsupportedOperationException("recoverExpired not supported"); + } + // Disabled for the memory store, can be enabled later if necessary private final MessageStoreSubscriptionStatistics stats = new MessageStoreSubscriptionStatistics(false); diff --git a/activemq-broker/src/main/java/org/apache/activemq/util/NetworkBridgeUtils.java b/activemq-broker/src/main/java/org/apache/activemq/util/NetworkBridgeUtils.java index 700baf678d7..4b39bb2bf7b 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/util/NetworkBridgeUtils.java +++ b/activemq-broker/src/main/java/org/apache/activemq/util/NetworkBridgeUtils.java @@ -20,7 +20,6 @@ import java.util.List; import java.util.Map; import java.util.Set; - import org.apache.activemq.advisory.AdvisoryBroker; import org.apache.activemq.advisory.AdvisorySupport; import org.apache.activemq.broker.BrokerService; @@ -36,6 +35,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.apache.activemq.network.NetworkBridgeConfiguration.DURABLE_SUB_PREFIX; + public class NetworkBridgeUtils { private static final Logger LOG = LoggerFactory.getLogger(NetworkBridgeUtils.class); @@ -54,31 +55,37 @@ public static BrokerSubscriptionInfo getBrokerSubscriptionInfo(final BrokerServi TopicRegion topicRegion = (TopicRegion) regionBroker.getTopicRegion(); Set subscriptionInfos = new HashSet<>(); - //Add all durable subscriptions to the set that match the network config - //which currently is just the dynamicallyIncludedDestinations list - for (SubscriptionKey key : topicRegion.getDurableSubscriptions().keySet()) { - DurableTopicSubscription sub = topicRegion.getDurableSubscriptions().get(key); - if (sub != null && NetworkBridgeUtils.matchesNetworkConfig(config, sub.getConsumerInfo().getDestination())) { + // Add all durable subscriptions to the set that match the network config + // which currently is just the dynamicallyIncludedDestinations list + for (Map.Entry entry : topicRegion.getDurableSubscriptions().entrySet()) { + final SubscriptionKey key = entry.getKey(); + final DurableTopicSubscription sub = entry.getValue(); + // We must use the key for matchesConfigForDurableSync() because the clientId for the ConsumerInfo object + // may be null if the subscription is offline, which is why we copy the ConsumerInfo below + // and set the clientId to match the key. The correct clientId is important for the receiving broker + // to do proper filtering when TTL is set + if (sub != null && NetworkBridgeUtils.matchesConfigForDurableSync(config, key.getClientId(), + key.getSubscriptionName(), sub.getActiveMQDestination())) { ConsumerInfo ci = sub.getConsumerInfo().copy(); ci.setClientId(key.getClientId()); subscriptionInfos.add(ci); } } - //We also need to iterate over all normal subscriptions and check if they are part of - //any dynamicallyIncludedDestination that is configured with forceDurable to be true - //over the network bridge. If forceDurable is true then we want to add the consumer to the set + // We also need to iterate over all normal subscriptions and check if they are part of + // any dynamicallyIncludedDestination that is configured with forceDurable to be true + // over the network bridge. If forceDurable is true then we want to add the consumer to the set for (Subscription sub : topicRegion.getSubscriptions().values()) { if (sub != null && NetworkBridgeUtils.isForcedDurable(sub.getConsumerInfo(), - config.getDynamicallyIncludedDestinations())) { + config.getDynamicallyIncludedDestinations())) { subscriptionInfos.add(sub.getConsumerInfo().copy()); } } try { - //Lastly, if isUseVirtualDestSubs is configured on this broker (to fire advisories) and - //configured on the network connector (to listen to advisories) then also add any virtual - //dest subscription to the set if forceDurable is true for its destination + // Lastly, if isUseVirtualDestSubs is configured on this broker (to fire advisories) and + // configured on the network connector (to listen to advisories) then also add any virtual + // dest subscription to the set if forceDurable is true for its destination AdvisoryBroker ab = (AdvisoryBroker) brokerService.getBroker().getAdaptor(AdvisoryBroker.class); if (ab != null && brokerService.isUseVirtualDestSubs() && config.isUseVirtualDestSubs()) { for (ConsumerInfo info : ab.getVirtualDestinationConsumers().keySet()) { @@ -97,10 +104,9 @@ public static BrokerSubscriptionInfo getBrokerSubscriptionInfo(final BrokerServi } public static boolean isForcedDurable(final ConsumerInfo info, - final List dynamicallyIncludedDestinations) { - return dynamicallyIncludedDestinations != null - ? isForcedDurable(info, - dynamicallyIncludedDestinations.toArray(new ActiveMQDestination[0]), null) : false; + final List dynamicallyIncludedDestinations) { + return dynamicallyIncludedDestinations != null && isForcedDurable(info, + dynamicallyIncludedDestinations.toArray(new ActiveMQDestination[0]), null); } public static boolean isForcedDurable(final ConsumerInfo info, @@ -113,7 +119,7 @@ public static boolean isForcedDurable(final ConsumerInfo info, ActiveMQDestination destination = info.getDestination(); if (AdvisorySupport.isAdvisoryTopic(destination) || destination.isTemporary() || - destination.isQueue()) { + destination.isQueue()) { return false; } @@ -128,44 +134,71 @@ public static boolean isForcedDurable(final ConsumerInfo info, return false; } - public static boolean matchesNetworkConfig(final NetworkBridgeConfiguration config, - ActiveMQDestination destination) { - List includedDests = config.getDynamicallyIncludedDestinations(); - if (includedDests != null && includedDests.size() > 0) { - for (ActiveMQDestination dest : includedDests) { - DestinationFilter inclusionFilter = DestinationFilter.parseFilter(dest); - if (dest != null && inclusionFilter.matches(destination) && dest.getDestinationType() == destination.getDestinationType()) { - return true; - } - } + /** + * This method is used to determine which durable subscriptions should be sent from + * a broker to a remote broker so that the remote broker can process the subscriptions + * to re-add demand when the bridge is first started during the durable sync phase + * of a bridge starting. We can cut down on the amount of durables sent/processed + * based on how the bridge is configured. + * + * @param config + * @param clientId + * @param subscriptionName + * @param destination + * @return + */ + public static boolean matchesConfigForDurableSync(final NetworkBridgeConfiguration config, + String clientId, String subscriptionName, ActiveMQDestination destination) { + + // If consumerTTL was set to 0 then we return false because no demand will be + // generated over the bridge as the messages will be limited to the local broker + if (config.getConsumerTTL() == 0) { + return false; } - return false; + // If this is a remote demand consumer for the current bridge we can also skip + // This consumer was created by another consumer on the remote broker, so we + // ignore this consumer for demand, or else we'd end up with a loop + if (isDirectBridgeConsumer(config, clientId, subscriptionName)) { + return false; + } + + // if TTL is set to 1 then we won't ever handle proxy durables as they + // are at least 2 hops away so the TTL check would always fail. Proxy durables + // are subs for other bridges, so we can skip these as well. + if (config.getConsumerTTL() == 1 && isProxyBridgeSubscription(config, clientId, + subscriptionName)) { + return false; + } + + // Verify the destination matches the dynamically included destination list + return matchesDestinations(config.getDynamicallyIncludedDestinations(), destination); } - public static boolean matchesDestinations(ActiveMQDestination[] dests, final ActiveMQDestination destination) { - if (dests != null && dests.length > 0) { - for (ActiveMQDestination dest : dests) { - DestinationFilter inclusionFilter = DestinationFilter.parseFilter(dest); - if (dest != null && inclusionFilter.matches(destination) && dest.getDestinationType() == destination.getDestinationType()) { + public static boolean matchesDestination(ActiveMQDestination destFilter, ActiveMQDestination destToMatch) { + DestinationFilter inclusionFilter = DestinationFilter.parseFilter(destFilter); + return inclusionFilter.matches(destToMatch) && destFilter.getDestinationType() == destToMatch.getDestinationType(); + } + + public static boolean matchesDestinations(final List includedDests, final ActiveMQDestination destination) { + if (includedDests != null && !includedDests.isEmpty()) { + for (ActiveMQDestination dest : includedDests) { + if (matchesDestination(dest, destination)) { return true; } } } - return false; } public static ActiveMQDestination findMatchingDestination(ActiveMQDestination[] dests, ActiveMQDestination destination) { - if (dests != null && dests.length > 0) { + if (dests != null) { for (ActiveMQDestination dest : dests) { - DestinationFilter inclusionFilter = DestinationFilter.parseFilter(dest); - if (dest != null && inclusionFilter.matches(destination) && dest.getDestinationType() == destination.getDestinationType()) { + if (matchesDestination(dest, destination)) { return dest; } } } - return null; } @@ -181,4 +214,16 @@ public static boolean isDestForcedDurable(final ActiveMQDestination destination) return isForceDurable; } + + public static boolean isDirectBridgeConsumer(NetworkBridgeConfiguration config, String clientId, String subName) { + return (subName != null && subName.startsWith(DURABLE_SUB_PREFIX)) && + (clientId == null || clientId.startsWith(config.getName())); + } + + public static boolean isProxyBridgeSubscription(NetworkBridgeConfiguration config, String clientId, String subName) { + if (subName != null && clientId != null) { + return subName.startsWith(DURABLE_SUB_PREFIX) && !clientId.startsWith(config.getName()); + } + return false; + } } diff --git a/activemq-cf/pom.xml b/activemq-cf/pom.xml index b36ba577e92..d9ab3353a6b 100644 --- a/activemq-cf/pom.xml +++ b/activemq-cf/pom.xml @@ -24,7 +24,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-cf diff --git a/activemq-client-jakarta/pom.xml b/activemq-client-jakarta/pom.xml index 30f319e5033..50c7872da1c 100644 --- a/activemq-client-jakarta/pom.xml +++ b/activemq-client-jakarta/pom.xml @@ -20,7 +20,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-client-jakarta bundle diff --git a/activemq-client/pom.xml b/activemq-client/pom.xml index 506a0164817..f16060d4f41 100644 --- a/activemq-client/pom.xml +++ b/activemq-client/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-client diff --git a/activemq-console/pom.xml b/activemq-console/pom.xml index 4399838b377..3c4ebf431b0 100644 --- a/activemq-console/pom.xml +++ b/activemq-console/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-console diff --git a/activemq-http/pom.xml b/activemq-http/pom.xml index c3d817f4b76..ea61afc49bd 100644 --- a/activemq-http/pom.xml +++ b/activemq-http/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-http diff --git a/activemq-jaas/pom.xml b/activemq-jaas/pom.xml index b23622b46f1..d0ff96c0288 100644 --- a/activemq-jaas/pom.xml +++ b/activemq-jaas/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-jaas diff --git a/activemq-jdbc-store/pom.xml b/activemq-jdbc-store/pom.xml index 1c21f936058..3109d288d48 100644 --- a/activemq-jdbc-store/pom.xml +++ b/activemq-jdbc-store/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-jdbc-store diff --git a/activemq-jdbc-store/src/main/java/org/apache/activemq/store/jdbc/JDBCMessageStore.java b/activemq-jdbc-store/src/main/java/org/apache/activemq/store/jdbc/JDBCMessageStore.java index 8adc2f78ee9..78c27b71839 100644 --- a/activemq-jdbc-store/src/main/java/org/apache/activemq/store/jdbc/JDBCMessageStore.java +++ b/activemq-jdbc-store/src/main/java/org/apache/activemq/store/jdbc/JDBCMessageStore.java @@ -482,4 +482,10 @@ public String toString() { return destination.getPhysicalName() + ",pendingSize:" + pendingAdditions.size(); } + + @Override + public StoreType getType() { + return StoreType.JDBC; + } + } diff --git a/activemq-jdbc-store/src/main/java/org/apache/activemq/store/jdbc/JDBCTopicMessageStore.java b/activemq-jdbc-store/src/main/java/org/apache/activemq/store/jdbc/JDBCTopicMessageStore.java index a857bcf4ef7..26390ba3089 100644 --- a/activemq-jdbc-store/src/main/java/org/apache/activemq/store/jdbc/JDBCTopicMessageStore.java +++ b/activemq-jdbc-store/src/main/java/org/apache/activemq/store/jdbc/JDBCTopicMessageStore.java @@ -22,6 +22,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -40,6 +41,7 @@ import org.apache.activemq.store.TopicMessageStore; import org.apache.activemq.util.ByteSequence; import org.apache.activemq.util.IOExceptionSupport; +import org.apache.activemq.util.SubscriptionKey; import org.apache.activemq.wireformat.WireFormat; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -360,6 +362,12 @@ public void addSubscription(SubscriptionInfo subscriptionInfo, boolean retroacti } } + @Override + public Map> recoverExpired(Set subs, int max, + MessageRecoveryListener listener) { + throw new UnsupportedOperationException("recoverExpired not supported"); + } + /** * @see org.apache.activemq.store.TopicMessageStore#lookupSubscription(String, * String) diff --git a/activemq-jdbc-store/src/main/java/org/apache/activemq/store/journal/JournalMessageStore.java b/activemq-jdbc-store/src/main/java/org/apache/activemq/store/journal/JournalMessageStore.java index 7ec10c4d9a6..b1f6ba5576c 100644 --- a/activemq-jdbc-store/src/main/java/org/apache/activemq/store/journal/JournalMessageStore.java +++ b/activemq-jdbc-store/src/main/java/org/apache/activemq/store/journal/JournalMessageStore.java @@ -16,6 +16,8 @@ */ package org.apache.activemq.store.journal; +import static org.apache.activemq.store.MessageStore.StoreType.JOURNAL; + import java.io.IOException; import java.util.ArrayList; import java.util.Collections; @@ -426,4 +428,9 @@ public void setBatch(MessageId messageId) throws Exception { longTermStore.setBatch(messageId); } + @Override + public StoreType getType() { + return JOURNAL; + } + } diff --git a/activemq-jdbc-store/src/main/java/org/apache/activemq/store/journal/JournalTopicMessageStore.java b/activemq-jdbc-store/src/main/java/org/apache/activemq/store/journal/JournalTopicMessageStore.java index 7aa61a269c2..1613caab503 100644 --- a/activemq-jdbc-store/src/main/java/org/apache/activemq/store/journal/JournalTopicMessageStore.java +++ b/activemq-jdbc-store/src/main/java/org/apache/activemq/store/journal/JournalTopicMessageStore.java @@ -20,6 +20,9 @@ import java.util.HashMap; import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Set; import org.apache.activeio.journal.RecordLocation; import org.apache.activemq.broker.ConnectionContext; import org.apache.activemq.command.ActiveMQTopic; @@ -81,6 +84,12 @@ public void addSubscription(SubscriptionInfo subscriptionInfo, boolean retroacti longTermStore.addSubscription(subscriptionInfo, retroactive); } + @Override + public Map> recoverExpired(Set subs, int max, + MessageRecoveryListener listener) { + throw new UnsupportedOperationException("recoverExpired not supported"); + } + @Override public void addMessage(ConnectionContext context, Message message) throws IOException { super.addMessage(context, message); diff --git a/activemq-jms-pool/pom.xml b/activemq-jms-pool/pom.xml index 402e202d7e3..7778769c127 100644 --- a/activemq-jms-pool/pom.xml +++ b/activemq-jms-pool/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-jms-pool diff --git a/activemq-kahadb-store/pom.xml b/activemq-kahadb-store/pom.xml index 39982070b18..c32a37b5327 100644 --- a/activemq-kahadb-store/pom.xml +++ b/activemq-kahadb-store/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-kahadb-store diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/KahaDBStore.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/KahaDBStore.java index b987eea6d13..63273bcce72 100644 --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/KahaDBStore.java +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/KahaDBStore.java @@ -83,12 +83,14 @@ import org.apache.activemq.store.kahadb.data.KahaUpdateMessageCommand; import org.apache.activemq.store.kahadb.disk.journal.Location; import org.apache.activemq.store.kahadb.disk.page.Transaction; +import org.apache.activemq.store.kahadb.disk.page.Transaction.CallableClosure; import org.apache.activemq.store.kahadb.disk.util.SequenceSet; import org.apache.activemq.store.kahadb.scheduler.JobSchedulerStoreImpl; import org.apache.activemq.usage.MemoryUsage; import org.apache.activemq.usage.SystemUsage; import org.apache.activemq.util.IOExceptionSupport; import org.apache.activemq.util.ServiceStopper; +import org.apache.activemq.util.SubscriptionKey; import org.apache.activemq.util.ThreadPoolUtils; import org.apache.activemq.wireformat.WireFormat; import org.slf4j.Logger; @@ -752,6 +754,25 @@ public void recoverMessages(final MessageRecoveryContext messageRecoveryContext) public void execute(Transaction tx) throws Exception { StoredDestination sd = getStoredDestination(dest, tx); + /** + * [AMQ-9773] + * + * The order index sequence key value increases for every message since the beginning of the + * creation of the index, so the first message available won't be at sequence key value:0 in + * the index when there were older messages acknowledged. + * + * The MessageOrderCursor _position_ value is relative to the index, so there is a disconnect + * between queue _position_ and index sequence value over time. + * + * The MessageRecoveryContext determines the recovery start position based off the provided + * offset, or the position of the requested startMessageId. If a startMessageId is specified, + * but not found in the index, then the value of 0 is used as a fallback. + * + * The MessageRecoveryContext determines the recovery end position based off of the provided + * endMessageId (if provided), or the maximum recovered message count, or if the + * MessageRecoveryListener signals that no more messages should be recovered + * (ie memory is full). + */ Long startSequenceOffset = null; Long endSequenceOffset = null; @@ -764,16 +785,15 @@ public void execute(Transaction tx) throws Exception { if(messageRecoveryContext.getEndMessageId() != null && !messageRecoveryContext.getEndMessageId().isBlank()) { endSequenceOffset = Optional.ofNullable(sd.messageIdIndex.get(tx, messageRecoveryContext.getEndMessageId())) .orElse(startSequenceOffset + Long.valueOf(messageRecoveryContext.getMaxMessageCountReturned())); - } else { - endSequenceOffset = startSequenceOffset + Long.valueOf(messageRecoveryContext.getMaxMessageCountReturned()); + messageRecoveryContext.setEndSequenceId(endSequenceOffset); } - if(endSequenceOffset < startSequenceOffset) { + if(endSequenceOffset != null && + endSequenceOffset < startSequenceOffset) { LOG.warn("Invalid offset parameters start:{} end:{}", startSequenceOffset, endSequenceOffset); throw new IllegalStateException("Invalid offset parameters start:" + startSequenceOffset + " end:" + endSequenceOffset); } - messageRecoveryContext.setEndSequenceId(endSequenceOffset); Entry entry = null; recoverRolledBackAcks(destination.getPhysicalName(), sd, tx, messageRecoveryContext.getMaxMessageCountReturned(), messageRecoveryContext); Set ackedAndPrepared = ackedAndPreparedMap.get(destination.getPhysicalName()); @@ -794,7 +814,11 @@ public void execute(Transaction tx) throws Exception { break; } } - sd.orderIndex.stoppedIterating(); + + // [AMQ-9773] The sd.orderIndex uses the destination's cursor + if(!messageRecoveryContext.isUseDedicatedCursor()) { + sd.orderIndex.stoppedIterating(); + } } }); } finally { @@ -958,9 +982,14 @@ public MessageStoreStatistics execute(Transaction tx) throws IOException { unlockAsyncJobQueue(); } } + + @Override + public StoreType getType() { + return StoreType.KAHADB; + } } - class KahaDBTopicMessageStore extends KahaDBMessageStore implements TopicMessageStore { + class KahaDBTopicMessageStore extends KahaDBMessageStore implements TopicMessageStore{ private final AtomicInteger subscriptionCount = new AtomicInteger(); protected final MessageStoreSubscriptionStatistics messageStoreSubStats = new MessageStoreSubscriptionStatistics(isEnableSubscriptionStatistics()); @@ -1323,6 +1352,64 @@ public void execute(Transaction tx) throws Exception { } } + @Override + public Map> recoverExpired(Set subscriptions, int maxBrowse, + MessageRecoveryListener listener) throws Exception { + indexLock.writeLock().lock(); + try { + return pageFile.tx().execute(tx -> { + StoredDestination sd = getStoredDestination(dest, tx); + sd.orderIndex.resetCursorPosition(); + int count = 0; + final Map> expired = new HashMap<>(); + final Map subKeys = new HashMap<>(); + + // Check each subscription and track the ones that exist + for (SubscriptionKey sub : subscriptions) { + final String subKeyString = subscriptionKey(sub.getClientId(), sub.getSubscriptionName()); + if (sd.subscriptionCache.contains(subKeyString)) { + subKeys.put(subKeyString, sub); + } + } + + // Iterate one time through the topic and check each message, stopping if we run out + // hit the max browse limit, or if the listener returns false for hasSpace() + final Set uniqueExpired = new HashSet<>(); + for (Iterator> iterator = + sd.orderIndex.iterator(tx, new MessageOrderCursor()); count < maxBrowse && iterator.hasNext() && listener.hasSpace(); ) { + count++; + Entry entry = iterator.next(); + Set ackedAndPrepared = ackedAndPreparedMap.get(destination.getPhysicalName()); + if (ackedAndPrepared != null && ackedAndPrepared.contains(entry.getValue().messageId)) { + continue; + } + + final Message msg = loadMessage(entry.getValue().location); + if (msg.isExpired()) { + // For every message that is expired, go through and check each subscription to see + // if the message has already been acked. We don't want to return subs that have already + // acked the message. + for(Entry subKeyEntry : subKeys.entrySet()) { + SequenceSet sequence = sd.ackPositions.get(tx, subKeyEntry.getKey()); + if (sequence != null && sequence.contains(entry.getKey())) { + List expMessages = expired.computeIfAbsent(subKeyEntry.getValue(), m -> new ArrayList<>()); + expMessages.add(msg); + // pass unique messages to the listener so it can do any custom + // handling for the next call to listener.hasSpace() + if (uniqueExpired.add(entry.getKey())) { + listener.recoverMessage(msg); + } + } + } + } + } + return expired; + }); + } finally { + indexLock.writeLock().unlock(); + } + } + @Override public void resetBatching(String clientId, String subscriptionName) { try { diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/MessageDatabase.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/MessageDatabase.java index 00b9bce25b1..33fead27494 100644 --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/MessageDatabase.java +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/MessageDatabase.java @@ -448,10 +448,10 @@ public void run() { } } catch (IOException ioe) { LOG.error("Checkpoint failed", ioe); - brokerService.handleIOException(ioe); + handleIOException("CheckpointRunner", ioe); } catch (Throwable e) { LOG.error("Checkpoint failed", e); - brokerService.handleIOException(IOExceptionSupport.create(e)); + handleIOException("CheckpointRunner", IOExceptionSupport.create(e)); } } } @@ -1967,7 +1967,7 @@ public void visit(List keys, List values) { final StoredDestination destination = entry.getValue(); final String subscriptionKey = subscription.getKey(); final SequenceSet pendingAcks = destination.ackPositions.get(tx, subscriptionKey); - LOG.trace("sub {} on {} in dataFile {} has pendingCount {}", subscriptionKey, entry.getKey(), dataFileId, pendingAcks.rangeSize()-1); + LOG.trace("sub {} on {} in dataFile {} has pendingCount {}", subscriptionKey, entry.getKey(), dataFileId, (pendingAcks != null ? pendingAcks.rangeSize()-1 : 0)); } gcCandidateSet.remove(dataFileId); } @@ -2120,10 +2120,10 @@ public void run() { forwarded = true; } catch (IOException ioe) { LOG.error("Forwarding of acks failed", ioe); - brokerService.handleIOException(ioe); + handleIOException("AckCompactionRunner", ioe); } catch (Throwable e) { LOG.error("Forwarding of acks failed", e); - brokerService.handleIOException(IOExceptionSupport.create(e)); + handleIOException("AckCompactionRunner", IOExceptionSupport.create(e)); } } finally { checkpointLock.readLock().unlock(); @@ -2136,10 +2136,10 @@ public void run() { } } catch (IOException ioe) { LOG.error("Checkpoint failed", ioe); - brokerService.handleIOException(ioe); + handleIOException("AckCompactionRunner", ioe); } catch (Throwable e) { LOG.error("Checkpoint failed", e); - brokerService.handleIOException(IOExceptionSupport.create(e)); + handleIOException("AckCompactionRunner", IOExceptionSupport.create(e)); } } } @@ -4274,4 +4274,27 @@ protected Class resolveClass(ObjectStreamClass desc) throws IOException, Clas } } + + /* + * Execute the configured IOExceptionHandler when an IOException is thrown during + * task execution and handle any runtime exceptions that the handler itself might throw. + * + * By default, the DefaultIOExceptionHandler will stop the broker when handling an IOException, + * however, if DefaultIOExceptionHandler is configured with startStopConnectors to be true + * it will throw a SuppressReplyException and not stop the broker. It's also possible another + * custom implementation of IOExceptionHandler could throw a runtime exception. + * + * This method will now handle and log those runtime exceptions so that the task will not + * die and will continue to execute future iterations if the broker is not shut down. + */ + private void handleIOException(String taskName, IOException ioe) { + try { + brokerService.handleIOException(ioe); + } catch (RuntimeException e) { + LOG.warn("IOException handler threw exception in task {} with " + + "error: {}, continuing.", taskName, + e.getMessage()); + LOG.debug(e.getMessage(), e); + } + } } diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/TempKahaDBStore.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/TempKahaDBStore.java index f7a49b8c186..6bbf9534eec 100644 --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/TempKahaDBStore.java +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/TempKahaDBStore.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; @@ -63,6 +64,7 @@ import org.apache.activemq.usage.MemoryUsage; import org.apache.activemq.usage.SystemUsage; import org.apache.activemq.util.ByteSequence; +import org.apache.activemq.util.SubscriptionKey; import org.apache.activemq.wireformat.WireFormat; public class TempKahaDBStore extends TempMessageDatabase implements PersistenceAdapter, BrokerServiceAware { @@ -299,6 +301,10 @@ public Integer execute(Transaction tx) throws IOException { getMessageStoreStatistics().getMessageCount().setCount(count); } + @Override + public StoreType getType() { + return StoreType.TEMP_KAHADB; + } } class KahaDBTopicMessageStore extends KahaDBMessageStore implements TopicMessageStore { @@ -332,6 +338,12 @@ public void addSubscription(SubscriptionInfo subscriptionInfo, boolean retroacti process(command); } + @Override + public Map> recoverExpired(Set subs, int max, + MessageRecoveryListener listener) { + throw new UnsupportedOperationException("recoverExpired not supported"); + } + @Override public void deleteSubscription(String clientId, String subscriptionName) throws IOException { KahaSubscriptionCommand command = new KahaSubscriptionCommand(); diff --git a/activemq-karaf-itest/pom.xml b/activemq-karaf-itest/pom.xml index c0b9bb3c034..fb9371bf027 100644 --- a/activemq-karaf-itest/pom.xml +++ b/activemq-karaf-itest/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-karaf-itest diff --git a/activemq-karaf/pom.xml b/activemq-karaf/pom.xml index ece2585f725..6fb9408be5b 100644 --- a/activemq-karaf/pom.xml +++ b/activemq-karaf/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-karaf diff --git a/activemq-karaf/src/main/resources/features-core.xml b/activemq-karaf/src/main/resources/features-core.xml index 197399a067f..7f6573d35b6 100644 --- a/activemq-karaf/src/main/resources/features-core.xml +++ b/activemq-karaf/src/main/resources/features-core.xml @@ -59,7 +59,7 @@ mvn:org.codehaus.jettison/jettison/${jettison-version} mvn:com.fasterxml.jackson.core/jackson-core/${jackson-version} mvn:com.fasterxml.jackson.core/jackson-databind/${jackson-version} - mvn:com.fasterxml.jackson.core/jackson-annotations/${jackson-version} + mvn:com.fasterxml.jackson.core/jackson-annotations/${jackson-annotations-version} diff --git a/activemq-log4j-appender/pom.xml b/activemq-log4j-appender/pom.xml index fe831da0d01..8617d74a677 100644 --- a/activemq-log4j-appender/pom.xml +++ b/activemq-log4j-appender/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-log4j-appender diff --git a/activemq-mqtt/pom.xml b/activemq-mqtt/pom.xml index 7d15a738a22..fbf677261b7 100644 --- a/activemq-mqtt/pom.xml +++ b/activemq-mqtt/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-mqtt diff --git a/activemq-openwire-generator/pom.xml b/activemq-openwire-generator/pom.xml index 6720fe12e19..c58cd0c4fb1 100644 --- a/activemq-openwire-generator/pom.xml +++ b/activemq-openwire-generator/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-openwire-generator diff --git a/activemq-openwire-legacy/pom.xml b/activemq-openwire-legacy/pom.xml index e6aafd12ea0..5ebd7a941a5 100644 --- a/activemq-openwire-legacy/pom.xml +++ b/activemq-openwire-legacy/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-openwire-legacy diff --git a/activemq-osgi/pom.xml b/activemq-osgi/pom.xml index ebacd77ee5a..2c502789aa8 100644 --- a/activemq-osgi/pom.xml +++ b/activemq-osgi/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-osgi diff --git a/activemq-osgi/src/main/resources/META-INF/spring.schemas b/activemq-osgi/src/main/resources/META-INF/spring.schemas index dd0abcad6a6..ea744c73365 100644 --- a/activemq-osgi/src/main/resources/META-INF/spring.schemas +++ b/activemq-osgi/src/main/resources/META-INF/spring.schemas @@ -90,6 +90,7 @@ http\://activemq.apache.org/schema/core/activemq-core-5.18.4.xsd=activemq.xsd http\://activemq.apache.org/schema/core/activemq-core-5.18.5.xsd=activemq.xsd http\://activemq.apache.org/schema/core/activemq-core-5.18.6.xsd=activemq.xsd http\://activemq.apache.org/schema/core/activemq-core-5.19.0.xsd=activemq.xsd +http\://activemq.apache.org/schema/core/activemq-core-5.19.1.xsd=activemq.xsd http\://camel.apache.org/schema/spring/camel-spring.xsd=camel-spring.xsd diff --git a/activemq-partition/pom.xml b/activemq-partition/pom.xml index d11ad3761ec..6cb10713e74 100644 --- a/activemq-partition/pom.xml +++ b/activemq-partition/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-partition diff --git a/activemq-pool/pom.xml b/activemq-pool/pom.xml index ef8d9f169c1..b220a8bb89d 100644 --- a/activemq-pool/pom.xml +++ b/activemq-pool/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-pool diff --git a/activemq-ra/pom.xml b/activemq-ra/pom.xml index 778588ec724..49ea6105687 100644 --- a/activemq-ra/pom.xml +++ b/activemq-ra/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-ra diff --git a/activemq-rar/pom.xml b/activemq-rar/pom.xml index 6c20b8ea0b5..82e9e39cd2a 100644 --- a/activemq-rar/pom.xml +++ b/activemq-rar/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-rar diff --git a/activemq-run/pom.xml b/activemq-run/pom.xml index c7098ab2549..e269b0c280a 100644 --- a/activemq-run/pom.xml +++ b/activemq-run/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-run diff --git a/activemq-runtime-config/pom.xml b/activemq-runtime-config/pom.xml index 829f27be259..6d907f214af 100644 --- a/activemq-runtime-config/pom.xml +++ b/activemq-runtime-config/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-runtime-config diff --git a/activemq-shiro/pom.xml b/activemq-shiro/pom.xml index ae834538a17..c816b488995 100644 --- a/activemq-shiro/pom.xml +++ b/activemq-shiro/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-shiro diff --git a/activemq-spring/pom.xml b/activemq-spring/pom.xml index 0b078747cf1..b2719e0d480 100644 --- a/activemq-spring/pom.xml +++ b/activemq-spring/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-spring diff --git a/activemq-spring/src/main/resources/META-INF/spring.schemas b/activemq-spring/src/main/resources/META-INF/spring.schemas index fe9dc7cb758..245cda50cfd 100644 --- a/activemq-spring/src/main/resources/META-INF/spring.schemas +++ b/activemq-spring/src/main/resources/META-INF/spring.schemas @@ -90,6 +90,7 @@ http\://activemq.apache.org/schema/core/activemq-core-5.18.4.xsd=activemq.xsd http\://activemq.apache.org/schema/core/activemq-core-5.18.5.xsd=activemq.xsd http\://activemq.apache.org/schema/core/activemq-core-5.18.6.xsd=activemq.xsd http\://activemq.apache.org/schema/core/activemq-core-5.19.0.xsd=activemq.xsd +http\://activemq.apache.org/schema/core/activemq-core-5.19.1.xsd=activemq.xsd http\://camel.apache.org/schema/osgi/camel-osgi.xsd=camel-osgi.xsd http\://camel.apache.org/schema/spring/camel-spring.xsd=camel-spring.xsd diff --git a/activemq-stomp/pom.xml b/activemq-stomp/pom.xml index bf23ce2e09d..62f92632e6c 100644 --- a/activemq-stomp/pom.xml +++ b/activemq-stomp/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-stomp diff --git a/activemq-tooling/activemq-junit/pom.xml b/activemq-tooling/activemq-junit/pom.xml index 5819b457e86..9415ef594b2 100644 --- a/activemq-tooling/activemq-junit/pom.xml +++ b/activemq-tooling/activemq-junit/pom.xml @@ -21,7 +21,7 @@ org.apache.activemq.tooling activemq-tooling - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-junit diff --git a/activemq-tooling/activemq-maven-plugin/pom.xml b/activemq-tooling/activemq-maven-plugin/pom.xml index 4e6e990d2ee..0faaaff0152 100644 --- a/activemq-tooling/activemq-maven-plugin/pom.xml +++ b/activemq-tooling/activemq-maven-plugin/pom.xml @@ -21,7 +21,7 @@ org.apache.activemq.tooling activemq-tooling - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-maven-plugin diff --git a/activemq-tooling/activemq-memtest-maven-plugin/pom.xml b/activemq-tooling/activemq-memtest-maven-plugin/pom.xml index c6add0db1bf..269a802222a 100644 --- a/activemq-tooling/activemq-memtest-maven-plugin/pom.xml +++ b/activemq-tooling/activemq-memtest-maven-plugin/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq.tooling activemq-tooling - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-memtest-maven-plugin diff --git a/activemq-tooling/activemq-perf-maven-plugin/pom.xml b/activemq-tooling/activemq-perf-maven-plugin/pom.xml index 54615d9c23f..40b335d2b53 100644 --- a/activemq-tooling/activemq-perf-maven-plugin/pom.xml +++ b/activemq-tooling/activemq-perf-maven-plugin/pom.xml @@ -21,7 +21,7 @@ org.apache.activemq.tooling activemq-tooling - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-perf-maven-plugin diff --git a/activemq-tooling/pom.xml b/activemq-tooling/pom.xml index 2ffcac877f9..eb1aef6157a 100644 --- a/activemq-tooling/pom.xml +++ b/activemq-tooling/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT org.apache.activemq.tooling diff --git a/activemq-unit-tests/pom.xml b/activemq-unit-tests/pom.xml index 9307084b59b..415759227bf 100644 --- a/activemq-unit-tests/pom.xml +++ b/activemq-unit-tests/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-unit-tests diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/JmsSendReceiveWithMessageExpirationTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/JmsSendReceiveWithMessageExpirationTest.java index 391253ee70c..2f61e41a3e0 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/JmsSendReceiveWithMessageExpirationTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/JmsSendReceiveWithMessageExpirationTest.java @@ -16,6 +16,8 @@ */ package org.apache.activemq; +import static org.junit.Assert.assertEquals; + import java.util.Date; import java.util.Vector; import java.util.concurrent.TimeUnit; @@ -31,8 +33,12 @@ import javax.jms.Topic; import org.apache.activemq.broker.BrokerRegistry; +import org.apache.activemq.broker.BrokerService; import org.apache.activemq.broker.region.DestinationStatistics; +import org.apache.activemq.broker.region.DurableTopicSubscription; +import org.apache.activemq.broker.region.Subscription; import org.apache.activemq.command.ActiveMQDestination; +import org.apache.activemq.store.TopicMessageStore; import org.apache.activemq.util.Wait; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,15 +57,20 @@ public class JmsSendReceiveWithMessageExpirationTest extends TestSupport { protected Destination producerDestination; protected boolean durable; protected int deliveryMode = DeliveryMode.PERSISTENT; - protected long timeToLive = 5000; + protected long timeToLive = 3000; protected boolean verbose; protected Connection connection; + protected BrokerService brokerService; protected void setUp() throws Exception { super.setUp(); + brokerService = new BrokerService(); + brokerService.setPersistent(false); + brokerService.start(); + data = new String[messageCount]; for (int i = 0; i < messageCount; i++) { @@ -225,7 +236,8 @@ public void testConsumeExpiredTopic() throws Exception { consumerDestination = session.createTopic(getConsumerSubject()); producerDestination = session.createTopic(getProducerSubject()); - MessageConsumer consumer = createConsumer(); + MessageConsumer consumer1 = createConsumer(); + MessageConsumer consumer2 = session.createConsumer(consumerDestination); connection.start(); for (int i = 0; i < data.length; i++) { @@ -247,7 +259,64 @@ public void testConsumeExpiredTopic() throws Exception { Thread.sleep(timeToLive + 1000); // message should have expired. - assertNull(consumer.receive(1000)); + assertNull(consumer1.receive(1000)); + assertNull(consumer2.receive(100)); + + for (Subscription consumer : brokerService.getDestination( + (ActiveMQDestination) consumerDestination) + .getConsumers()) { + assertEquals(0, consumer.getPendingQueueSize()); + } + + // Memory usage should be 0 after expiration + assertEquals(0, brokerService.getDestination((ActiveMQDestination) consumerDestination) + .getMemoryUsage().getUsage()); + } + + public void testConsumeExpiredTopicDurable() throws Exception { + brokerService.stop(); + + // Use persistent broker and durables so restart + brokerService = new BrokerService(); + brokerService.setPersistent(true); + brokerService.start(); + connection.close(); + connection = createConnection(); + connection.setClientID(getClass().getName()); + connection.start(); + session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + + MessageProducer producer = createProducer(timeToLive); + Topic topic = session.createTopic("test.expiration.topic"); + MessageConsumer consumer1 = session.createDurableSubscriber(topic, "sub1"); + MessageConsumer consumer2 = session.createDurableSubscriber(topic, "sub2"); + + for (int i = 0; i < data.length; i++) { + Message message = session.createTextMessage(data[i]); + message.setStringProperty("stringProperty", data[i]); + message.setIntProperty("intProperty", i); + producer.send(topic, message); + } + + // sleeps a second longer than the expiration time. + // Basically waits till topic expires. + Thread.sleep(timeToLive + 1000); + + // message should have expired for both clients + assertNull(consumer1.receive(1000)); + assertNull(consumer2.receive(100)); + + TopicMessageStore store = (TopicMessageStore) brokerService.getDestination((ActiveMQDestination) topic).getMessageStore(); + assertEquals(0, store.getMessageCount(getClass().getName(), "sub1")); + assertEquals(0, store.getMessageCount(getClass().getName(), "sub2")); + + for (Subscription consumer : brokerService.getDestination((ActiveMQDestination) topic) + .getConsumers()) { + assertEquals(0, consumer.getPendingQueueSize()); + } + // Memory usage should be 0 after expiration + assertEquals(0, brokerService.getDestination((ActiveMQDestination) topic) + .getMemoryUsage().getUsage()); } /** @@ -303,8 +372,15 @@ protected void tearDown() throws Exception { LOG.info("Dumping stats..."); LOG.info("Closing down connection"); - session.close(); - connection.close(); + try { + session.close(); + connection.close(); + } catch (Exception e) { + // ignore + } + if (brokerService != null) { + brokerService.stop(); + } } } diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/MessageExpirationTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/MessageExpirationTest.java index 5c7f29d257e..2f3ee82ad2e 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/MessageExpirationTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/MessageExpirationTest.java @@ -18,6 +18,7 @@ import javax.jms.DeliveryMode; + import junit.framework.Test; import org.apache.activemq.broker.region.policy.PolicyEntry; @@ -30,6 +31,7 @@ import org.apache.activemq.command.MessageAck; import org.apache.activemq.command.ProducerInfo; import org.apache.activemq.command.SessionInfo; +import org.apache.activemq.util.Wait; public class MessageExpirationTest extends BrokerTestSupport { @@ -261,6 +263,11 @@ public void testMessagesInSubscriptionPendingListExpire() throws Exception { assertNoMessagesLeft(connection); connection.send(closeConnectionInfo(connectionInfo)); + + if (!destination.isTemporary()) { + assertTrue(Wait.waitFor( + () -> broker.getDestination(destination).getMemoryUsage().getUsage() == 0, 1000, 100)); + } } public static Test suite() { diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/region/cursors/AbstractPendingMessageCursorTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/region/cursors/AbstractPendingMessageCursorTest.java index cd92da6969b..e6d0537f059 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/region/cursors/AbstractPendingMessageCursorTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/region/cursors/AbstractPendingMessageCursorTest.java @@ -76,7 +76,7 @@ public abstract class AbstractPendingMessageCursorTest extends AbstractStoreStat protected boolean enableSubscriptionStatistics; @Rule - public Timeout globalTimeout= new Timeout(60, TimeUnit.SECONDS); + public Timeout globalTimeout = new Timeout(60, TimeUnit.SECONDS); /** * @param prioritizedMessages diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/region/cursors/KahaDBPendingMessageCursorTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/region/cursors/KahaDBPendingMessageCursorTest.java index 8fd42e275e0..9bba67f723c 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/region/cursors/KahaDBPendingMessageCursorTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/region/cursors/KahaDBPendingMessageCursorTest.java @@ -35,9 +35,12 @@ import org.apache.activemq.ActiveMQConnectionFactory; import org.apache.activemq.broker.BrokerService; import org.apache.activemq.broker.region.DurableTopicSubscription; +import org.apache.activemq.broker.region.MessageReference; import org.apache.activemq.broker.region.Topic; +import org.apache.activemq.command.ActiveMQDestination; import org.apache.activemq.command.ActiveMQTextMessage; import org.apache.activemq.command.ActiveMQTopic; +import org.apache.activemq.command.MessageAck; import org.apache.activemq.store.MessageStoreSubscriptionStatistics; import org.apache.activemq.store.TopicMessageStore; import org.apache.activemq.store.kahadb.KahaDBPersistenceAdapter; @@ -352,4 +355,55 @@ public boolean isSatisified() throws Exception { } + // Test for AMQ-9721 + @Test + public void testDurableCursorRemoveRefMessageSize() throws Exception { + AtomicLong publishedMessageSize = new AtomicLong(); + + Connection connection = new ActiveMQConnectionFactory(brokerConnectURI).createConnection(); + connection.setClientID("clientId"); + connection.start(); + + // send 100 persistent and non persistent + Topic topic = publishTestMessagesDurable(connection, new String[] {"sub1"}, 100, + publishedMessageSize, DeliveryMode.NON_PERSISTENT); + publishTestMessagesDurable(connection, new String[] {"sub1"}, 100, + publishedMessageSize, DeliveryMode.PERSISTENT); + + SubscriptionKey subKey = new SubscriptionKey("clientId", "sub1"); + + // verify the count and size + verifyPendingStats(topic, subKey, 200, publishedMessageSize.get()); + + // Iterate and remove using the pending cursor to test removal + final DurableTopicSubscription sub = topic.getDurableTopicSubs().get(subKey); + PendingMessageCursor pending = sub.getPending(); + try { + pending.reset(); + while (pending.hasNext()) { + MessageReference node = pending.next(); + node.decrementReferenceCount(); + // test the remove(ref) method which has been updated + // to check persistence type + pending.remove(node); + + // If persistent remove out of the store + if (node.isPersistent()) { + MessageAck ack = new MessageAck(); + ack.setLastMessageId(node.getMessageId()); + ack.setAckType(MessageAck.STANDARD_ACK_TYPE); + ack.setDestination(topic.getActiveMQDestination()); + topic.acknowledge(sub.getContext(), sub, ack, node); + } + } + } finally { + pending.release(); + } + + // verify everything has been cleared correctly, persistent and + // non-persistent + verifyPendingStats(topic, subKey, 0, 0); + // Memory usage should be 0 after removal + assertEquals(0, topic.getMemoryUsage().getUsage()); + } } diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/region/cursors/StoreCursorRemoveFromCacheTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/region/cursors/StoreCursorRemoveFromCacheTest.java new file mode 100644 index 00000000000..4dd93646ed0 --- /dev/null +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/region/cursors/StoreCursorRemoveFromCacheTest.java @@ -0,0 +1,143 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.activemq.broker.region.cursors; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.util.function.BiConsumer; +import org.apache.activemq.broker.BrokerService; +import org.apache.activemq.broker.region.DestinationStatistics; +import org.apache.activemq.broker.region.MessageReference; +import org.apache.activemq.broker.region.Queue; +import org.apache.activemq.command.ActiveMQQueue; +import org.apache.activemq.command.ActiveMQTextMessage; +import org.apache.activemq.command.MessageId; +import org.apache.activemq.store.MessageStore; +import org.apache.activemq.store.kahadb.KahaDBStore; +import org.apache.activemq.usage.SystemUsage; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +public class StoreCursorRemoveFromCacheTest { + + @Rule + public TemporaryFolder dataFileDir = new TemporaryFolder(); + + private final ActiveMQQueue destination = new ActiveMQQueue("queue"); + private BrokerService broker; + private SystemUsage systemUsage; + private KahaDBStore store; + + @Before + public void setUp() throws Exception { + broker = new BrokerService(); + broker.setUseJmx(false); + broker.setPersistent(true); + KahaDBStore store = new KahaDBStore(); + store.setDirectory(dataFileDir.getRoot()); + broker.setPersistenceAdapter(store); + broker.start(); + systemUsage = broker.getSystemUsage(); + this.store = (KahaDBStore) broker.getPersistenceAdapter(); + } + + @After + public void tearDown() throws Exception { + broker.stop(); + } + + @Test(timeout = 10000) + public void testRemoveFromCacheIterator() throws Exception { + testRemoveFromCache((cursor, ref) -> { + // test removing using the iterator + cursor.remove(); + }); + } + + @Test(timeout = 10000) + public void testRemoveFromCacheRemoveMethod() throws Exception { + testRemoveFromCache((cursor, ref) -> { + // test using the remove method directly + // remove should also decrement after AMQ-9698, previously it did not + cursor.remove(ref); + assertEquals(0, ref.getReferenceCount()); + + // Call a second time to make sure we don't go negative + // and it will skip + cursor.remove(ref); + assertEquals(0, ref.getReferenceCount()); + }); + } + + private void testRemoveFromCache(BiConsumer remove) throws Exception { + var systemUsage = broker.getSystemUsage(); + final KahaDBStore store = (KahaDBStore) broker.getPersistenceAdapter(); + final MessageStore messageStore = store.createQueueMessageStore(destination); + final Queue queue = new Queue(broker, destination, messageStore, new DestinationStatistics(), null); + var memoryUsage = queue.getMemoryUsage(); + + // create cursor and make sure cache is enabled + QueueStorePrefetch cursor = new QueueStorePrefetch(queue, broker.getBroker()); + cursor.setSystemUsage(systemUsage); + cursor.start(); + assertTrue("cache enabled", cursor.isUseCache() && cursor.isCacheEnabled()); + + for (int i = 0; i < 10; i++) { + ActiveMQTextMessage msg = getMessage(i); + msg.setMemoryUsage(memoryUsage); + cursor.addMessageLast(msg); + // reference count of 1 for the cache + assertEquals(1, msg.getReferenceCount()); + } + + assertTrue(memoryUsage.getUsage() > 0); + + cursor.reset(); + while (cursor.hasNext()) { + // next will increment again so need to decrement the reference + // next is required to be called for remove() to work + var ref = cursor.next(); + assertEquals(2, ref.getReferenceCount()); + ref.decrementReferenceCount(); + remove.accept(cursor, ref); + assertEquals(0, ref.getReferenceCount()); + } + + assertEquals(0, memoryUsage.getUsage()); + assertEquals(0, cursor.size()); + } + + private ActiveMQTextMessage getMessage(int i) throws Exception { + ActiveMQTextMessage message = new ActiveMQTextMessage(); + MessageId id = new MessageId("11111:22222:" + i); + id.setBrokerSequenceId(i); + id.setProducerSequenceId(i); + message.setMessageId(id); + message.setDestination(destination); + message.setPersistent(true); + message.setResponseRequired(true); + message.setText("Msg:" + i + " " + "test"); + assertEquals(message.getMessageId().getProducerSequenceId(), i); + return message; + } + +} diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/region/cursors/StoreQueueCursorOrderTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/region/cursors/StoreQueueCursorOrderTest.java index 5a1ab90d589..771b884d801 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/region/cursors/StoreQueueCursorOrderTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/region/cursors/StoreQueueCursorOrderTest.java @@ -516,6 +516,11 @@ public void setBatch(MessageId message) { batch.incrementAndGet(); } + @Override + public StoreType getType() { + return StoreType.MEMORY; + } + @Override public void recoverMessageStoreStatistics() throws IOException { this.getMessageStoreStatistics().reset(); diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/virtual/AMQ9685Test.java b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/virtual/AMQ9685Test.java new file mode 100644 index 00000000000..59396d95492 --- /dev/null +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/virtual/AMQ9685Test.java @@ -0,0 +1,113 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.broker.virtual; + +import javax.jms.Connection; +import javax.jms.Session; +import org.apache.activemq.ActiveMQConnectionFactory; +import org.apache.activemq.broker.BrokerService; +import org.apache.activemq.broker.region.DestinationInterceptor; +import org.apache.activemq.broker.region.virtual.VirtualDestination; +import org.apache.activemq.broker.region.virtual.VirtualDestinationInterceptor; +import org.apache.activemq.broker.region.virtual.VirtualTopic; +import org.apache.activemq.command.ActiveMQQueue; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import static org.junit.Assert.assertTrue; + +public class AMQ9685Test { + + @Rule + public final TemporaryFolder temp = new TemporaryFolder(); + private BrokerService brokerService; + private Connection connection; + private String dir; + private ActiveMQQueue destination = new ActiveMQQueue("Consumer.foo."); + + @Before + public void init() throws Exception { + dir = temp.newFolder().getAbsolutePath(); + brokerService = createBroker(); + brokerService.start(); + connection = new ActiveMQConnectionFactory( + brokerService.getVmConnectorURI()).createConnection(); + connection.start(); + } + + @After + public void after() throws Exception { + try { + connection.close(); + } catch (Exception e) { + //swallow any error so broker can still be stopped + } + brokerService.stop(); + } + + @Test + public void testVirtualTopicMissingNameManual() throws Exception { + // Test manual creation + brokerService.getRegionBroker().addDestination(brokerService.getAdminConnectionContext(), + destination, false); + + // Verify created + assertTrue(brokerService.getBroker().getDestinationMap().containsKey(destination)); + assertTrue(brokerService.getPersistenceAdapter().getDestinations().contains(destination)); + + // Verify restart without issue + restart(); + } + + @Test + public void testVirtualTopicMissingNameConsumer() throws Exception { + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + // test consumer attempt dynamic creation + session.createConsumer(destination, null); + + // Verify created + assertTrue(brokerService.getBroker().getDestinationMap().containsKey(destination)); + assertTrue(brokerService.getPersistenceAdapter().getDestinations().contains(destination)); + + // Verify restart without issue + restart(); + } + + private BrokerService createBroker() { + BrokerService broker = new BrokerService(); + broker.setAdvisorySupport(false); + broker.setPersistent(true); + broker.setDataDirectory(dir); + + VirtualTopic virtualTopic = new VirtualTopic(); + VirtualDestinationInterceptor interceptor = new VirtualDestinationInterceptor(); + interceptor.setVirtualDestinations(new VirtualDestination[]{virtualTopic}); + broker.setDestinationInterceptors(new DestinationInterceptor[]{interceptor}); + return broker; + } + + private void restart() throws Exception { + brokerService.stop(); + brokerService.waitUntilStopped(); + brokerService = createBroker(); + brokerService.start(); + brokerService.waitUntilStarted(); + } +} diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/bugs/MessageExpirationReaperTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/bugs/MessageExpirationReaperTest.java index 4c8527a52cb..4e5d98684b7 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/bugs/MessageExpirationReaperTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/bugs/MessageExpirationReaperTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import javax.jms.*; import javax.management.ObjectName; @@ -30,6 +31,7 @@ import org.apache.activemq.broker.region.policy.PolicyMap; import org.apache.activemq.command.ActiveMQDestination; import org.apache.activemq.command.ActiveMQTopic; +import org.apache.activemq.util.Wait; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -164,11 +166,93 @@ public void testExpiredMessagesOnTopic() throws Exception{ consumer.close(); // Let the messages reach an expiry time - Thread.sleep(2000); + assertTrue("Incorrect queue size count", Wait.waitFor(() -> view.getQueueSize() == 0, + 3000, 100)); + assertTrue("Incorrect inflight count: " + view.getInFlightCount(), + Wait.waitFor(() -> view.getInFlightCount() == 0, 3000, 100)); + assertTrue("Incorrect expired size count", Wait.waitFor(() -> view.getEnqueueCount() == view.getExpiredCount(), + 3000, 100)); + + // Memory usage should be 0 after expiration + assertEquals(0, broker.getDestination(destination).getMemoryUsage().getUsage()); + } - assertEquals("Incorrect inflight count: " + view.getInFlightCount(), 0, view.getInFlightCount()); - assertEquals("Incorrect queue size count", 0, view.getQueueSize()); - assertEquals("Incorrect expired size count", view.getEnqueueCount(), view.getExpiredCount()); + @Test + public void testExpiredMessagesOnTopic2Durables() throws Exception{ + Session session = createSession(); + + // use a zero prefetch so messages don't go inflight + ActiveMQTopic destination = new ActiveMQTopic(destinationName + "?consumer.prefetchSize=0"); + + MessageProducer producer = session.createProducer(destination); + MessageConsumer consumer = session.createDurableSubscriber(destination, "test-durable"); + MessageConsumer consumer2 = session.createDurableSubscriber(destination, "test-durable-2"); + + producer.setTimeToLive(500); + + final int count = 3; + // Send some messages with an expiration + for (int i = 0; i < count; i++) { + TextMessage message = session.createTextMessage("" + i); + producer.send(message); + } + + DestinationViewMBean view = createView(destination); + // not expired yet... + assertEquals("Incorrect enqueue count", 3, view.getEnqueueCount() ); + + // close consumer so topic thinks consumer is inactive + consumer.close(); + consumer2.close(); + + // Let the messages reach an expiry time + assertTrue("Incorrect queue size count", Wait.waitFor(() -> view.getQueueSize() == 0, + 3000, 100)); + assertTrue("Incorrect inflight count: " + view.getInFlightCount(), + Wait.waitFor(() -> view.getInFlightCount() == 0, 3000, 100)); + + // should be 2x enqueued for 2 subs + assertTrue("Incorrect expired size count", Wait.waitFor(() -> view.getEnqueueCount() * 2 == view.getExpiredCount(), + 3000, 100)); + + // check store and memory usage + org.apache.activemq.broker.region.Destination brokerDest = broker.getDestination(destination); + assertEquals("Incorrect queue size count", 0, brokerDest.getMessageStore().getMessageCount()); + assertEquals(0, brokerDest.getMemoryUsage().getUsage()); + } + + @Test + public void testExpiredMessagesOnTopicRecoveryListener() throws Exception{ + Session session = createSession(); + + // use a zero prefetch so messages don't go inflight + ActiveMQTopic destination = new ActiveMQTopic(destinationName + "?consumer.prefetchSize=0"); + + MessageProducer producer = session.createProducer(destination); + MessageConsumer consumer = session.createDurableSubscriber(destination, "test-durable"); + producer.setTimeToLive(1000); + + final int count = 3; + // Send some messages with an expiration + for (int i = 0; i < count; i++) { + TextMessage message = session.createTextMessage("" + i); + producer.send(message); + } + + // Set a very low memory usage so we will be > 100% which should prevent the expiry + // thread from continuing + broker.getSystemUsage().getMemoryUsage().setLimit(1024); + DestinationViewMBean view = createView(destination); + + // not expired yet... + assertEquals("Incorrect enqueue count", 3, view.getEnqueueCount() ); + + // close consumer so topic thinks consumer is inactive + consumer.close(); + + // Memory is > 100% so we shouldn't expire + Thread.sleep(3000); + assertEquals(0, view.getExpiredCount()); } protected DestinationViewMBean createView(ActiveMQDestination destination) throws Exception { diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/network/DurableFiveBrokerNetworkBridgeTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/network/DurableFiveBrokerNetworkBridgeTest.java index 86672c5577d..876e44dd1ce 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/network/DurableFiveBrokerNetworkBridgeTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/network/DurableFiveBrokerNetworkBridgeTest.java @@ -25,6 +25,7 @@ import javax.jms.MessageConsumer; import javax.jms.Session; +import junit.framework.AssertionFailedError; import org.apache.activemq.JmsMultipleBrokersTestSupport; import org.apache.activemq.broker.BrokerService; import org.apache.activemq.broker.region.Destination; @@ -49,6 +50,11 @@ public class DurableFiveBrokerNetworkBridgeTest extends JmsMultipleBrokersTestSu @Override protected NetworkConnector bridgeBrokers(String localBrokerName, String remoteBrokerName) throws Exception { + return bridgeBrokers(localBrokerName, remoteBrokerName, false, -1); + } + + protected NetworkConnector bridgeBrokers(String localBrokerName, String remoteBrokerName, + boolean dynamicOnly, int networkTTL) throws Exception { NetworkConnector connector = super.bridgeBrokers(localBrokerName, remoteBrokerName); ArrayList includedDestinations = new ArrayList<>(); includedDestinations.add(new ActiveMQTopic("TEST.FOO?forceDurable=true")); @@ -57,7 +63,8 @@ protected NetworkConnector bridgeBrokers(String localBrokerName, String remoteBr connector.setDecreaseNetworkConsumerPriority(false); connector.setConduitSubscriptions(true); connector.setSyncDurableSubs(true); - connector.setNetworkTTL(-1); + connector.setDynamicOnly(dynamicOnly); + connector.setNetworkTTL(networkTTL); connector.setClientIdToken("|"); return connector; } @@ -604,6 +611,203 @@ public void testDurablePropagationMultipleBridgesDifferentDestinations() throws assertNCDurableSubsCount(brokers.get("Broker_A_A").broker, dest2, 0); } + /* + * The following tests should work for correct sync/propagation of durables even if + * TTL is missing on broker restart. This is for TTL: 0, -1, 1, or 4 (same number + * of network hops in the network) + */ + public void testDurablePropagationSyncTtl0BrokerRestart() throws Exception { + testDurablePropagation(0, false, true, List.of(0, 0, 0, 0, 0)); + } + + public void testDurablePropagationSyncTtl1BrokerRestart() throws Exception { + testDurablePropagation(1, false, true, List.of(0, 1, 0, 1, 0)); + } + + public void testDurablePropagationSyncTtl4BrokerRestart() throws Exception { + testDurablePropagation(4, false, true, List.of(1, 2, 2, 2, 1)); + } + + // This test also tests the fix in NetworkBridgeUtils.getBrokerSubscriptionInfo() + // where the clientId was missing for the offline durable which could cause demand to be created + // by mistake and create a loop. Without the clientId the sync could not tell the + // network durable was for its direct bridge (was created because of a local consumer + // on its broker) so a loop could be created on restart if the sync happened before the + // real consumer connected first. + public void testDurablePropagationSyncTtlNotSetBrokerRestart() throws Exception { + testDurablePropagation(-1, false, true, List.of(1, 2, 2, 2, 1)); + } + + /* + * The following test demonstrates the problem with missing TTL and is only solved + * by making dynamicOnly true, on restart consumers have yet to come back online + * for offline durables so we end up missing TTL and create extra demand on sync. + * TTL is missing on broker restart. This is for TTL > 1 but less than same number + * of network hops in the network + */ + public void testDurablePropagationDynamicFalseTtl2BrokerRestartFail() throws Exception { + try { + testDurablePropagation(2, false, true, List.of(0, 1, 2, 1, 0)); + fail("Exepected to fail"); + } catch (AssertionFailedError e) { + // expected + } + } + + public void testDurablePropagationDynamicFalseTtl3BrokerRestartFail() throws Exception { + try { + testDurablePropagation(2, false, true, List.of(0, 2, 2, 2, 0)); + fail("Exepected to fail"); + } catch (AssertionFailedError e) { + // expected + } + } + + /* + * The following tests make sure propagation works correctly with TTL set + * when dynamicOnly is false or true, and durable sync enabled. + * When false, durables get reactivated and a sync is done and TTL can be + * missing for offline durables so this works correctly ONLY if the consumers + * are online and have correct TTL info. When true, consumers drive reactivation + * entirely and the sync is skipped. + * + * For dynamicOnly being false, these tests for TTL > 1 demonstrate the improvement in + * createDemandSubscription() inside of DemandForwardingBridgeSupport + * where we now include the full broker path (all TTL info) if the + * consumer is already online that created the subscription. Before, + * we just included the remote broker. + * + * If brokers are restarted/consumers offline, TTL can be missing so it's recommended + * to set dynamicOnly to true if TTL is > 1 (TTL 1 and TTL -1 can still be handled) + * to prevent propagation of durables that shouldn't exist and let consumers drive + * the reactivation. + * + * The tests keep the consumers online and only restart the connectors and not + * the brokers so the consumer info and TTL are preserved for the tests. + */ + public void testDurablePropagationDynamicFalseTtl0() throws Exception { + testDurablePropagation(0, false, List.of(0, 0, 0, 0, 0)); + } + + public void testDurablePropagationDynamicFalseTtl1() throws Exception { + testDurablePropagation(1, false, List.of(0, 1, 0, 1, 0)); + } + + public void testDurablePropagationDynamicFalseTtl2() throws Exception { + testDurablePropagation(2, false, List.of(0, 1, 2, 1, 0)); + } + + public void testDurablePropagationDynamicFalseTtl3() throws Exception { + testDurablePropagation(3, false, List.of(0, 2, 2, 2, 0)); + } + + public void testDurablePropagationDynamicFalseTtl4() throws Exception { + testDurablePropagation(4, false, List.of(1, 2, 2, 2, 1)); + } + + public void testDurablePropagationDynamicFalseTtlNotSet() throws Exception { + testDurablePropagation(-1, false, List.of(1, 2, 2, 2, 1)); + } + + public void testDurablePropagationDynamicTrueTtl0() throws Exception { + testDurablePropagation(0, true, List.of(0, 0, 0, 0, 0)); + } + + public void testDurablePropagationDynamicTrueTtl1() throws Exception { + testDurablePropagation(1, true, List.of(0, 1, 0, 1, 0)); + } + + public void testDurablePropagationDynamicTrueTtl2() throws Exception { + testDurablePropagation(2, true, List.of(0, 1, 2, 1, 0)); + } + + public void testDurablePropagationDynamicTrueTtl3() throws Exception { + testDurablePropagation(3, true, List.of(0, 2, 2, 2, 0)); + } + + public void testDurablePropagationDynamicTrueTtl4() throws Exception { + testDurablePropagation(4, true, List.of(1, 2, 2, 2, 1)); + } + + public void testDurablePropagationDynamicTrueTtlNotSet() throws Exception { + testDurablePropagation(-1, true, List.of(1, 2, 2, 2, 1)); + } + + private void testDurablePropagation(int ttl, boolean dynamicOnly, + List expected) throws Exception { + testDurablePropagation(ttl, dynamicOnly, false, expected); + } + + private void testDurablePropagation(int ttl, boolean dynamicOnly, boolean restartBrokers, + List expected) throws Exception { + duplex = true; + + // Setup broker networks + NetworkConnector nc1 = bridgeBrokers("Broker_A_A", "Broker_B_B", dynamicOnly, ttl); + NetworkConnector nc2 = bridgeBrokers("Broker_B_B", "Broker_C_C", dynamicOnly, ttl); + NetworkConnector nc3 = bridgeBrokers("Broker_C_C", "Broker_D_D", dynamicOnly, ttl); + NetworkConnector nc4 = bridgeBrokers("Broker_D_D", "Broker_E_E", dynamicOnly, ttl); + + startAllBrokers(); + stopNetworkConnectors(nc1, nc2, nc3, nc4); + + // Setup destination + ActiveMQTopic dest = (ActiveMQTopic) createDestination("TEST.FOO", true); + + // Setup consumers + Session ses = createSession("Broker_A_A"); + Session ses2 = createSession("Broker_E_E"); + MessageConsumer clientA = ses.createDurableSubscriber(dest, "subA"); + MessageConsumer clientE = ses2.createDurableSubscriber(dest, "subE"); + Thread.sleep(1000); + + assertNCDurableSubsCount(brokers.get("Broker_A_A").broker, dest, 0); + assertNCDurableSubsCount(brokers.get("Broker_B_B").broker, dest, 0); + assertNCDurableSubsCount(brokers.get("Broker_C_C").broker, dest, 0); + assertNCDurableSubsCount(brokers.get("Broker_D_D").broker, dest, 0); + assertNCDurableSubsCount(brokers.get("Broker_E_E").broker, dest, 0); + + startNetworkConnectors(nc1, nc2, nc3, nc4); + Thread.sleep(1000); + + // Check that the correct network durables exist + assertNCDurableSubsCount(brokers.get("Broker_A_A").broker, dest, expected.get(0)); + assertNCDurableSubsCount(brokers.get("Broker_B_B").broker, dest, expected.get(1)); + assertNCDurableSubsCount(brokers.get("Broker_C_C").broker, dest, expected.get(2)); + assertNCDurableSubsCount(brokers.get("Broker_D_D").broker, dest, expected.get(3)); + assertNCDurableSubsCount(brokers.get("Broker_E_E").broker, dest, expected.get(4)); + + if (restartBrokers) { + // go offline and restart to make sure sync works to re-enable and doesn't + // propagate wrong demand + clientA.close(); + clientE.close(); + destroyAllBrokers(); + setUp(); + brokers.values().forEach(bi -> bi.broker.setDeleteAllMessagesOnStartup(false)); + bridgeBrokers("Broker_A_A", "Broker_B_B", dynamicOnly, ttl); + bridgeBrokers("Broker_B_B", "Broker_C_C", dynamicOnly, ttl); + bridgeBrokers("Broker_C_C", "Broker_D_D", dynamicOnly, ttl); + bridgeBrokers("Broker_D_D", "Broker_E_E", dynamicOnly, ttl); + startAllBrokers(); + } else { + // restart just the network connectors but leave the consumers online + // to test sync works ok. Things should work for all cases both dynamicOnly + // false and true because TTL info still exits and consumers are online + stopNetworkConnectors(nc1, nc2, nc3, nc4); + Thread.sleep(1000); + startNetworkConnectors(nc1, nc2, nc3, nc4); + Thread.sleep(1000); + } + + // after restarting the bridges, check sync/demand are correct + assertNCDurableSubsCount(brokers.get("Broker_A_A").broker, dest, expected.get(0)); + assertNCDurableSubsCount(brokers.get("Broker_B_B").broker, dest, expected.get(1)); + assertNCDurableSubsCount(brokers.get("Broker_C_C").broker, dest, expected.get(2)); + assertNCDurableSubsCount(brokers.get("Broker_D_D").broker, dest, expected.get(3)); + assertNCDurableSubsCount(brokers.get("Broker_E_E").broker, dest, expected.get(4)); + } + protected void assertNCDurableSubsCount(final BrokerService brokerService, final ActiveMQTopic dest, final int count) throws Exception { assertTrue(Wait.waitFor(new Condition() { @@ -628,7 +832,7 @@ protected List getNCDurableSubs(final BrokerService br for (SubscriptionKey key : destination.getDurableTopicSubs().keySet()) { if (key.getSubscriptionName().startsWith(DemandForwardingBridge.DURABLE_SUB_PREFIX)) { DurableTopicSubscription sub = destination.getDurableTopicSubs().get(key); - if (sub != null) { + if (sub != null && sub.isActive()) { subs.add(sub); } } @@ -657,6 +861,18 @@ protected void configureBroker(BrokerService broker) { broker.setDataDirectory("target" + File.separator + "test-data" + File.separator + "DurableFiveBrokerNetworkBridgeTest"); } + protected void startNetworkConnectors(NetworkConnector... connectors) throws Exception { + for (NetworkConnector connector : connectors) { + connector.start(); + } + } + + protected void stopNetworkConnectors(NetworkConnector... connectors) throws Exception { + for (NetworkConnector connector : connectors) { + connector.stop(); + } + } + protected Session createSession(String broker) throws Exception { Connection con = createConnection(broker); con.start(); diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBOffsetRecoveryListenerTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBOffsetRecoveryListenerTest.java index f5df1fb9c82..0adaac052ee 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBOffsetRecoveryListenerTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBOffsetRecoveryListenerTest.java @@ -32,7 +32,9 @@ import org.apache.activemq.store.MessageStore; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TestName; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -52,9 +54,17 @@ public class KahaDBOffsetRecoveryListenerTest { protected BrokerService brokerService = null; protected BrokerService restartBrokerService = null; + @Rule + public TestName testName = new TestName(); + + protected final int PRETEST_MSG_COUNT = 17531; + @Before public void beforeEach() throws Exception { - + // Send+Recv a odd number of messages beyond cache sizes + // to confirm the queue's sequence number gets pushed off + sendMessages(PRETEST_MSG_COUNT, testName.getMethodName()); + assertEquals(Integer.valueOf(PRETEST_MSG_COUNT), Integer.valueOf(receiveMessages(testName.getMethodName()))); } @After @@ -68,7 +78,7 @@ protected BrokerService createBroker(KahaDBStore kaha) throws Exception { broker.setUseJmx(false); broker.setPersistenceAdapter(kaha); broker.start(); - broker.waitUntilStarted(10_000l); + broker.waitUntilStarted(10_000L); return broker; } @@ -130,7 +140,7 @@ protected void runOffsetLoopTest(final int sendCount, final int expectedMessageC restartBrokerService = createBroker(createStore(false)); restartBrokerService.start(); - restartBrokerService.waitUntilStarted(30_000l); + restartBrokerService.waitUntilStarted(30_000L); assertEquals(sendCount, receiveMessages(queueName)); restartBrokerService.stop(); @@ -139,42 +149,42 @@ protected void runOffsetLoopTest(final int sendCount, final int expectedMessageC @Test public void testOffsetZero() throws Exception { - runOffsetTest(1_000, 1_000, 0, 1, 1, 0, "TEST.OFFSET.ZERO"); + runOffsetTest(1_000, 1_000, 0, 1, 1, 0, testName.getMethodName()); } @Test public void testOffsetOne() throws Exception { - runOffsetTest(1_000, 1_000, 1, 1, 1, 1, "TEST.OFFSET.ONE"); + runOffsetTest(1_000, 1_000, 1, 1, 1, 1, testName.getMethodName()); } @Test public void testOffsetLastMinusOne() throws Exception { - runOffsetTest(1_000, 1_000, 999, 1, 1, 999, "TEST.OFFSET.LASTMINUSONE"); + runOffsetTest(1_000, 1_000, 999, 1, 1, 999, testName.getMethodName()); } @Test public void testOffsetLast() throws Exception { - runOffsetTest(1_000, 1_000, 1_000, 1, 0, -1, "TEST.OFFSET.LAST"); + runOffsetTest(1_000, 1_000, 1_000, 1, 0, -1, testName.getMethodName()); } @Test public void testOffsetBeyondQueueSizeNoError() throws Exception { - runOffsetTest(1_000, 1_000, 10_000, 1, 0, -1, "TEST.OFFSET.BEYOND"); + runOffsetTest(1_000, 1_000, 10_000, 1, 0, -1, testName.getMethodName()); } @Test public void testOffsetEmptyQueue() throws Exception { - runOffsetTest(0, 0, 10_000, 1, 0, -1, "TEST.OFFSET.EMPTY"); + runOffsetTest(0, 0, 10_000, 1, 0, -1, testName.getMethodName()); } @Test public void testOffsetWalk() throws Exception { - runOffsetLoopTest(10_000, 10_000, 9_000, 200, 200, 9_000, "TEST.OFFSET.WALK", 8, false); + runOffsetLoopTest(10_000, 10_000, 9_000, 200, 200, 9_000, testName.getMethodName(), 8, false); } @Test public void testOffsetRepeat() throws Exception { - runOffsetLoopTest(10_000, 10_000, 7_000, 133, 133, 7_000, "TEST.OFFSET.REPEAT", 10, true); + runOffsetLoopTest(10_000, 10_000, 7_000, 133, 133, 7_000, testName.getMethodName(), 10, true); } private void sendMessages(int count, String queueName) throws JMSException { @@ -198,12 +208,15 @@ private void sendMessages(int count, String queueName) throws JMSException { private int receiveMessages(String queueName) throws JMSException { int rc=0; ActiveMQConnectionFactory cf = new ActiveMQConnectionFactory("vm://localhost"); + cf.setWatchTopicAdvisories(false); + Connection connection = cf.createConnection(); + try { connection.start(); Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); MessageConsumer messageConsumer = session.createConsumer(new ActiveMQQueue(queueName)); - while ( messageConsumer.receive(1000) !=null ) { + while (messageConsumer.receive(1_000L) != null) { rc++; } return rc; diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBRecoverExpiredTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBRecoverExpiredTest.java new file mode 100644 index 00000000000..5491b2755ad --- /dev/null +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBRecoverExpiredTest.java @@ -0,0 +1,390 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.store.kahadb; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import javax.jms.Connection; +import javax.jms.JMSException; +import javax.jms.Message; +import javax.jms.MessageProducer; +import javax.jms.Session; +import javax.jms.TopicSession; +import java.io.File; +import java.net.URI; +import java.util.HashSet; +import java.util.List; +import java.util.Map.Entry; +import java.util.Set; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.activemq.ActiveMQConnectionFactory; +import org.apache.activemq.broker.BrokerService; +import org.apache.activemq.broker.TransportConnector; +import org.apache.activemq.broker.region.Destination; +import org.apache.activemq.broker.region.Topic; +import org.apache.activemq.command.ActiveMQTextMessage; +import org.apache.activemq.command.ActiveMQTopic; +import org.apache.activemq.command.MessageAck; +import org.apache.activemq.command.MessageId; +import org.apache.activemq.store.MessageRecoveryListener; +import org.apache.activemq.store.TopicMessageStore; +import org.apache.activemq.util.SubscriptionKey; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.rules.Timeout; + +/** + * Test for {@link TopicMessageStore#recoverExpired(Set, int, org.apache.activemq.store.MessageRecoveryListener)} + */ +public class KahaDBRecoverExpiredTest { + + @Rule + public Timeout globalTimeout = new Timeout(60, TimeUnit.SECONDS); + + @Rule + public TemporaryFolder dataFileDir = new TemporaryFolder(new File("target")); + + private BrokerService broker; + private URI brokerConnectURI; + private final ActiveMQTopic topic = new ActiveMQTopic("test.topic"); + private final SubscriptionKey subKey1 = new SubscriptionKey("clientId", "sub1"); + private final SubscriptionKey subKey2 = new SubscriptionKey("clientId", "sub2"); + + @Before + public void startBroker() throws Exception { + broker = new BrokerService(); + broker.setPersistent(true); + KahaDBPersistenceAdapter persistenceAdapter = new KahaDBPersistenceAdapter(); + persistenceAdapter.setDirectory(dataFileDir.getRoot()); + broker.setPersistenceAdapter(persistenceAdapter); + //set up a transport + TransportConnector connector = broker + .addConnector(new TransportConnector()); + connector.setUri(new URI("tcp://0.0.0.0:0")); + connector.setName("tcp"); + broker.start(); + broker.waitUntilStarted(); + brokerConnectURI = broker.getConnectorByName("tcp").getConnectUri(); + } + + @After + public void stopBroker() throws Exception { + broker.stop(); + broker.waitUntilStopped(); + } + + private Session initializeSubs() throws JMSException { + Connection connection = new ActiveMQConnectionFactory(brokerConnectURI).createConnection(); + connection.setClientID("clientId"); + connection.start(); + + Session session = connection.createSession(false, TopicSession.AUTO_ACKNOWLEDGE); + session.createDurableSubscriber(topic, "sub1"); + session.createDurableSubscriber(topic, "sub2"); + + return session; + } + + // test recover expired works in general, verify does not return + // expired if subs have already acked + @Test + public void testRecoverExpired() throws Exception { + try (Session session = initializeSubs()) { + MessageProducer prod = session.createProducer(topic); + + Destination dest = broker.getDestination(topic); + TopicMessageStore store = (TopicMessageStore) dest.getMessageStore(); + + // nothing should be expired yet, no messags + var expired = store.recoverExpired(Set.of(subKey1, subKey2), 100, listener); + assertTrue(expired.isEmpty()); + + // Sent 10 messages, alternating no expiration and 1 second ttl + for (int i = 0; i < 10; i++) { + ActiveMQTextMessage message = new ActiveMQTextMessage(); + message.setText("message" + i); + var ttl = i % 2 == 0 ? 1000 : 0; + prod.send(message, Message.DEFAULT_DELIVERY_MODE, Message.DEFAULT_PRIORITY, ttl); + } + + // wait for the time to pass the point of needing expiration + Thread.sleep(1500); + // We should now find both durables have 5 expired messages + expired = store.recoverExpired(Set.of(subKey1, subKey2), 100, listener); + assertEquals(2, expired.size()); + assertEquals(5, expired.get(subKey1).size()); + assertEquals(5, expired.get(subKey2).size()); + + // Acknowledge the first 2 messages of only the first sub + for (int i = 0; i < 2; i++) { + MessageAck ack = new MessageAck(); + ack.setLastMessageId(expired.get(subKey1).get(i).getMessageId()); + ack.setAckType(MessageAck.EXPIRED_ACK_TYPE); + ack.setDestination(topic); + store.acknowledge(broker.getAdminConnectionContext(),"clientId", "sub1", + ack.getLastMessageId(), ack); + } + + // Now the first sub should only have 3 expired, but still 5 on the second + expired = store.recoverExpired(Set.of(subKey1, subKey2), 100, listener); + assertEquals(3, expired.get(subKey1).size()); + assertEquals(5, expired.get(subKey2).size()); + + // ack all remaining + for (Entry> entry : expired.entrySet()) { + for (org.apache.activemq.command.Message message : entry.getValue()) { + MessageAck ack = new MessageAck(); + ack.setLastMessageId(message.getMessageId()); + ack.setAckType(MessageAck.EXPIRED_ACK_TYPE); + ack.setDestination(topic); + store.acknowledge(broker.getAdminConnectionContext(),entry.getKey().getClientId(), + entry.getKey().getSubscriptionName(), ack.getLastMessageId(), ack); + } + } + + // should be empty again + expired = store.recoverExpired(Set.of(subKey1, subKey2), 100, listener); + assertTrue(expired.isEmpty()); + } + + } + + // test max number of messages to check works + @Test + public void testRecoverExpiredMax() throws Exception { + try (Session session = initializeSubs()) { + MessageProducer prod = session.createProducer(topic); + + Destination dest = broker.getDestination(topic); + TopicMessageStore store = (TopicMessageStore) dest.getMessageStore(); + + // nothing should be expired yet, no messags + var expired = store.recoverExpired(Set.of(subKey1, subKey2), 100, listener); + assertTrue(expired.isEmpty()); + + // Sent 50 messages with no ttl followed by 50 with ttl + ActiveMQTextMessage message = new ActiveMQTextMessage(); + for (int i = 0; i < 100; i++) { + message.setText("message" + i); + var ttl = i >= 50 ? 1000 : 0; + prod.send(message, Message.DEFAULT_DELIVERY_MODE, Message.DEFAULT_PRIORITY, ttl); + } + + // wait for the time to pass the point of needing expiration + Thread.sleep(1500); + + // We should now find both durables have 50 expired messages + expired = store.recoverExpired(Set.of(subKey1, subKey2), 100, listener); + assertEquals(2, expired.size()); + assertEquals(50, expired.get(subKey1).size()); + assertEquals(50, expired.get(subKey2).size()); + + // Max is 50, should find none expired + expired = store.recoverExpired(Set.of(subKey1, subKey2), 50, listener); + assertTrue(expired.isEmpty()); + + // We should now find both durables have 25 expired messages with + // max at 75 + expired = store.recoverExpired(Set.of(subKey1, subKey2), 75, listener); + assertEquals(2, expired.size()); + assertEquals(25, expired.get(subKey1).size()); + assertEquals(25, expired.get(subKey2).size()); + + // Acknowledge the first 25 messages of only the first sub + for (int i = 0; i < 25; i++) { + MessageAck ack = new MessageAck(); + ack.setLastMessageId(expired.get(subKey1).get(i).getMessageId()); + ack.setAckType(MessageAck.EXPIRED_ACK_TYPE); + ack.setDestination(topic); + store.acknowledge(broker.getAdminConnectionContext(),"clientId", "sub1", + ack.getLastMessageId(), ack); + } + + // We should now find 25 on sub1 and 50 on sub2 with a max of 100 + expired = store.recoverExpired(Set.of(subKey1, subKey2), 100, listener); + assertEquals(2, expired.size()); + assertEquals(25, expired.get(subKey1).size()); + assertEquals(50, expired.get(subKey2).size()); + + } + + } + + // Test that filtering works by the set of subscriptions + @Test + public void testRecoverExpiredSubSet() throws Exception { + try (Session session = initializeSubs()) { + MessageProducer prod = session.createProducer(topic); + + Destination dest = broker.getDestination(topic); + TopicMessageStore store = (TopicMessageStore) dest.getMessageStore(); + + // nothing should be expired yet, no messags + var expired = store.recoverExpired(Set.of(subKey1, subKey2), 100, listener); + assertTrue(expired.isEmpty()); + + // Send 10 expired + for (int i = 0; i < 10; i++) { + ActiveMQTextMessage message = new ActiveMQTextMessage(); + message.setText("message" + i); + prod.send(message, Message.DEFAULT_DELIVERY_MODE, Message.DEFAULT_PRIORITY, 1000); + } + + // wait for the time to pass the point of needing expiration + Thread.sleep(1500); + + // Test getting each sub individually, get sub2 first + expired = store.recoverExpired(Set.of(subKey2), 100, listener); + assertEquals(1, expired.size()); + assertEquals(10, expired.get(subKey2).size()); + + // ack the first message of sub2 + MessageAck ack = new MessageAck(); + ack.setLastMessageId(expired.get(subKey2).get(0).getMessageId()); + ack.setAckType(MessageAck.EXPIRED_ACK_TYPE); + ack.setDestination(topic); + store.acknowledge(broker.getAdminConnectionContext(),"clientId", "sub2", + ack.getLastMessageId(), ack); + + // check only sub2 has 9 + expired = store.recoverExpired(Set.of(subKey2), 100, listener); + assertEquals(1, expired.size()); + assertEquals(9, expired.get(subKey2).size()); + + // check only sub1 still has 10 + expired = store.recoverExpired(Set.of(subKey1), 100, listener); + assertEquals(1, expired.size()); + assertEquals(10, expired.get(subKey1).size()); + + // verify passing in unmatched sub leaves it out of the result set + var unmatched = new SubscriptionKey("clientId", "sub3"); + expired = store.recoverExpired(Set.of(unmatched), 100, listener); + assertTrue(expired.isEmpty()); + + // try 2 that exist and 1 that doesn't + expired = store.recoverExpired(Set.of(subKey1, subKey2, unmatched), 100, listener); + assertEquals(2, expired.size()); + + } + + } + + // test recovery listener works with hasSpace() + @Test + public void testRecoverExpiredRecoveryListener() throws Exception { + try (Session session = initializeSubs()) { + MessageProducer prod = session.createProducer(topic); + + Destination dest = broker.getDestination(topic); + TopicMessageStore store = (TopicMessageStore) dest.getMessageStore(); + + // Sent 50 messages with no ttl followed by 50 with ttl + ActiveMQTextMessage message = new ActiveMQTextMessage(); + for (int i = 0; i < 10; i++) { + message.setText("message" + i); + prod.send(message, Message.DEFAULT_DELIVERY_MODE, Message.DEFAULT_PRIORITY, 1000); + } + + // wait for the time to pass the point of needing expiration + Thread.sleep(1500); + + // don't return any, has space is false + final AtomicBoolean hasSpaceCalled = new AtomicBoolean(); + var expired = store.recoverExpired(Set.of(subKey1, subKey2), 100, new MessageRecoveryListener() { + @Override + public boolean recoverMessage(org.apache.activemq.command.Message message) { + return true; + } + + @Override + public boolean recoverMessageReference(MessageId ref) { + return false; + } + + @Override + public boolean hasSpace() { + hasSpaceCalled.set(true); + return false; + } + + @Override + public boolean isDuplicate(MessageId ref) { + return false; + } + }); + + assertTrue(expired.isEmpty()); + assertTrue(hasSpaceCalled.get()); + + // check we only call recoverMessage() once for each unique id\ + Set ids = new HashSet<>(); + store.recoverExpired(Set.of(subKey1, subKey2), 100, new MessageRecoveryListener() { + @Override + public boolean recoverMessage(org.apache.activemq.command.Message message) { + // assert unique + assertTrue("duplicate message passed to listener", ids.add(message.getMessageId())); + return true; + } + + @Override + public boolean recoverMessageReference(MessageId ref) { + return false; + } + + @Override + public boolean hasSpace() { + return true; + } + + @Override + public boolean isDuplicate(MessageId ref) { + return false; + } + }); + } + + } + + private final MessageRecoveryListener listener = new MessageRecoveryListener() { + + @Override + public boolean recoverMessage(org.apache.activemq.command.Message message) { + return true; + } + + @Override + public boolean recoverMessageReference(MessageId ref) { + return true; + } + + @Override + public boolean hasSpace() { + return true; + } + + @Override + public boolean isDuplicate(MessageId ref) { + return false; + } + }; + +} diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBTest.java index 53306bd7558..8cf84fb0d60 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBTest.java @@ -27,11 +27,15 @@ import javax.jms.MessageProducer; import javax.jms.Session; +import java.util.concurrent.atomic.AtomicInteger; import junit.framework.TestCase; import org.apache.activemq.ActiveMQConnectionFactory; import org.apache.activemq.broker.BrokerService; import org.apache.activemq.command.ActiveMQQueue; +import org.apache.activemq.util.DefaultIOExceptionHandler; +import org.apache.activemq.util.IOExceptionHandler; +import org.apache.activemq.util.Wait; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.core.LogEvent; @@ -236,6 +240,69 @@ public void append(LogEvent event) { assertFalse("Did not replay any records from the journal", didSomeRecovery.get()); } + + /** + * Test the checkpoint runner task continues to run if the configured + * IOExceptionHandler throws a runtime exception while processing + * the IOException it is handling and the broker is not shut down. This + * could happen if using the DefaultIOExceptionHandler and startStopConnectors + * is set to true, or if a user provides their own IOExceptionHandler that + * throws an exception. + */ + public void testCheckpointExceptionKeepRunning() throws Exception { + testCheckpointIOException(true); + } + + /** + * Test the broker shuts down by when DefaultIOExceptionHandler + * handles an IOException thrown by the checkpoint runner task. This is the + * default behavior of the broker if not configured with a custom + * IOExceptionHandler and startStopConnectors is false + */ + public void testCheckpointExceptionShutdown() throws Exception { + testCheckpointIOException(false); + } + + private void testCheckpointIOException(boolean startStopConnectors) throws Exception { + final AtomicInteger iterations = new AtomicInteger(); + // Create a store that always throws an IOException when checkpoint is called + final KahaDBStore kaha = new KahaDBStore() { + @Override + protected void checkpointCleanup(boolean cleanup) throws IOException { + iterations.incrementAndGet(); + throw new IOException("fail"); + } + }; + kaha.setDirectory(new File("target/activemq-data/kahadb")); + kaha.deleteAllMessages(); + // Set the checkpoint interval to be very short so we can quickly + // check number of iterations + kaha.setCheckpointInterval(100); + + BrokerService broker = createBroker(kaha); + DefaultIOExceptionHandler ioExceptionHandler = new DefaultIOExceptionHandler(); + ioExceptionHandler.setStopStartConnectors(startStopConnectors); + broker.setIoExceptionHandler(ioExceptionHandler); + broker.start(); + + try { + if (startStopConnectors) { + // If startStopConnectors is true, the task should continue with future iterations + // as the SuppressReplyException that will be thrown is now handled so just verify + // we see 10 iterations which should happen quickly + assertTrue(Wait.waitFor(() -> iterations.get() == 10, 2000, 100)); + // broker should not be stopped + assertFalse(broker.isStopped()); + } else { + // If startStopConnectors is false, an IOException should shut down the broker + // which is the normal behavior + assertTrue(Wait.waitFor(broker::isStopped, 2000, 100)); + } + } finally { + broker.stop(); + } + } + private void assertExistsAndDelete(File file) { assertTrue(file.exists()); file.delete(); @@ -281,4 +348,4 @@ private String createContent(int i) { return sb.toString(); } -} \ No newline at end of file +} diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/plist/KahaDBFilePendingMessageCursorTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/plist/KahaDBFilePendingMessageCursorTest.java index ce4a8ed0e1c..481950e4d04 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/plist/KahaDBFilePendingMessageCursorTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/plist/KahaDBFilePendingMessageCursorTest.java @@ -29,6 +29,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * @author Hiram Chirino @@ -94,5 +96,86 @@ public void testAddRemoveAddIndexSize() throws Exception { assertEquals("expected page usage", initialPageCount -1, pageFile.getPageCount() - pageFile.getFreePageCount() ); } + // Test for AMQ-9726 + @Test + public void testClearCursor() throws Exception { + brokerService = new BrokerService(); + brokerService.setUseJmx(false); + SystemUsage usage = brokerService.getSystemUsage(); + usage.getMemoryUsage().setLimit(1024*150); + Destination dest = new Queue(brokerService, new ActiveMQQueue("Q"), null, new DestinationStatistics(), null); + dest.setMemoryUsage(usage.getMemoryUsage()); + brokerService.start(); + + underTest = new FilePendingMessageCursor(brokerService.getBroker(), "test", false); + underTest.setSystemUsage(usage); + + // Add 10 messages to the cursor in memory + addTestMessages(dest); + + // Verify memory usage was increased and cache is enabled + assertTrue(dest.getMemoryUsage().getUsage() > 0); + assertEquals(10, underTest.size()); + assertTrue(underTest.isCacheEnabled()); + assertEquals(0, dest.getTempUsage().getUsage()); + + // Clear, this will verify memory usage is correctly decremented + // and the memory map is cleared as well. Memory was previously + // incorrectly not being cleared. + underTest.clear(); + assertEquals(0, underTest.size()); + assertEquals(0, dest.getMemoryUsage().getUsage()); + + // Now test the disk cursor + // set the memory usage limit very small so messages will go to + // the disk list and not memory and send 10 more messages + usage.getMemoryUsage().setLimit(1); + addTestMessages(dest); + + // confirm the cache is false and the memory is 0 because + // the messages exist on disk and not in the memory map + // also very temp usage is greater than 0 now + assertFalse(underTest.isCacheEnabled()); + assertEquals(0, dest.getMemoryUsage().getUsage()); + assertTrue(dest.getTempUsage().getUsage() > 0); + assertEquals(10, underTest.size()); + + // Test clearing the disk list shows a size of 0 + underTest.clear(); + assertEquals(0, underTest.size()); + + // Send 10 more messages to verify that we can send again + // to the disk list after clear. Previously clear did not + // correctly destroy/reset the disk cursor so an exception + // was thrown when adding messages again after calling clear() + addTestMessages(dest); + assertFalse(underTest.isCacheEnabled()); + assertEquals(0, dest.getMemoryUsage().getUsage()); + assertTrue(dest.getTempUsage().getUsage() > 0); + assertEquals(10, underTest.size()); + + // one final clear() and reset limit to make sure we can send to + // memory again + underTest.clear(); + usage.getMemoryUsage().setLimit(1024*150); + assertEquals(0, underTest.size()); + assertEquals(0, dest.getMemoryUsage().getUsage()); + + // Verify memory usage was increased and cache is enabled + addTestMessages(dest); + assertTrue(dest.getMemoryUsage().getUsage() > 0); + assertEquals(10, underTest.size()); + assertTrue(underTest.isCacheEnabled()); + } + + private void addTestMessages(Destination dest) throws Exception { + for (int i = 0; i< 10; i++) { + ActiveMQMessage mqMessage = new ActiveMQMessage(); + mqMessage.setMessageId(new MessageId("1:2:3:" + i)); + mqMessage.setMemoryUsage(dest.getMemoryUsage()); + mqMessage.setRegionDestination(dest); + underTest.addMessageLast(new IndirectMessageReference(mqMessage)); + } + } } diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/usecases/DurableSubscriptionHangTestCase.java b/activemq-unit-tests/src/test/java/org/apache/activemq/usecases/DurableSubscriptionHangTestCase.java index bef912a514b..b802a9b5cdf 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/usecases/DurableSubscriptionHangTestCase.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/usecases/DurableSubscriptionHangTestCase.java @@ -30,6 +30,8 @@ import org.apache.activemq.broker.BrokerService; import org.apache.activemq.broker.region.policy.PolicyEntry; import org.apache.activemq.broker.region.policy.PolicyMap; +import org.apache.activemq.command.ActiveMQTopic; +import org.apache.activemq.util.Wait; import org.apache.commons.lang.RandomStringUtils; import org.junit.After; import org.junit.Before; @@ -39,6 +41,7 @@ import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; public class DurableSubscriptionHangTestCase { private static final Logger LOG = LoggerFactory.getLogger(DurableSubscriptionHangTestCase.class); @@ -55,7 +58,7 @@ public void startBroker() throws Exception { brokerService.setBrokerName(brokerName); PolicyMap policyMap = new PolicyMap(); PolicyEntry defaultEntry = new PolicyEntry(); - defaultEntry.setExpireMessagesPeriod(5000); + defaultEntry.setExpireMessagesPeriod(1000); policyMap.setDefaultEntry(defaultEntry); brokerService.setDestinationPolicy(policyMap); brokerService.start(); @@ -67,12 +70,13 @@ public void brokerStop() throws Exception { } @Test - public void testHanging() throws Exception - { + public void testHanging() throws Exception { registerDurableSubscription(); produceExpiredAndOneNonExpiredMessages(); - TimeUnit.SECONDS.sleep(10); // make sure messages are expired - Message message = collectMessagesFromDurableSubscriptionForOneMinute(); + assertTrue(Wait.waitFor(() -> brokerService.getDestination(new ActiveMQTopic(topicName)) + .getDestinationStatistics().getExpired().getCount() == 1000, 30000, 500)); + + Message message = getUnexpiredMessageFromDurableSubscription(); LOG.info("got message:" + message); assertNotNull("Unable to read unexpired message", message); } @@ -84,8 +88,7 @@ private void produceExpiredAndOneNonExpiredMessages() throws JMSException { Topic topic = session.createTopic(topicName); MessageProducer producer = session.createProducer(topic); producer.setTimeToLive(TimeUnit.SECONDS.toMillis(1)); - for(int i=0; i<40000; i++) - { + for(int i = 0; i < 1000; i++) { sendRandomMessage(session, producer); } producer.setTimeToLive(TimeUnit.DAYS.toMillis(1)); @@ -108,8 +111,7 @@ private void registerDurableSubscription() throws JMSException LOG.info("Durable Sub Registered"); } - private Message collectMessagesFromDurableSubscriptionForOneMinute() throws Exception - { + private Message getUnexpiredMessageFromDurableSubscription() throws Exception { ActiveMQConnectionFactory connectionFactory = new ActiveMQConnectionFactory("vm://" + brokerName); TopicConnection connection = connectionFactory.createTopicConnection(); @@ -119,7 +121,7 @@ private Message collectMessagesFromDurableSubscriptionForOneMinute() throws Exce connection.start(); TopicSubscriber subscriber = topicSession.createDurableSubscriber(topic, durableSubName); LOG.info("About to receive messages"); - Message message = subscriber.receive(120000); + Message message = subscriber.receive(1000); subscriber.close(); connection.close(); LOG.info("collectMessagesFromDurableSubscriptionForOneMinute done"); diff --git a/activemq-web-console/pom.xml b/activemq-web-console/pom.xml index 69e1f0d2d07..c1fee269624 100644 --- a/activemq-web-console/pom.xml +++ b/activemq-web-console/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-web-console diff --git a/activemq-web-console/src/main/webapp/403.html b/activemq-web-console/src/main/webapp/403.html index bfc95506f67..7503723b737 100644 --- a/activemq-web-console/src/main/webapp/403.html +++ b/activemq-web-console/src/main/webapp/403.html @@ -26,13 +26,7 @@ Apache ActiveMQ - - - - + @@ -54,10 +48,10 @@ @@ -75,7 +69,7 @@ -
+

Restricted!

diff --git a/activemq-web-console/src/main/webapp/404.html b/activemq-web-console/src/main/webapp/404.html index cf11458a4ed..98638913a92 100644 --- a/activemq-web-console/src/main/webapp/404.html +++ b/activemq-web-console/src/main/webapp/404.html @@ -26,13 +26,7 @@ Apache ActiveMQ - - - - + @@ -54,10 +48,10 @@ @@ -75,7 +69,7 @@ -
+

Page Not Found!

diff --git a/activemq-web-console/src/main/webapp/500.html b/activemq-web-console/src/main/webapp/500.html index f9305ff4ae7..ee6966bc7dc 100644 --- a/activemq-web-console/src/main/webapp/500.html +++ b/activemq-web-console/src/main/webapp/500.html @@ -26,13 +26,7 @@ Apache ActiveMQ - - - - + @@ -54,10 +48,10 @@ @@ -75,7 +69,7 @@ -
+

Error!

Exception occurred while processing this request, check the log for more information!

diff --git a/activemq-web-console/src/main/webapp/connections.jsp b/activemq-web-console/src/main/webapp/connections.jsp index 5663b358de7..951db78cbb3 100644 --- a/activemq-web-console/src/main/webapp/connections.jsp +++ b/activemq-web-console/src/main/webapp/connections.jsp @@ -57,7 +57,7 @@
-
+

Network Connectors

diff --git a/activemq-web-console/src/main/webapp/decorators/head.jsp b/activemq-web-console/src/main/webapp/decorators/head.jsp index abb76eaf741..6da545b73e8 100644 --- a/activemq-web-console/src/main/webapp/decorators/head.jsp +++ b/activemq-web-console/src/main/webapp/decorators/head.jsp @@ -20,17 +20,14 @@ <c:out value="${requestContext.brokerQuery.brokerAdmin.brokerName} : ${pageTitle}" /> - + + + - + diff --git a/activemq-web-console/src/main/webapp/decorators/header.jsp b/activemq-web-console/src/main/webapp/decorators/header.jsp index 99bd1d42dec..63674886621 100644 --- a/activemq-web-console/src/main/webapp/decorators/header.jsp +++ b/activemq-web-console/src/main/webapp/decorators/header.jsp @@ -33,9 +33,9 @@ @@ -71,5 +71,5 @@
- - + @@ -146,12 +154,12 @@ No message could be found for ID " onclick="return confirm('Are you sure you want to retry this message?')" - title="Retry - attempt reprocessing on original destination">Retry + title="Retry - attempt reprocessing on original destination">Retry - + +
+
diff --git a/activemq-web-console/src/main/webapp/js/head.js b/activemq-web-console/src/main/webapp/js/head.js new file mode 100644 index 00000000000..d78cd393e56 --- /dev/null +++ b/activemq-web-console/src/main/webapp/js/head.js @@ -0,0 +1,20 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +window.onload = function () { + addEvent(window, 'load', prettyPrint) +} diff --git a/activemq-web-console/src/main/webapp/js/message.js b/activemq-web-console/src/main/webapp/js/message.js new file mode 100644 index 00000000000..d45b70a4cb7 --- /dev/null +++ b/activemq-web-console/src/main/webapp/js/message.js @@ -0,0 +1,78 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +function sortSelect(selElem) { + const tmpAry = []; + for (var i=0;i 0) { + selElem.options[0] = null; + } + for (var i=0;i Apache ActiveMQ - Login - - - - + @@ -54,10 +48,10 @@ diff --git a/activemq-web-console/src/main/webapp/message.jsp b/activemq-web-console/src/main/webapp/message.jsp index 132d596d685..02f43fc5a81 100644 --- a/activemq-web-console/src/main/webapp/message.jsp +++ b/activemq-web-console/src/main/webapp/message.jsp @@ -133,8 +133,16 @@ No message could be found for ID + + + + + + +
" onclick="return confirm('Are you sure you want to delete the message?')" >Delete">Delete
')">CopyCopy ')" - >MoveMove
@@ -195,70 +201,10 @@ No message could be found for ID -function sortSelect(selElem) { - var tmpAry = new Array(); - for (var i=0;i 0) { - selElem.options[0] = null; - } - for (var i=0;i - - - - - "; - url = url + "&destination=" + encodeURIComponent(value); - - if (confirm("Are you sure?")) - location.href=url; -} - -window.onload=function() { - sortSelect( document.getElementById('queue') ); - selectOptionByText( document.getElementById('queue'), "-- Please select --" ); -} - - + <%@include file="decorators/footer.jsp" %> - diff --git a/activemq-web-console/src/main/webapp/network.jsp b/activemq-web-console/src/main/webapp/network.jsp index df2a08f3110..9ee9b7d7f26 100644 --- a/activemq-web-console/src/main/webapp/network.jsp +++ b/activemq-web-console/src/main/webapp/network.jsp @@ -25,7 +25,7 @@ <%@include file="decorators/header.jsp" %> -
+

Network Bridges

diff --git a/activemq-web-console/src/main/webapp/queueGraph.jsp b/activemq-web-console/src/main/webapp/queueGraph.jsp index ac8b7806a09..a427a287f68 100644 --- a/activemq-web-console/src/main/webapp/queueGraph.jsp +++ b/activemq-web-console/src/main/webapp/queueGraph.jsp @@ -28,36 +28,21 @@ + -<%@include file="decorators/header.jsp" %> + + + - +<%@include file="decorators/header.jsp" %> -
+
<%--- Other values we can graph... diff --git a/activemq-web-console/src/main/webapp/scheduled.jsp b/activemq-web-console/src/main/webapp/scheduled.jsp index cac68c39922..8c32a2f2729 100644 --- a/activemq-web-console/src/main/webapp/scheduled.jsp +++ b/activemq-web-console/src/main/webapp/scheduled.jsp @@ -29,7 +29,7 @@ -
+
@@ -63,7 +63,7 @@
-
+

Scheduler not started!

diff --git a/activemq-web-console/src/main/webapp/slave.jsp b/activemq-web-console/src/main/webapp/slave.jsp index 202d089d92e..8cb1e2a9df5 100644 --- a/activemq-web-console/src/main/webapp/slave.jsp +++ b/activemq-web-console/src/main/webapp/slave.jsp @@ -23,13 +23,7 @@ Apache ActiveMQ - - - - + @@ -51,10 +45,10 @@ @@ -72,7 +66,7 @@ -
+

Broker is currently in slave mode!

diff --git a/activemq-web-console/src/main/webapp/styles/head.css b/activemq-web-console/src/main/webapp/styles/head.css new file mode 100644 index 00000000000..9f46a641c29 --- /dev/null +++ b/activemq-web-console/src/main/webapp/styles/head.css @@ -0,0 +1,26 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* ===================================================== */ +/* Single place to import all css files used in head.jsp */ +/* ===================================================== */ + +@import url('sorttable.css'); +@import url('type-settings.css'); +@import url('site.css'); +@import url('prettify.css'); +@import url('header.css'); \ No newline at end of file diff --git a/activemq-web-console/src/main/webapp/styles/header.css b/activemq-web-console/src/main/webapp/styles/header.css new file mode 100644 index 00000000000..6b74b63ce03 --- /dev/null +++ b/activemq-web-console/src/main/webapp/styles/header.css @@ -0,0 +1,46 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* ============================== */ +/* Styles used in header.jsp */ +/* ============================== */ + +.header-logo-active-mq { + float:left; + width:280px; + display:block; + text-indent:-5000px; + text-decoration:none; + line-height:60px; + margin-top:10px; + margin-left:100px; +} + +.header-logo-apache { + float:right; + width:210px; + display:block; + text-indent:-5000px; + text-decoration:none; + line-height:60px; + margin-top:15px; + margin-right:10px +} + +.body-container { + overflow:hidden; +} \ No newline at end of file diff --git a/activemq-web-console/src/main/webapp/styles/site.css b/activemq-web-console/src/main/webapp/styles/site.css index 6ebcd0347ad..687a81bd723 100644 --- a/activemq-web-console/src/main/webapp/styles/site.css +++ b/activemq-web-console/src/main/webapp/styles/site.css @@ -19,6 +19,10 @@ body { padding: 20px; } +.section-container { + margin-top: 5em +} + /* ====================================================== */ /* Rounded Box Styles */ /* ====================================================== */ diff --git a/activemq-web-console/src/main/webapp/xml/queues.jsp b/activemq-web-console/src/main/webapp/xml/queues.jsp index 4e2ba876875..c7382a149a6 100644 --- a/activemq-web-console/src/main/webapp/xml/queues.jsp +++ b/activemq-web-console/src/main/webapp/xml/queues.jsp @@ -20,13 +20,10 @@ "> - - - diff --git a/activemq-web-demo/pom.xml b/activemq-web-demo/pom.xml index 4d3f6d73118..5fe703ade64 100644 --- a/activemq-web-demo/pom.xml +++ b/activemq-web-demo/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-web-demo diff --git a/activemq-web/pom.xml b/activemq-web/pom.xml index b22f976da66..326c3f6cb83 100644 --- a/activemq-web/pom.xml +++ b/activemq-web/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT activemq-web diff --git a/activemq-web/src/main/java/org/apache/activemq/web/MessageServletSupport.java b/activemq-web/src/main/java/org/apache/activemq/web/MessageServletSupport.java index d038295f9f2..643f2deb180 100644 --- a/activemq-web/src/main/java/org/apache/activemq/web/MessageServletSupport.java +++ b/activemq-web/src/main/java/org/apache/activemq/web/MessageServletSupport.java @@ -358,7 +358,7 @@ protected String getPostedMessageBody(HttpServletRequest request) throws IOExcep if (answer == null && contentType != null && contentLengthLong > -1l) { LOG.debug("Content-Type={} Content-Length={} maxMessageSize={}", contentType, contentLengthLong, maxMessageSize); - if (contentLengthLong > maxMessageSize) { + if (maxMessageSize != -1 && contentLengthLong > maxMessageSize) { LOG.warn("Message body exceeds max allowed size. Content-Type={} Content-Length={} maxMessageSize={}", contentType, contentLengthLong, maxMessageSize); throw new IOException("Message body exceeds max allowed size"); } @@ -397,6 +397,6 @@ protected String getSelector(HttpServletRequest request) throws IOException { } private boolean isMaxBodySizeExceeded(int totalRead, int expectedBodySize) { - return totalRead < 0 || totalRead >= Integer.MAX_VALUE || totalRead >= maxMessageSize || totalRead > expectedBodySize; + return totalRead < 0 || totalRead == Integer.MAX_VALUE || (maxMessageSize != -1 && totalRead >= maxMessageSize) || totalRead > expectedBodySize; } } diff --git a/assembly/pom.xml b/assembly/pom.xml index 5515d034da4..d1f063d31cf 100644 --- a/assembly/pom.xml +++ b/assembly/pom.xml @@ -22,7 +22,7 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT apache-activemq diff --git a/assembly/src/release/RELEASE-NOTES-EAP-5.19.x b/assembly/src/release/RELEASE-NOTES-EAP-5.19.x index 29da78ae41f..3b588bfce55 100644 --- a/assembly/src/release/RELEASE-NOTES-EAP-5.19.x +++ b/assembly/src/release/RELEASE-NOTES-EAP-5.19.x @@ -1,5 +1,8 @@ Apache ActiveMQ 5.19.x-TT.x +Changes in ActiveMQ EAP 5.19.2-TT.1 +- Merged upstream Apache ActiveMQ 5.19.1 release changes (broker/store fixes, web console updates, and test improvements) + Changes in ActiveMQ EAP 5.19.1-TT.4 - CVE-2025-41249 Spring Framework Annotation Detection Vulnerability diff --git a/assembly/src/release/bin/activemq b/assembly/src/release/bin/activemq index 130517c6a0e..2b2650fb18c 100755 --- a/assembly/src/release/bin/activemq +++ b/assembly/src/release/bin/activemq @@ -112,7 +112,7 @@ if [ -z "$ACTIVEMQ_USER_CLASSPATH" ] ; then fi # ActiveMQ Classpath configuration -ACTIVEMQ_CLASSPATH="${ACTIVEMQ_BASE%/}/../lib/:$ACTIVEMQ_USER_CLASSPATH" +ACTIVEMQ_CLASSPATH="$ACTIVEMQ_USER_CLASSPATH" # Active MQ configuration directory if [ -z "$ACTIVEMQ_CONF" ] ; then diff --git a/assembly/src/release/conf/jetty.xml b/assembly/src/release/conf/jetty.xml index 6f09f6ea2e9..bd8170342dc 100644 --- a/assembly/src/release/conf/jetty.xml +++ b/assembly/src/release/conf/jetty.xml @@ -73,6 +73,17 @@ + + + + + + + + + + + diff --git a/assembly/src/release/webapps/index.html b/assembly/src/release/webapps/index.html index 08332609622..cdfac1b98ae 100644 --- a/assembly/src/release/webapps/index.html +++ b/assembly/src/release/webapps/index.html @@ -25,14 +25,9 @@ - - Apache ActiveMQ - + + Apache ActiveMQ + @@ -54,10 +49,10 @@ @@ -75,7 +70,7 @@ -
+

Welcome to the Apache ActiveMQ!

diff --git a/pom.xml b/pom.xml index 0b17b590e10..9c881a07dde 100644 --- a/pom.xml +++ b/pom.xml @@ -25,14 +25,14 @@ org.apache.activemq activemq-parent - 5.19.1-TT.5-SNAPSHOT + 5.19.2-TT.1-SNAPSHOT pom ActiveMQ 2005 - 2025-10-15T22:47:04Z + 2025-10-08T12:16:52Z 3.1.4 activemq-${project.version} @@ -53,8 +53,8 @@ 3.2.2-TT.1 1.4.1 2.13.0 - 2.18.0 - 3.14.0 + 2.20.0 + 3.18.0 1.3.5 2.12.1 1.0 @@ -69,12 +69,13 @@ 1.2.0.Beta4 2.0.3 3.1.0 - 2.18.3 + 2.20.0 + 2.20 1.9.3 2.3.2_1 - 9.4.57.v20241219 + 9.4.58.v20250814 ${jetty9-version} - 3.6.0 + 3.6.2 9.0.100 3.30.2-GA 1.5.4 @@ -107,7 +108,7 @@ 2.4.1 1.1.4c 1.4.21 - 4.26 + 4.27 2.12.2 0.12.0 1.19 @@ -488,7 +489,7 @@ org.ow2.asm asm - 9.7.1 + 9.9 @@ -718,7 +719,7 @@ com.fasterxml.jackson.core jackson-annotations - ${jackson-version} + ${jackson-annotations-version} com.fasterxml.jackson.core