diff --git a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/collections/AbstractHashMapPersister.java b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/collections/AbstractHashMapPersister.java index 16cbfe657aba..f644008f9d7b 100644 --- a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/collections/AbstractHashMapPersister.java +++ b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/collections/AbstractHashMapPersister.java @@ -22,7 +22,7 @@ import org.apache.activemq.artemis.core.persistence.Persister; import org.apache.activemq.artemis.utils.DataConstants; -public abstract class AbstractHashMapPersister implements Persister> { +public abstract class AbstractHashMapPersister implements Persister> { private final byte VERSION = 0; @@ -32,14 +32,20 @@ public byte getID() { } @Override - public final int getEncodeSize(JournalHashMap.MapRecord record) { + public final int getEncodeSize(JournalHashMap.MapRecord record) { return DataConstants.SIZE_LONG + // recordID DataConstants.SIZE_BYTE + // Version - DataConstants.SIZE_LONG + // collectionID + getCollectionIdSize(record.collectionID) + getKeySize(record.key) + getValueSize(record.value); } + protected abstract int getCollectionIdSize(I collectionID); + + protected abstract void encodeCollectionId(ActiveMQBuffer buffer, I collectionID); + + protected abstract I decodeCollectionId(ActiveMQBuffer buffer); + protected abstract int getKeySize(K key); protected abstract void encodeKey(ActiveMQBuffer buffer, K key); @@ -53,28 +59,28 @@ public final int getEncodeSize(JournalHashMap.MapRecord record) { protected abstract V decodeValue(ActiveMQBuffer buffer, K key); @Override - public final void encode(ActiveMQBuffer buffer, JournalHashMap.MapRecord record) { + public final void encode(ActiveMQBuffer buffer, JournalHashMap.MapRecord record) { buffer.writeLong(record.id); buffer.writeByte(VERSION); - buffer.writeLong(record.collectionID); + encodeCollectionId(buffer, record.collectionID); encodeKey(buffer, record.key); encodeValue(buffer, record.value); } @Override - public final JournalHashMap.MapRecord decode(ActiveMQBuffer buffer, - JournalHashMap.MapRecord record, + public final JournalHashMap.MapRecord decode(ActiveMQBuffer buffer, + JournalHashMap.MapRecord record, CoreMessageObjectPools pool) { long id = buffer.readLong(); byte version = buffer.readByte(); assert version == VERSION; - long collectionID = buffer.readLong(); + I collectionID = decodeCollectionId(buffer); K key = decodeKey(buffer); V value = decodeValue(buffer, key); - JournalHashMap.MapRecord mapRecord = new JournalHashMap.MapRecord<>(collectionID, id, key, value); + JournalHashMap.MapRecord mapRecord = new JournalHashMap.MapRecord<>(collectionID, id, key, value); return mapRecord; } } diff --git a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/collections/JournalHashMap.java b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/collections/JournalHashMap.java index a0a7a7b3e617..12dd0cc3b085 100644 --- a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/collections/JournalHashMap.java +++ b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/collections/JournalHashMap.java @@ -27,7 +27,7 @@ import java.util.Objects; import java.util.Set; import java.util.function.BiConsumer; -import java.util.function.LongFunction; +import java.util.function.Function; import java.util.function.LongSupplier; import java.util.function.Supplier; @@ -38,21 +38,22 @@ import org.slf4j.LoggerFactory; /** + * I = Collection ID type * K = Key * V = Value * C = Context */ -public class JournalHashMap implements Map { +public class JournalHashMap implements Map { private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - public static class MapRecord implements Entry { - final long collectionID; + public static class MapRecord implements Entry { + final I collectionID; final long id; final K key; V value; - MapRecord(long collectionID, long id, K key, V value) { + MapRecord(I collectionID, long id, K key, V value) { this.collectionID = collectionID; this.id = id; this.key = key; @@ -86,11 +87,11 @@ public boolean equals(Object obj) { if (this == obj) { return true; } - if (!(obj instanceof MapRecord other)) { + if (!(obj instanceof MapRecord other)) { return false; } - return collectionID == other.collectionID && + return Objects.equals(collectionID, other.collectionID) && id == other.id && Objects.equals(key, other.key) && Objects.equals(value, other.value); @@ -102,7 +103,7 @@ public int hashCode() { } } - public JournalHashMap(long collectionId, MapStorageManager journal, LongSupplier idGenerator, Persister> persister, byte recordType, Supplier completionSupplier, LongFunction contextProvider, IOCriticalErrorListener ioExceptionListener) { + public JournalHashMap(I collectionId, MapStorageManager journal, LongSupplier idGenerator, Persister> persister, byte recordType, Supplier completionSupplier, Function contextProvider, IOCriticalErrorListener ioExceptionListener) { this.collectionId = collectionId; this.journal = journal; this.idGenerator = idGenerator; @@ -115,13 +116,13 @@ public JournalHashMap(long collectionId, MapStorageManager journal, LongSupplier C context; - LongFunction contextProvider; + Function contextProvider; - private final Persister> persister; + private final Persister> persister; private final MapStorageManager journal; - private final long collectionId; + private final I collectionId; private final byte recordType; @@ -131,9 +132,9 @@ public JournalHashMap(long collectionId, MapStorageManager journal, LongSupplier private final IOCriticalErrorListener exceptionListener; - private final Map> map = new HashMap<>(); + private final Map> map = new HashMap<>(); - public long getCollectionId() { + public I getCollectionId() { return collectionId; } @@ -149,7 +150,7 @@ public C getContext() { return context; } - public JournalHashMap setContext(C context) { + public JournalHashMap setContext(C context) { this.context = context; return this; } @@ -166,7 +167,7 @@ public synchronized boolean containsKey(Object key) { @Override public synchronized boolean containsValue(Object value) { - for (Entry> entry : map.entrySet()) { + for (Entry> entry : map.entrySet()) { if (value.equals(entry.getValue().value)) { return true; } @@ -176,7 +177,7 @@ public synchronized boolean containsValue(Object value) { @Override public synchronized V get(Object key) { - MapRecord record = map.get(key); + MapRecord record = map.get(key); if (record == null) { return null; } else { @@ -187,7 +188,7 @@ public synchronized V get(Object key) { /** * This is to be called from a single thread during reload, no need to be synchronized */ - public void reload(MapRecord reloadValue) { + public void reload(MapRecord reloadValue) { map.put(reloadValue.getKey(), reloadValue); } @@ -195,9 +196,9 @@ public void reload(MapRecord reloadValue) { public synchronized V put(K key, V value) { logger.debug("adding {} = {}", key, value); long id = idGenerator.getAsLong(); - MapRecord record = new MapRecord<>(collectionId, id, key, value); + MapRecord record = new MapRecord<>(collectionId, id, key, value); store(record); - MapRecord oldRecord = map.put(key, record); + MapRecord oldRecord = map.put(key, record); if (oldRecord != null) { removed(oldRecord); @@ -208,13 +209,12 @@ public synchronized V put(K key, V value) { } - private synchronized void store(MapRecord record) { + private synchronized void store(MapRecord record) { try { IOCompletion callback = null; if (completionSupplier != null) { callback = completionSupplier.get(); } - if (callback == null) { journal.storeMapRecord(record.id, recordType, persister, record, false); } else { @@ -227,7 +227,7 @@ private synchronized void store(MapRecord record) { } // callers must be synchronized - private void removed(MapRecord record) { + private void removed(MapRecord record) { if (logger.isTraceEnabled()) { logger.trace("Removing record {}", record); } @@ -239,7 +239,7 @@ private void removed(MapRecord record) { } // callers must be synchronized - private void removed(MapRecord record, long txid) { + private void removed(MapRecord record, long txid) { try { journal.deleteMapRecordTx(txid, record.id); } catch (Exception e) { @@ -249,7 +249,10 @@ private void removed(MapRecord record, long txid) { @Override public synchronized V remove(Object key) { - MapRecord record = map.remove(key); + MapRecord record = map.remove(key); + if (record == null) { + return null; + } this.removed(record); return record.value; } @@ -261,7 +264,10 @@ public synchronized V remove(Object key) { * not expected. */ public synchronized V remove(Object key, long transactionID) { - MapRecord record = map.remove(key); + MapRecord record = map.remove(key); + if (record == null) { + return null; + } this.removed(record, transactionID); return record.value; } diff --git a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/collections/JournalHashMapProvider.java b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/collections/JournalHashMapProvider.java index 675d55aefbca..560d5cbd4643 100644 --- a/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/collections/JournalHashMapProvider.java +++ b/artemis-journal/src/main/java/org/apache/activemq/artemis/core/journal/collections/JournalHashMapProvider.java @@ -18,30 +18,31 @@ package org.apache.activemq.artemis.core.journal.collections; import java.util.ArrayList; +import java.util.HashMap; import java.util.Iterator; import java.util.List; -import java.util.function.LongFunction; +import java.util.Map; +import java.util.function.Function; import java.util.function.LongSupplier; import java.util.function.Supplier; -import io.netty.util.collection.LongObjectHashMap; import org.apache.activemq.artemis.core.io.IOCriticalErrorListener; import org.apache.activemq.artemis.core.journal.IOCompletion; import org.apache.activemq.artemis.core.journal.RecordInfo; import org.apache.activemq.artemis.core.persistence.Persister; -public class JournalHashMapProvider { +public class JournalHashMapProvider { final MapStorageManager journal; - final Persister> persister; - final LongObjectHashMap> journalMaps = new LongObjectHashMap<>(); + final Persister> persister; + final Map> journalMaps = new HashMap<>(); final LongSupplier idSupplier; final byte recordType; final IOCriticalErrorListener ioExceptionListener; final Supplier ioCompletionSupplier; - final LongFunction contextProvider; + final Function contextProvider; - public JournalHashMapProvider(LongSupplier idSupplier, MapStorageManager journal, AbstractHashMapPersister persister, byte recordType, Supplier ioCompletionSupplier, LongFunction contextProvider, IOCriticalErrorListener ioExceptionListener) { + public JournalHashMapProvider(LongSupplier idSupplier, MapStorageManager journal, AbstractHashMapPersister persister, byte recordType, Supplier ioCompletionSupplier, Function contextProvider, IOCriticalErrorListener ioExceptionListener) { this.idSupplier = idSupplier; this.persister = persister; this.journal = journal; @@ -51,10 +52,8 @@ public JournalHashMapProvider(LongSupplier idSupplier, MapStorageManager journal this.ioCompletionSupplier = ioCompletionSupplier; } - public List> getMaps() { - List> maps = new ArrayList<>(); - journalMaps.values().forEach(maps::add); - return maps; + public List> getMaps() { + return new ArrayList<>(journalMaps.values()); } public void clear() { @@ -62,16 +61,16 @@ public void clear() { } public void reload(RecordInfo recordInfo) { - JournalHashMap.MapRecord mapRecord = persister.decode(recordInfo.wrapData(), null, null); + JournalHashMap.MapRecord mapRecord = persister.decode(recordInfo.wrapData(), null, null); getMap(mapRecord.collectionID, null).reload(mapRecord); } - public Iterator> iterMaps() { + public Iterator> iterMaps() { return journalMaps.values().iterator(); } - public synchronized JournalHashMap getMap(long collectionID, C context) { - JournalHashMap journalHashMap = journalMaps.get(collectionID); + public synchronized JournalHashMap getMap(I collectionID, C context) { + JournalHashMap journalHashMap = journalMaps.get(collectionID); if (journalHashMap == null) { journalHashMap = new JournalHashMap<>(collectionID, journal, idSupplier, persister, recordType, ioCompletionSupplier, contextProvider, ioExceptionListener).setContext(context); journalMaps.put(collectionID, journalHashMap); @@ -79,7 +78,11 @@ public synchronized JournalHashMap getMap(long collectionID, C context) return journalHashMap; } - public JournalHashMap getMap(long collectionID) { + public JournalHashMap getMap(I collectionID) { return getMap(collectionID, null); } + + public boolean containsMap(I collectionID) { + return journalMaps.containsKey(collectionID); + } } diff --git a/artemis-pom/pom.xml b/artemis-pom/pom.xml index f4d8d7c0b4f5..003d220496b2 100644 --- a/artemis-pom/pom.xml +++ b/artemis-pom/pom.xml @@ -111,6 +111,11 @@ org.eclipse.paho.mqttv5.client ${paho.client.mqtt.version} + + com.hivemq + hivemq-mqtt-client + ${hive.client.mqtt.version} + org.fusesource.mqtt-client mqtt-client diff --git a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/mirror/AckManager.java b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/mirror/AckManager.java index 1165d94c8019..2dfa80b7056c 100644 --- a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/mirror/AckManager.java +++ b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/mirror/AckManager.java @@ -72,7 +72,7 @@ public class AckManager implements ActiveMQComponent { final Set mirrorControllerTargets = new HashSet<>(); final LongSupplier sequenceGenerator; - final JournalHashMapProvider journalHashMapProvider; + final JournalHashMapProvider journalHashMapProvider; final ActiveMQServer server; final Configuration configuration; final ReferenceIDSupplier referenceIDSupplier; @@ -94,13 +94,15 @@ public AckManager(ActiveMQServer server) { this.sequenceGenerator = server.getStorageManager()::generateID; this.mirrorRegistry = server.getMirrorRegistry(); // The JournalHashMap has to use the storage manager to guarantee we are using the Replicated Journal Wrapper in case this is a replicated journal - journalHashMapProvider = new JournalHashMapProvider<>(sequenceGenerator, server.getStorageManager(), AckRetry.getPersister(), JournalRecordIds.ACK_RETRY, OperationContextImpl::getContext, server.getPostOffice()::findQueue, server.getIoCriticalErrorListener()); + journalHashMapProvider = new JournalHashMapProvider<>(sequenceGenerator, server.getStorageManager(), AckRetry.getPersister(), JournalRecordIds.ACK_RETRY, OperationContextImpl::getContext, id -> server.getPostOffice().findQueue(id), server.getIoCriticalErrorListener()); this.referenceIDSupplier = new ReferenceIDSupplier(server); } public void reload(RecordInfo recordInfo) { - journalHashMapProvider.reload(recordInfo); - mirrorRegistry.incrementMirrorAckSize(); + if (recordInfo.userRecordType == JournalRecordIds.ACK_RETRY) { + journalHashMapProvider.reload(recordInfo); + mirrorRegistry.incrementMirrorAckSize(); + } } @Override @@ -177,7 +179,7 @@ public boolean initRetry() { return false; } - Map>> retries = sortRetries(); + Map>> retries = sortRetries(); flushMirrorTargets(); @@ -216,20 +218,20 @@ private synchronized List copyTargets() { // Sort the ACK list by address // We have the retries by queue, we need to sort them by address // as we will perform all the retries on the same addresses at the same time (in the Multicast case with multiple queues acking) - public Map>> sortRetries() { + public Map>> sortRetries() { // We will group the retries by address, // so we perform all of the queues in the same address at once - Map>> retriesByAddress = new HashMap<>(); + Map>> retriesByAddress = new HashMap<>(); - Iterator> queueRetriesIterator = journalHashMapProvider.getMaps().iterator(); + Iterator> queueRetriesIterator = journalHashMapProvider.getMaps().iterator(); while (queueRetriesIterator.hasNext()) { - JournalHashMap ackRetries = queueRetriesIterator.next(); + JournalHashMap ackRetries = queueRetriesIterator.next(); if (!ackRetries.isEmpty()) { Queue queue = ackRetries.getContext(); if (queue != null) { SimpleString address = queue.getAddress(); - LongObjectHashMap> queueRetriesOnAddress = retriesByAddress.get(address); + LongObjectHashMap> queueRetriesOnAddress = retriesByAddress.get(address); if (queueRetriesOnAddress == null) { queueRetriesOnAddress = new LongObjectHashMap<>(); retriesByAddress.put(address, queueRetriesOnAddress); @@ -254,7 +256,7 @@ private boolean isSnapshotComplete(LongObjectHashMap pendingSnaps } // to be used with the same executor as the PagingStore executor - public void retryAddress(SimpleString address, LongObjectHashMap> acksToRetry) { + public void retryAddress(SimpleString address, LongObjectHashMap> acksToRetry) { // This is an optimization: @@ -329,7 +331,7 @@ public void retryAddress(SimpleString address, LongObjectHashMap buildCounterSnapshot(LongObjectHashMap> acksToRetry) { + private static LongObjectHashMap buildCounterSnapshot(LongObjectHashMap> acksToRetry) { LongObjectHashMap snapshotCount = new LongObjectHashMap<>(); acksToRetry.forEach((l, map) -> { AtomicInteger recordCount = new AtomicInteger(0); @@ -354,11 +356,11 @@ private Page openPage(PagingStore store, long pageID) throws Throwable { } - private void validateExpiredSet(SimpleString address, LongObjectHashMap> queuesToRetry) { + private void validateExpiredSet(SimpleString address, LongObjectHashMap> queuesToRetry) { queuesToRetry.forEach((q, r) -> this.validateExpireSet(address, q, r)); } - private void validateExpireSet(SimpleString address, long queueID, JournalHashMap retries) { + private void validateExpireSet(SimpleString address, long queueID, JournalHashMap retries) { for (AckRetry retry : retries.valuesCopy()) { // we only remove or configure to be removed if the retry was initially seen on the start of the process // this is to avoid a race where an ACK entered the list after the scan been through where the element was supposed to be @@ -385,7 +387,7 @@ private void validateExpireSet(SimpleString address, long queueID, JournalHashMa } private void retryPage(LongObjectHashMap snapshotCount, - LongObjectHashMap> queuesToRetry, + LongObjectHashMap> queuesToRetry, SimpleString address, Page page) throws Exception { @@ -395,7 +397,7 @@ private void retryPage(LongObjectHashMap snapshotCount, page.getMessages().forEach(pagedMessage -> { for (int i = 0; i < pagedMessage.getQueueIDs().length; i++) { long queueID = pagedMessage.getQueueIDs()[i]; - JournalHashMap retries = queuesToRetry.get(queueID); + JournalHashMap retries = queuesToRetry.get(queueID); AtomicInteger snapshotOnQueue = snapshotCount.get(queueID); if (retries != null) { String serverID = referenceIDSupplier.getServerID(pagedMessage.getMessage()); @@ -468,13 +470,13 @@ private void decrementSnapshotCount(AckRetry retry, AtomicInteger queueSnapshotC /** * {@return {@code true} if there are retries ready to be scanned on paging} */ - private boolean checkRetriesAndPaging(LongObjectHashMap> queuesToRetry, LongObjectHashMap snapshotCount) { + private boolean checkRetriesAndPaging(LongObjectHashMap> queuesToRetry, LongObjectHashMap snapshotCount) { boolean needScanOnPaging = false; - Iterator>> iter = queuesToRetry.entrySet().iterator(); + Iterator>> iter = queuesToRetry.entrySet().iterator(); while (iter.hasNext()) { - Map.Entry> entry = iter.next(); - JournalHashMap queueRetries = entry.getValue(); + Map.Entry> entry = iter.next(); + JournalHashMap queueRetries = entry.getValue(); Queue queue = queueRetries.getContext(); AtomicInteger queueSnapshotCount = snapshotCount.get(queue.getID()); for (AckRetry retry : queueRetries.valuesCopy()) { @@ -571,12 +573,12 @@ private void doACK(Queue targetQueue, MessageReference reference, AckReason reas * It will perform each address individually, one by one. */ class MultiStepProgress { - Map>> retryList; + Map>> retryList; - Iterator>>> retryIterator; + Iterator>>> retryIterator; - MultiStepProgress(Map>> retryList) { + MultiStepProgress(Map>> retryList) { this.retryList = retryList; retryIterator = retryList.entrySet().iterator(); } @@ -587,7 +589,7 @@ public void nextStep() { logger.trace("Iterator is done on retry, server={}", server); AckManager.this.endRetry(); } else { - Map.Entry>> entry = retryIterator.next(); + Map.Entry>> entry = retryIterator.next(); ////////////////////////////////////////////////////////////////////// // Issue a deliverAsync on each queue before doing the retries @@ -607,7 +609,7 @@ public void nextStep() { } } - private void deliveryAsync(JournalHashMap map) { + private void deliveryAsync(JournalHashMap map) { Queue queue = map.getContext(); if (queue != null) { queue.deliverAsync(); diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/CoreDeliveryInfo.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/CoreDeliveryInfo.java new file mode 100644 index 000000000000..853210c74050 --- /dev/null +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/CoreDeliveryInfo.java @@ -0,0 +1,79 @@ +/* + * 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.artemis.core.protocol.mqtt; + +import java.util.Objects; + +import org.apache.activemq.artemis.api.core.SimpleString; + +/** + * Tracks volatile, in-memory state for an in-flight MQTT delivery: the consumer that originated it and the + * {@link PacketIdCorrelationKey} that identifies the underlying core message and address. This information is held only + * for the lifetime of the connection and is discarded on disconnect, whereas the {@link PacketIdCorrelationKey} mapping + * is persisted in the journal so that packet IDs can be correlated across reconnects. + */ +public class CoreDeliveryInfo { + private long consumerId; + private PacketIdCorrelationKey packetIdCorrelationKey; + + public static CoreDeliveryInfo of(long consumerId, PacketIdCorrelationKey packetIdCorrelationKey) { + return new CoreDeliveryInfo(consumerId, packetIdCorrelationKey); + } + + private CoreDeliveryInfo(long consumerId, PacketIdCorrelationKey packetIdCorrelationKey) { + this.consumerId = consumerId; + this.packetIdCorrelationKey = packetIdCorrelationKey; + } + + public long getConsumerId() { + return consumerId; + } + + public PacketIdCorrelationKey getPacketIdCorrelationKey() { + return packetIdCorrelationKey; + } + + public long getCoreMessageId() { + return packetIdCorrelationKey.getCoreMessageId(); + } + + public SimpleString getAddress() { + return packetIdCorrelationKey.getAddress(); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof CoreDeliveryInfo other)) { + return false; + } + return consumerId == other.consumerId && + Objects.equals(packetIdCorrelationKey, other.packetIdCorrelationKey); + } + + @Override + public int hashCode() { + return Objects.hash(consumerId, packetIdCorrelationKey); + } + + @Override + public String toString() { + return "CoreDeliveryInfo[" + "consumerId=" + consumerId + ", packetIdCorrelationKey=" + packetIdCorrelationKey + "]"; + } +} diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTBundle.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTBundle.java index 42e5d472e5c4..88563f636bde 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTBundle.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTBundle.java @@ -33,4 +33,7 @@ public interface MQTTBundle { @Message(id = 850001, value = "Unable to generate MQTT packet ID. All valid values between 1 and 65535 are in use. IDs will become available as messages are acknowledged by the client that has received them.") IllegalStateException unableToGenerateID(); + + @Message(id = 850002, value = "StorageManager is null") + IllegalStateException storageManagerIsNull(); } diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnection.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnection.java index 5184f3346853..55ffe6a18176 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnection.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnection.java @@ -40,7 +40,7 @@ public class MQTTConnection extends AbstractRemotingConnection { private boolean clientIdAssignedByBroker = false; - public MQTTConnection(Connection transportConnection) throws Exception { + public MQTTConnection(Connection transportConnection) { super(transportConnection, null); this.destroyed = false; } diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnectionManager.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnectionManager.java index 1a4031262d4f..40194d2de857 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnectionManager.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnectionManager.java @@ -123,10 +123,21 @@ synchronized void connect(MqttConnectMessage connect, String validatedUser, Stri connackProperties = MqttProperties.NO_PROPERTIES; } + if (sessionState.getClientSessionExpiryInterval() == -1 || sessionState.getClientSessionExpiryInterval() > 0) { + session.getStateManager().storeDurableState(sessionState); + } + session.getConnection().setConnected(true); session.getProtocolHandler().sendConnack(MQTTReasonCodes.SUCCESS, sessionPresent && !cleanStart, connackProperties); - // ensure we don't publish before the CONNACK - session.start(); + // [MQTT-3.2.0-1] the CONNACK is sent via IO callback so the session should be started the same way to avoid a race + session.getProtocolHandler().runAfterStorageOperations(() -> { + try { + session.start(); + } catch (Exception e) { + MQTTLogger.LOGGER.errorDisconnectingClient(e); + disconnect(true); + } + }); } private MqttProperties getConnackProperties() { diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTLogger.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTLogger.java index 7a2ef08b2cb7..b39baa360e3a 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTLogger.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTLogger.java @@ -29,11 +29,11 @@ public interface MQTTLogger { MQTTLogger LOGGER = BundleFactory.newBundle(MQTTLogger.class, MQTTLogger.class.getPackage().getName()); - @LogMessage(id = 832000, value = "Unable to send message: {}", level = LogMessage.Level.WARN) - void unableToSendMessage(MessageReference message, Exception e); + @LogMessage(id = 832000, value = "Unable to send message to MQTT client {}: {}", level = LogMessage.Level.WARN) + void unableToSendMessage(String clientId, MessageReference message, Exception e); - @LogMessage(id = 832001, value = "MQTT client({}) attempted to ack already ack'd message: ", level = LogMessage.Level.WARN) - void failedToAckMessage(String clientId, Exception e); + @LogMessage(id = 832001, value = "MQTT client {} failed to acknowledge message: {}", level = LogMessage.Level.WARN) + void failedToAckMessage(String clientId, String exceptionMessage); @LogMessage(id = 834000, value = "Error removing subscription.", level = LogMessage.Level.ERROR) void errorRemovingSubscription(Exception e); @@ -41,8 +41,8 @@ public interface MQTTLogger { @LogMessage(id = 834001, value = "Error disconnecting client.", level = LogMessage.Level.ERROR) void errorDisconnectingClient(Exception e); - @LogMessage(id = 834002, value = "Error processing control packet: {}", level = LogMessage.Level.ERROR) - void errorProcessingControlPacket(String packet, Exception e); + @LogMessage(id = 834002, value = "Error processing MQTT packet; client ID: {}; packet: {}; {}", level = LogMessage.Level.ERROR) + void errorProcessingPacket(String clientId, String packet, String exceptionMessage, Exception e); @LogMessage(id = 834003, value = "Error sending will message.", level = LogMessage.Level.ERROR) void errorSendingWillMessage(Exception e); @@ -53,8 +53,8 @@ public interface MQTTLogger { @LogMessage(id = 834005, value = "Failed to cast property {}.", level = LogMessage.Level.ERROR) void failedToCastProperty(String property); - @LogMessage(id = 834006, value = "Failed to publish MQTT message: {}.", level = LogMessage.Level.ERROR) - void failedToPublishMqttMessage(String exceptionMessage, Throwable t); + @LogMessage(id = 834006, value = "Failed to publish MQTT message. Client ID: {}. Packet ID: {}. Exception message: {}", level = LogMessage.Level.ERROR) + void failedToPublishMqttMessage(String clientId, int packetId, String exceptionMessage, Throwable t); @LogMessage(id = 834007, value = "Authorization failure sending will message: {}", level = LogMessage.Level.ERROR) void authorizationFailureSendingWillMessage(String message); @@ -62,8 +62,8 @@ public interface MQTTLogger { @LogMessage(id = 834008, value = "Failed to remove session state for client with ID: {}", level = LogMessage.Level.ERROR) void failedToRemoveSessionState(String clientID, Exception e); - @LogMessage(id = 834009, value = "Ignoring duplicate MQTT QoS2 PUBLISH packet for packet ID {} from client with ID {}.", level = LogMessage.Level.WARN) - void ignoringQoS2Publish(String clientId, long packetId); + @LogMessage(id = 834009, value = "Ignoring duplicate MQTT QoS2 PUBLISH; packet ID: {}; client ID: {}.", level = LogMessage.Level.WARN) + void ignoringQoS2Publish(long packetId, String clientId); @LogMessage(id = 834010, value = "Unable to scan MQTT sessions", level = LogMessage.Level.ERROR) void unableToScanSessions(Exception e); @@ -76,4 +76,13 @@ public interface MQTTLogger { @LogMessage(id = 834013, value = "Invalid MQTT session state message. Will not load this state into memory.", level = LogMessage.Level.WARN) void errorDeserializingStateMessage(Exception e); + + @LogMessage(id = 834014, value = "MQTT client {} sent PUBREC for packet {}, but acknowledgement failed. Internal consumer {} not found. Internal session is {}.", level = LogMessage.Level.WARN) + void failedToAckMessageConsumerNotFound(String clientId, int packetId, long consumerId, String closed); + + @LogMessage(id = 834015, value = "Unable to handle MQTT packet [{}] from {}. Internal session is closed.", level = LogMessage.Level.ERROR) + void internalSessionClosed(String packet, String clientId); + + @LogMessage(id = 834016, value = "Storage operation failed. Error code: {}; message: {}", level = LogMessage.Level.ERROR) + void storageOperationError(int errorCode, String errorMessage); } diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTMessageInfo.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTMessageInfo.java deleted file mode 100644 index f25640008e6f..000000000000 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTMessageInfo.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * 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.artemis.core.protocol.mqtt; - -/** - * MQTT Acks only hold message ID information. From this we must infer the internal message ID and consumer. - */ -class MQTTMessageInfo { - - private long serverMessageId; - - private long consumerId; - - private String address; - - MQTTMessageInfo(long serverMessageId, long consumerId, String address) { - this.serverMessageId = serverMessageId; - this.consumerId = consumerId; - this.address = address; - } - - long getServerMessageId() { - return serverMessageId; - } - - long getConsumerId() { - return consumerId; - } - - String getAddress() { - return address; - } - - @Override - public String toString() { - return ("ServerMessageId: " + serverMessageId + " ConsumerId: " + consumerId + " addr: " + address); - } -} diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolHandler.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolHandler.java index 071df5c5fb5f..5673742e0e7c 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolHandler.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolHandler.java @@ -45,6 +45,8 @@ import org.apache.activemq.artemis.api.core.ActiveMQSecurityException; import org.apache.activemq.artemis.api.core.Pair; import org.apache.activemq.artemis.core.io.IOCallback; +import org.apache.activemq.artemis.core.persistence.OperationContext; +import org.apache.activemq.artemis.core.persistence.StorageManager; import org.apache.activemq.artemis.core.protocol.mqtt.exceptions.DisconnectException; import org.apache.activemq.artemis.core.protocol.mqtt.exceptions.InvalidClientIdException; import org.apache.activemq.artemis.core.server.ActiveMQServer; @@ -91,11 +93,10 @@ public MQTTProtocolHandler(ActiveMQServer server, MQTTProtocolManager protocolMa this.mqttMessageActor = new Actor<>(server.getThreadPool(), this::act); } - void setConnection(MQTTConnection connection, ConnectionEntry entry) throws Exception { + void setConnection(MQTTConnection connection, ConnectionEntry entry) { this.connectionEntry = entry; this.connection = connection; this.session = new MQTTSession(this, connection, protocolManager, server.getConfiguration().getWildcardConfiguration(), server.newOperationContext()); - server.getStorageManager().setContext(session.getSessionContext()); } void stop() { @@ -124,6 +125,15 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) { return; } + if (session.getServerSession() != null && session.getServerSession().isClosed()) { + MQTTLogger.LOGGER.internalSessionClosed(MQTTUtil.getMessageForLogging(message, session.getVersion()), session.getState().getClientId()); + if (session.getVersion() == MQTTVersion.MQTT_5) { + sendDisconnect(MQTTReasonCodes.IMPLEMENTATION_SPECIFIC_ERROR); + } + disconnect(true); + return; + } + String interceptResult = this.protocolManager.invokeIncoming(message, this.connection); if (interceptResult != null) { logger.debug("Interceptor {} rejected MQTT control packet: {}", interceptResult, message); @@ -151,7 +161,18 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) { } } + private OperationContext recoverContext() { + OperationContext oldContext = server.getStorageManager().getContext(); + server.getStorageManager().setContext(session.getSessionContext()); + return oldContext; + } + + private void resetContext(OperationContext oldContext) { + server.getStorageManager().setContext(oldContext); + } + public void act(MqttMessage message) { + OperationContext oldContext = recoverContext(); try { switch (message.fixedHeader().messageType()) { case AUTH: @@ -193,13 +214,14 @@ public void act(MqttMessage message) { disconnect(true); } } catch (Exception e) { - MQTTLogger.LOGGER.errorProcessingControlPacket(message.toString(), e); + MQTTLogger.LOGGER.errorProcessingPacket(session.getState().getClientId(), MQTTUtil.getMessageForLogging(message, session.getVersion()), e.getMessage(), e); if (session.getVersion() == MQTTVersion.MQTT_5) { sendDisconnect(MQTTReasonCodes.IMPLEMENTATION_SPECIFIC_ERROR); } disconnect(true); } finally { ReferenceCountUtil.release(message); + resetContext(oldContext); } } @@ -273,8 +295,8 @@ void disconnect(boolean error) { } void disconnect(boolean error, MqttMessage disconnect) { - if (disconnect != null && disconnect.variableHeader() instanceof MqttReasonCodeAndPropertiesVariableHeader) { - Integer sessionExpiryInterval = MQTTUtil.getProperty(Integer.class, ((MqttReasonCodeAndPropertiesVariableHeader)disconnect.variableHeader()).properties(), SESSION_EXPIRY_INTERVAL, null); + if (disconnect != null && disconnect.variableHeader() instanceof MqttReasonCodeAndPropertiesVariableHeader variableHeader) { + Integer sessionExpiryInterval = MQTTUtil.getProperty(Integer.class, variableHeader.properties(), SESSION_EXPIRY_INTERVAL, null); if (sessionExpiryInterval != null) { session.getState().setClientSessionExpiryInterval(sessionExpiryInterval); } @@ -318,6 +340,12 @@ void handlePublish(MqttPublishMessage message) throws Exception { return; } + if (message.fixedHeader().qosLevel().value() == 2 && session.getState().getPublishCache().contains(message.variableHeader().packetId())) { + MQTTLogger.LOGGER.ignoringQoS2Publish(message.variableHeader().packetId(), session.getState().getClientId()); + sendPubRec(message.variableHeader().packetId(), MQTTReasonCodes.SUCCESS); + return; + } + try { session.getMqttPublishManager().sendToQueue(message, false); } catch (DisconnectException e) { @@ -332,8 +360,8 @@ void sendPubAck(int messageId, byte reasonCode) { sendPublishProtocolControlMessage(messageId, MqttMessageType.PUBACK, reasonCode); } - void sendPubRel(int messageId) { - sendPublishProtocolControlMessage(messageId, MqttMessageType.PUBREL); + void sendPubRel(int messageId, byte reasonCode) { + sendPublishProtocolControlMessage(messageId, MqttMessageType.PUBREL, reasonCode); } void sendPubRec(int messageId, byte reasonCode) { @@ -341,11 +369,7 @@ void sendPubRec(int messageId, byte reasonCode) { } void sendPubComp(int messageId) { - sendPublishProtocolControlMessage(messageId, MqttMessageType.PUBCOMP); - } - - void sendPublishProtocolControlMessage(int messageId, MqttMessageType messageType) { - sendPublishProtocolControlMessage(messageId, messageType, MQTTReasonCodes.SUCCESS); + sendPublishProtocolControlMessage(messageId, MqttMessageType.PUBCOMP, MQTTReasonCodes.SUCCESS); } void sendPublishProtocolControlMessage(int messageId, MqttMessageType messageType, byte reasonCode) { @@ -364,19 +388,27 @@ void sendPublishProtocolControlMessage(int messageId, MqttMessageType messageTyp void handlePuback(MqttPubAckMessage message) throws Exception { // ((MqttPubReplyMessageVariableHeader)message.variableHeader()).reasonCode(); - session.getMqttPublishManager().handlePubAck(getMessageId(message)); + session.getMqttPublishManager().handlePubAck(getPacketId(message)); } void handlePubrec(MqttMessage message) throws Exception { - session.getMqttPublishManager().handlePubRec(getMessageId(message)); + byte reasonCode = MQTTReasonCodes.SUCCESS; + if (message.variableHeader() instanceof MqttPubReplyMessageVariableHeader header) { + reasonCode = header.reasonCode(); + } + if ((reasonCode & 0xFF) >= 0x80) { + session.getMqttPublishManager().handlePubRecError(getPacketId(message)); + } else { + session.getMqttPublishManager().handlePubRec(getPacketId(message)); + } } - void handlePubrel(MqttMessage message) { - session.getMqttPublishManager().handlePubRel(getMessageId(message)); + void handlePubrel(MqttMessage message) throws Exception { + session.getMqttPublishManager().handlePubRel(getPacketId(message)); } void handlePubcomp(MqttMessage message) throws Exception { - session.getMqttPublishManager().handlePubComp(getMessageId(message)); + session.getMqttPublishManager().handlePubComp(getPacketId(message)); } void handleSubscribe(MqttSubscribeMessage message) throws Exception { @@ -410,19 +442,28 @@ protected void sendToClient(MqttMessage message) { return; } MQTTUtil.logMessage(session.getState(), message, false, session.getVersion()); - server.getStorageManager().afterCompleteOperations(new IOCallback() { + runAfterStorageOperations(() -> ctx.writeAndFlush(message, ctx.voidPromise())); + } + + void runAfterStorageOperations(Runnable runnable) { + StorageManager storageManager = server.getStorageManager(); + if (storageManager == null) { + throw MQTTBundle.BUNDLE.storageManagerIsNull(); + } + storageManager.afterCompleteOperations(new IOCallback() { @Override public void done() { - ctx.writeAndFlush(message, ctx.voidPromise()); + runnable.run(); } @Override public void onError(int errorCode, String errorMessage) { + MQTTLogger.LOGGER.storageOperationError(errorCode, errorMessage); } }); } - private int getMessageId(MqttMessage message) { + private int getPacketId(MqttMessage message) { return ((MqttMessageIdVariableHeader) message.variableHeader()).messageId(); } @@ -528,16 +569,7 @@ private Pair validateUser(String username, String password) thr session.getProtocolHandler().sendConnack(MQTTReasonCodes.NOT_AUTHORIZED_3); } // avoid a race with sending the CONNACK packet and disconnecting the client - server.getStorageManager().afterCompleteOperations(new IOCallback() { - @Override - public void done() { - disconnect(true); - } - - @Override - public void onError(int errorCode, String errorMessage) { - } - }); + runAfterStorageOperations(() -> disconnect(true)); result = Boolean.FALSE; } diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java index 575efccbde0d..2161c4754f17 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java @@ -29,6 +29,7 @@ import io.netty.handler.codec.mqtt.MqttMessage; import org.apache.activemq.artemis.api.core.ActiveMQBuffer; import org.apache.activemq.artemis.api.core.BaseInterceptor; +import org.apache.activemq.artemis.api.core.FilterConstants; import org.apache.activemq.artemis.api.core.SimpleString; import org.apache.activemq.artemis.api.core.management.CoreNotificationType; import org.apache.activemq.artemis.api.core.management.ManagementHelper; @@ -76,6 +77,10 @@ public class MQTTProtocolManager extends AbstractProtocolManager services) { + public void loadProtocolServices(ActiveMQServer server, List services) throws Exception { + server.registerRecordsLoader(MQTTStateManager.getInstance(server)::reload); services.add(new MQTTPeriodicTasks(server, server.getScheduledPool())); } diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTPublishManager.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTPublishManager.java index 5f8d8453f827..8c337426f295 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTPublishManager.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTPublishManager.java @@ -33,17 +33,12 @@ import io.netty.handler.codec.mqtt.MqttQoS; import io.netty.handler.codec.mqtt.MqttTopicSubscription; import org.apache.activemq.artemis.api.core.ActiveMQBuffer; -import org.apache.activemq.artemis.api.core.ActiveMQIllegalStateException; import org.apache.activemq.artemis.api.core.ActiveMQSecurityException; import org.apache.activemq.artemis.api.core.ICoreMessage; import org.apache.activemq.artemis.api.core.Message; -import org.apache.activemq.artemis.api.core.Pair; -import org.apache.activemq.artemis.api.core.QueueConfiguration; import org.apache.activemq.artemis.api.core.RoutingType; import org.apache.activemq.artemis.api.core.SimpleString; -import org.apache.activemq.artemis.core.io.IOCallback; import org.apache.activemq.artemis.core.protocol.mqtt.exceptions.DisconnectException; -import org.apache.activemq.artemis.core.server.Queue; import org.apache.activemq.artemis.core.server.ServerConsumer; import org.apache.activemq.artemis.core.server.ServerProducer; import org.apache.activemq.artemis.core.server.impl.AddressInfo; @@ -68,7 +63,6 @@ import static org.apache.activemq.artemis.core.protocol.mqtt.MQTTUtil.MQTT_RESPONSE_TOPIC_KEY; import static org.apache.activemq.artemis.core.protocol.mqtt.MQTTUtil.MQTT_USER_PROPERTY_EXISTS_KEY; import static org.apache.activemq.artemis.core.protocol.mqtt.MQTTUtil.MQTT_USER_PROPERTY_KEY_PREFIX_SIMPLE; -import static org.apache.activemq.artemis.core.protocol.mqtt.MQTTUtil.createServerMessage; /** * Handles MQTT Exactly Once (QoS level 2) Protocol. @@ -77,24 +71,14 @@ public class MQTTPublishManager { private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - private SimpleString qos2ManagementAddress; - - private Queue qos2ManagementQueue; - private final String senderName = UUIDGenerator.getInstance().generateUUID().toString(); private boolean createProducer = true; - private ServerConsumer qos2ManagementConsumer; - private final MQTTSession session; private final Object lock = new Object(); - private MQTTSessionState state; - - private MQTTSessionState.OutboundStore outboundStore; - private boolean closeMqttConnectionOnPublishAuthorizationFailure; public MQTTPublishManager(MQTTSession session, boolean closeMqttConnectionOnPublishAuthorizationFailure) { @@ -102,58 +86,56 @@ public MQTTPublishManager(MQTTSession session, boolean closeMqttConnectionOnPubl this.closeMqttConnectionOnPublishAuthorizationFailure = closeMqttConnectionOnPublishAuthorizationFailure; } - synchronized void start() { - this.state = session.getState(); - this.outboundStore = state.getOutboundStore(); - } - - synchronized void stop() throws Exception { + void stop() throws Exception { ServerSessionImpl serversession = session.getServerSession(); if (serversession != null) { serversession.removeProducer(serversession.getName()); } - if (qos2ManagementConsumer != null) { - qos2ManagementConsumer.removeItself(); - qos2ManagementConsumer.setStarted(false); - qos2ManagementConsumer.close(false); - } - } - - void clean() throws Exception { - if (qos2ManagementQueue != null) { - qos2ManagementQueue.deleteQueue(); - } - } - - boolean isQos2ManagementConsumer(ServerConsumer consumer) { - return consumer == qos2ManagementConsumer; } /** - * Since MQTT Subscriptions can overlap, a client may receive the same message twice. When this happens the client - * returns a PubRec or PubAck with ID. But we need to know which consumer to ack, since we only have the ID to go on - * we are not able to decide which consumer to ack. Instead we send MQTT messages with different IDs and store a - * reference to original ID and consumer in the Session state. This way we can look up the consumer Id and the - * message Id from the PubAck or PubRec message id. + * Delivers a message to the MQTT client at the appropriate QoS level. For QoS 1 and 2, each delivery is tracked by a + * journal-persisted {@link PacketIdCorrelationKey} that pairs the core message ID and subscription address to an + * MQTT packet ID. This ensures the same packet ID is reused when a message is redelivered after a broker restart, as + * required by the MQTT specification. The address component of the key allows overlapping subscriptions to receive + * the same message with distinct packet IDs. + *

+ * A {@link CoreDeliveryInfo} is also stored in-memory for each in-flight packet ID, mapping it back to the consumer + * ID and correlation key so the correct consumer can be acknowledged when the client responds with PUBACK or + * PUBCOMP. */ - protected void sendMessage(ICoreMessage message, ServerConsumer consumer, int deliveryCount) throws Exception { - // This is to allow retries of PubRel. - if (isQos2ManagementConsumer(consumer)) { - sendPubRelMessage(message); - } else { - int qos = decideQoS(message, consumer); - if (qos == 0) { - if (publishToClient((int) message.getMessageID(), message, deliveryCount, qos, consumer.getID())) { - session.getServerSession().individualAcknowledge(consumer.getID(), message.getMessageID()); + protected void publishToClient(ICoreMessage message, ServerConsumer consumer) throws Exception { + MQTTSessionState state = session.getState(); + int qos = decideQoS(message, consumer); + if (qos == 0) { + // [MQTT-2.2.1-2] Hard-code the packet ID to 0 as QoS0 PUBLISH packets don't have a packet ID + // [MQTT-3.3.1-2] The DUP flag MUST be set to 0 for all QoS 0 messages. + if (publishToClient(0, message, false, qos)) { + consumer.individualAcknowledge(null, message.getMessageID()); + } + } else if (qos == 1 || qos == 2) { + Integer existingPacketId; + final int packetIdToUse; + boolean redelivery = false; + synchronized (this) { + PacketIdCorrelationKey correlationKey = PacketIdCorrelationKey.of(message.getMessageID(), message.getAddressSimpleString()); + existingPacketId = session.getStateManager().getPacketIdCorrelation(state.getClientId(), correlationKey); + if (existingPacketId != null && !state.coreDeliveryInfoExists(existingPacketId)) { + // re-delivery after reconnect or restart; reuse persisted packet ID + packetIdToUse = existingPacketId; + redelivery = true; + } else { + // first delivery, or same message via a different subscription + packetIdToUse = state.generatePacketId(); + session.getStateManager().putPacketIdCorrelation(state.getClientId(), correlationKey, packetIdToUse); } - } else if (qos == 1 || qos == 2) { - int mqttid = outboundStore.generateMqttId(message.getMessageID(), consumer.getID()); - outboundStore.publish(mqttid, message.getMessageID(), consumer.getID()); - publishToClient(mqttid, message, deliveryCount, qos, consumer.getID()); - } else { - // Client must have disconnected and it's Subscription QoS cleared - consumer.individualCancel(message.getMessageID(), false); + state.putCoreDeliveryInfo(packetIdToUse, CoreDeliveryInfo.of(consumer.getID(), correlationKey)); + state.incrementSendQuota(); } + publishToClient(packetIdToUse, message, redelivery, qos); + } else { + // Client must have disconnected and it's Subscription QoS cleared + consumer.individualCancel(message.getMessageID(), false); } } @@ -205,77 +187,81 @@ void sendToQueue(MqttPublishMessage message, boolean internal) throws Exception if (qos > 0) { serverMessage.setDurable(MQTTUtil.DURABLE_MESSAGES); } - int packetId = message.variableHeader().packetId(); - boolean qos2PublishAlreadyReceived = state.getPubRec().contains(packetId); - if (qos < 2 || !qos2PublishAlreadyReceived) { - Transaction tx = session.getServerSession().newTransaction(); - try { - AddressInfo addressInfo = session.getServer().getAddressInfo(address); - if (addressInfo == null && session.getServer().getAddressSettingsRepository().getMatch(coreAddress).isAutoCreateAddresses()) { - session.getServerSession().createAddress(address, RoutingType.MULTICAST, true); - serverMessage.setRoutingType(RoutingType.MULTICAST); - } - if (addressInfo != null) { - serverMessage.setRoutingType(addressInfo.getRoutingType()); - } - session.getServerSession().send(tx, serverMessage, true, senderName, false); - if (qos == 2 && !internal) { - state.getPubRec().add(packetId); - } + // only start a transction if really necessary + Transaction tx = (qos == 2 && !internal) || message.fixedHeader().isRetain() ? session.getServerSession().newTransaction() : null; - if (message.fixedHeader().isRetain()) { - ByteBuf payload = message.payload(); - boolean reset = payload instanceof EmptyByteBuf || payload.capacity() == 0; - session.getRetainMessageManager().handleRetainedMessage(serverMessage, topic, reset, tx); - } + try { + AddressInfo addressInfo = session.getServer().getAddressInfo(address); + if (addressInfo == null && session.getServer().getAddressSettingsRepository().getMatch(coreAddress).isAutoCreateAddresses()) { + session.getServerSession().createAddress(address, RoutingType.MULTICAST, true); + serverMessage.setRoutingType(RoutingType.MULTICAST); + } + if (addressInfo != null) { + serverMessage.setRoutingType(addressInfo.getRoutingType()); + } + + session.getServerSession().send(tx, serverMessage, true, senderName, false); + + if (qos == 2 && !internal) { + session.getState().getPublishCache().add(message.variableHeader().packetId(), tx); + } + + if (message.fixedHeader().isRetain()) { + ByteBuf payload = message.payload(); + boolean reset = payload instanceof EmptyByteBuf || payload.capacity() == 0; + session.getRetainMessageManager().handleRetainedMessage(serverMessage, topic, reset, tx); + } + if (tx != null) { tx.commit(); - } catch (ActiveMQSecurityException e) { + } + } catch (ActiveMQSecurityException e) { + if (tx != null) { tx.rollback(); - if (internal) { - throw e; - } - if (session.getVersion() == MQTTVersion.MQTT_5) { - sendMessageAck(internal, qos, packetId, MQTTReasonCodes.NOT_AUTHORIZED); - return; - } else if (session.getVersion() == MQTTVersion.MQTT_3_1_1) { - /* - * For MQTT 3.1.1 clients: - * - * [MQTT-3.3.5-2] If a Server implementation does not authorize a PUBLISH to be performed by a Client; - * it has no way of informing that Client. It MUST either make a positive acknowledgement, according - * to the normal QoS rules, or close the Network Connection - * - * Throwing an exception here will ultimately close the connection. This is the default behavior. - */ - if (closeMqttConnectionOnPublishAuthorizationFailure) { - throw new DisconnectException(); - } else { - logger.debug("MQTT 3.1.1 client not authorized to publish message."); - } + } + if (internal) { + throw e; + } + if (session.getVersion() == MQTTVersion.MQTT_5) { + sendMessageAck(internal, qos, message.variableHeader().packetId(), MQTTReasonCodes.NOT_AUTHORIZED); + return; + } else if (session.getVersion() == MQTTVersion.MQTT_3_1_1) { + /* + * For MQTT 3.1.1 clients: + * + * [MQTT-3.3.5-2] If a Server implementation does not authorize a PUBLISH to be performed by a Client; + * it has no way of informing that Client. It MUST either make a positive acknowledgement, according + * to the normal QoS rules, or close the Network Connection + * + * Throwing an exception here will ultimately close the connection. This is the default behavior. + */ + if (closeMqttConnectionOnPublishAuthorizationFailure) { + throw new DisconnectException(); } else { - /* - * For MQTT 3.1 clients: - * - * Note that if a server implementation does not authorize a PUBLISH to be made by a client, it has no - * way of informing that client. It must therefore make a positive acknowledgement, according to the - * normal QoS rules, and the client will *not* be informed that it was not authorized to publish the - * message. - * - * Log the failure since we have to just swallow it. - */ - logger.debug("MQTT 3.1 client not authorized to publish message."); + logger.debug("MQTT 3.1.1 client not authorized to publish message."); } - } catch (Throwable t) { - MQTTLogger.LOGGER.failedToPublishMqttMessage(t.getMessage(), t); + } else { + /* + * For MQTT 3.1 clients: + * + * Note that if a server implementation does not authorize a PUBLISH to be made by a client, it has no + * way of informing that client. It must therefore make a positive acknowledgement, according to the + * normal QoS rules, and the client will *not* be informed that it was not authorized to publish the + * message. + * + * Log the failure since we have to just swallow it. + */ + logger.debug("MQTT 3.1 client not authorized to publish message."); + } + } catch (Throwable t) { + MQTTLogger.LOGGER.failedToPublishMqttMessage(session.getState().getClientId(), message.variableHeader().packetId(), t.getMessage(), t); + if (tx != null) { tx.rollback(); - throw t; } - } else if (qos2PublishAlreadyReceived) { - MQTTLogger.LOGGER.ignoringQoS2Publish(state.getClientId(), packetId); + throw t; } - createMessageAck(packetId, qos, internal); + session.getProtocolHandler().runAfterStorageOperations(() -> sendMessageAck(internal, qos, message.variableHeader().packetId(), MQTTReasonCodes.SUCCESS)); } } @@ -289,54 +275,19 @@ private void sendMessageAck(boolean internal, int qos, int messageId, byte reaso } } - void sendPubRelMessage(Message message) { - int messageId = message.getIntProperty(MQTTUtil.MQTT_MESSAGE_ID_KEY); - session.getState().getOutboundStore().publishReleasedSent(messageId, message.getMessageID()); - session.getProtocolHandler().sendPubRel(messageId); - } - - void handlePubRec(int messageId) throws Exception { - try { - Pair ref = outboundStore.publishReceived(messageId); - if (ref != null) { - initQos2Resources(); - MQTTUtil.sendMessageDirectlyToQueue(session.getServer().getStorageManager(), session.getServer().getPostOffice(), createPubRelMessage(session, messageId), qos2ManagementQueue, null); - session.getServerSession().individualAcknowledge(ref.getB(), ref.getA()); - releaseFlowControl(ref.getB()); - } else { - session.getProtocolHandler().sendPubRel(messageId); - } - } catch (ActiveMQIllegalStateException e) { - MQTTLogger.LOGGER.failedToAckMessage(session.getState().getClientId(), e); - } + synchronized void handlePubRecError(int packetId) throws Exception { + acknowledgeDelivery(packetId, false); } - /* - * Only create these resources if we actually need them (i.e. we're sending a message to a subscriber via QoS 2) - */ - private void initQos2Resources() throws Exception { - if (qos2ManagementAddress == null) { - qos2ManagementAddress = SimpleString.of(MQTTUtil.QOS2_MANAGEMENT_QUEUE_PREFIX + session.getState().getClientId()); - } - if (qos2ManagementQueue == null) { - qos2ManagementQueue = session.getServer().createQueue(QueueConfiguration.of(qos2ManagementAddress) - .setRoutingType(RoutingType.ANYCAST) - .setDurable(MQTTUtil.DURABLE_MESSAGES), - true); - qos2ManagementConsumer = session.getServerSession().createInternalConsumer(qos2ManagementAddress); - qos2ManagementConsumer.setStarted(true); + synchronized void handlePubRec(int packetId) throws Exception { + MQTTSessionState state = session.getState(); + if (state.getPubRecCache().contains(packetId)) { + sendAcknowledgementReply(packetId, MQTTReasonCodes.SUCCESS, true); + } else { + acknowledgeDelivery(packetId, true); } } - private Message createPubRelMessage(MQTTSession session, int messageId) { - MqttFixedHeader fixedHeader = new MqttFixedHeader(MqttMessageType.PUBREL, false, MqttQoS.AT_LEAST_ONCE, false, 0); - MqttPublishMessage publishMessage = new MqttPublishMessage(fixedHeader, null, null); - Message message = createServerMessage(session, qos2ManagementAddress, publishMessage) - .putIntProperty(MQTTUtil.MQTT_MESSAGE_ID_KEY, messageId) - .putIntProperty(MQTTUtil.MQTT_MESSAGE_TYPE_KEY, MqttMessageType.PUBREL.value()); - return message; - } - /** * Once we get an acknowledgement for a QoS 1 or 2 message we allow messages to flow */ @@ -347,76 +298,105 @@ private void releaseFlowControl(Long consumerId) { } } - void handlePubComp(int messageId) throws Exception { - Pair ref = session.getState().getOutboundStore().publishComplete(messageId); - if (ref != null) { - // ack the message via the internal server session to bypass security. - session.getServerSession().individualAcknowledge(qos2ManagementConsumer.getID(), ref.getA()); - } + synchronized void handlePubComp(int packetId) throws Exception { + session.getState().getPubRecCache().remove(packetId); } - private void createMessageAck(final int messageId, final int qos, final boolean internal) { - session.getServer().getStorageManager().afterCompleteOperations(new IOCallback() { - @Override - public void done() { - sendMessageAck(internal, qos, messageId, MQTTReasonCodes.SUCCESS); - } - - @Override - public void onError(int errorCode, String errorMessage) { - logger.error("Pub Sync Failed"); - } - }); + synchronized void handlePubRel(int packetId) throws Exception { + boolean deleted = session.getState().getPublishCache().remove(packetId); + if (!deleted) { + logger.debug("MQTT client {} sent PUBREL for packet {} but no corresponding PUBLISH was found in the cache", session.getState().getClientId(), packetId); + } + session.getProtocolHandler().sendPubComp(packetId); } - void handlePubRel(int messageId) { - // We don't check to see if a PubRel existed for this message. We assume it did and so send PubComp. - state.getPubRec().remove(messageId); - session.getProtocolHandler().sendPubComp(messageId); - state.removeMessageRef(messageId); + synchronized void handlePubAck(int packetId) throws Exception { + acknowledgeDelivery(packetId, false); } - void handlePubAck(int messageId) throws Exception { + /** + * Acknowledges an outbound QoS 1 or QoS 2 delivery when the client confirms receipt. + *

+ * For QoS 1 no reply is sent since the flow is complete. + *

+ * For QoS 2 the packet ID is recorded in the PUBREC cache and a PUBREL is sent to continue the handshake. It + * is also called when the client's PUBREC carries an error reason code, in which case the delivery is simply cleaned + * up with no reply. + *

+ * The Core message acknowledgement, packet ID correlation removal, and (for QoS 2) PUBREC cache insertion are + * performed atomically in a single transaction. + * + * @param packetId the MQTT packet identifier for the in-flight delivery + * @param needsPubRel {@code true} to record the PUBREC and reply with PUBREL (QoS 2 normal path); {@code false} to + * acknowledge silently (QoS 1 or QoS 2 error path) + */ + private void acknowledgeDelivery(int packetId, boolean needsPubRel) throws Exception { + MQTTSessionState state = session.getState(); + Transaction tx = null; try { - Pair ref = outboundStore.publishAckd(messageId); - if (ref != null) { - session.getServerSession().individualAcknowledge(ref.getB(), ref.getA()); - releaseFlowControl(ref.getB()); + CoreDeliveryInfo delivery = state.getCoreDeliveryInfo(packetId); + if (delivery != null) { + ServerConsumer consumer = session.getServerSession().locateConsumer(delivery.getConsumerId()); + if (consumer == null) { + MQTTLogger.LOGGER.failedToAckMessageConsumerNotFound(state.getClientId(), packetId, delivery.getConsumerId(), session.getServerSession().isClosed() ? "closed" : "not closed"); + sendAcknowledgementReply(packetId, MQTTReasonCodes.PACKET_IDENTIFIER_NOT_FOUND, needsPubRel); + return; + } + tx = session.getServerSession().newTransaction(); + if (needsPubRel) { + state.getPubRecCache().add(packetId, tx); + } + session.getStateManager().removePacketIdCorrelation(state.getClientId(), delivery.getPacketIdCorrelationKey(), tx.getID()); + consumer.individualAcknowledge(tx, delivery.getCoreMessageId()); + tx.commit(); + state.removeCoreDeliveryInfo(packetId); + state.decrementSendQuota(); + releaseFlowControl(delivery.getConsumerId()); + sendAcknowledgementReply(packetId, MQTTReasonCodes.SUCCESS, needsPubRel); + } else { + sendAcknowledgementReply(packetId, MQTTReasonCodes.PACKET_IDENTIFIER_NOT_FOUND, needsPubRel); } - } catch (ActiveMQIllegalStateException e) { - logger.warn("MQTT Client({}) attempted to Ack already Ack'd message", session.getState().getClientId()); + } catch (Exception e) { + if (tx != null) { + tx.rollback(); + } + MQTTLogger.LOGGER.failedToAckMessage(state.getClientId(), e.getMessage()); + sendAcknowledgementReply(packetId, MQTTReasonCodes.PACKET_IDENTIFIER_NOT_FOUND, needsPubRel); + } + } + + private void sendAcknowledgementReply(int packetId, byte reasonCode, boolean needsPubRel) throws Exception { + if (needsPubRel) { + session.getProtocolHandler().sendPubRel(packetId, reasonCode); } } - private boolean publishToClient(int messageId, ICoreMessage message, int deliveryCount, int qos, long consumerId) throws Exception { - String topic = MQTTUtil.getMqttTopicFromCoreAddress(Objects.requireNonNullElse(message.getAddress(), ""), session.getWildcardConfiguration()); + private boolean publishToClient(int packetId, ICoreMessage coreMessage, boolean redelivery, int qos) throws Exception { + String topic = MQTTUtil.getMqttTopicFromCoreAddress(Objects.requireNonNullElse(coreMessage.getAddress(), ""), session.getWildcardConfiguration()); ByteBuf payload; - switch (message.getType()) { + switch (coreMessage.getType()) { case Message.TEXT_TYPE: - SimpleString text = message.getDataBuffer().readNullableSimpleString(); + SimpleString text = coreMessage.getDataBuffer().readNullableSimpleString(); final int utf8Bytes = ByteBufUtil.utf8Bytes(text); payload = ByteBufAllocator.DEFAULT.directBuffer(utf8Bytes); // IMPORTANT: this one won't enlarge ByteBuf by ByteBufUtil.maxUtf8Bytes(text), but just utf8Bytes ByteBufUtil.reserveAndWriteUtf8(payload, text, utf8Bytes); break; default: - ActiveMQBuffer bodyBuffer = message.getDataBuffer(); + ActiveMQBuffer bodyBuffer = coreMessage.getDataBuffer(); payload = ByteBufAllocator.DEFAULT.directBuffer(bodyBuffer.writerIndex()); payload.writeBytes(bodyBuffer.byteBuf()); break; } - // [MQTT-3.3.1-2] The DUP flag MUST be set to 0 for all QoS 0 messages. - boolean redelivery = qos == 0 ? false : (deliveryCount > 1); - - boolean isRetain = message.containsProperty(MQTT_MESSAGE_RETAIN_INITIAL_DISTRIBUTION_KEY); + boolean isRetain = coreMessage.containsProperty(MQTT_MESSAGE_RETAIN_INITIAL_DISTRIBUTION_KEY); MqttProperties mqttProperties = null; if (session.getVersion() == MQTTVersion.MQTT_5) { - mqttProperties = getPublishProperties(message); - if (!isRetain && message.getBooleanProperty(MQTT_MESSAGE_RETAIN_KEY)) { - MqttTopicSubscription sub = session.getState().getSubscription(topic); + mqttProperties = getPublishProperties(coreMessage); + if (!isRetain && coreMessage.getBooleanProperty(MQTT_MESSAGE_RETAIN_KEY)) { + MqttTopicSubscription sub = session.getState().getSubscriptionItem(topic).getSubscription(); if (sub != null && sub.option().isRetainAsPublished()) { isRetain = true; } @@ -437,8 +417,8 @@ private boolean publishToClient(int messageId, ICoreMessage message, int deliver } int remainingLength = MQTTUtil.calculateRemainingLength(topic, mqttProperties, payload); - MqttFixedHeader header = new MqttFixedHeader(MqttMessageType.PUBLISH, redelivery, MqttQoS.valueOf(qos), isRetain, remainingLength); - MqttPublishVariableHeader varHeader = new MqttPublishVariableHeader(topic, messageId, mqttProperties); + MqttFixedHeader header = new MqttFixedHeader(MqttMessageType.PUBLISH, qos == 0 ? false : redelivery, MqttQoS.valueOf(qos), isRetain, remainingLength); + MqttPublishVariableHeader varHeader = new MqttPublishVariableHeader(topic, packetId, mqttProperties); MqttPublishMessage publish = new MqttPublishMessage(header, varHeader, payload); int maxSize = session.getState().getClientMaxPacketSize(); @@ -449,8 +429,12 @@ private boolean publishToClient(int messageId, ICoreMessage message, int deliver * [MQTT-3.1.2-25] Where a Packet is too large to send, the Server MUST discard it without sending it and then * behave as if it had completed sending that Application Message */ - logger.debug("Not sending message {} to client as its size ({}) exceeds the max ({})", message, size, maxSize); - session.getServerSession().individualAcknowledge(consumerId, message.getMessageID()); + logger.debug("Not sending message {} to client as its size ({}) exceeds the max ({})", coreMessage, size, maxSize); + if (qos == 0) { + return true; + } else if (qos == 1 || qos == 2) { + acknowledgeDelivery(packetId, false); + } return false; } } diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSession.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSession.java index 551ab416262d..68726dd94b7b 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSession.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSession.java @@ -16,6 +16,7 @@ */ package org.apache.activemq.artemis.core.protocol.mqtt; +import java.lang.invoke.MethodHandles; import java.util.UUID; import io.netty.buffer.EmptyByteBuf; @@ -33,7 +34,6 @@ import org.apache.activemq.artemis.spi.core.protocol.SessionCallback; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.lang.invoke.MethodHandles; public class MQTTSession { @@ -81,7 +81,7 @@ public MQTTSession(MQTTProtocolHandler protocolHandler, MQTTConnection connection, MQTTProtocolManager protocolManager, WildcardConfiguration wildcardConfiguration, - OperationContext sessionContext) throws Exception { + OperationContext sessionContext) { this.protocolHandler = protocolHandler; this.protocolManager = protocolManager; this.stateManager = protocolManager.getStateManager(); @@ -92,7 +92,7 @@ public MQTTSession(MQTTProtocolHandler protocolHandler, mqttConnectionManager = new MQTTConnectionManager(this); mqttPublishManager = new MQTTPublishManager(this, protocolManager.isCloseMqttConnectionOnPublishAuthorizationFailure()); sessionCallback = new MQTTSessionCallback(this, connection, protocolManager.getDefaultMaximumInFlightPublishMessages()); - subscriptionManager = new MQTTSubscriptionManager(this, stateManager); + subscriptionManager = new MQTTSubscriptionManager(this); retainMessageManager = new MQTTRetainMessageManager(this); state = MQTTSessionState.DEFAULT; @@ -105,7 +105,6 @@ public MQTTSession(MQTTProtocolHandler protocolHandler, * which is synchronized with MQTTConnectionManager.disconnect */ void start() throws Exception { - mqttPublishManager.start(); subscriptionManager.start(); stopped = false; } @@ -130,7 +129,8 @@ void stop(boolean failure) throws Exception { state.setAttached(false); state.setDisconnectedTime(System.currentTimeMillis()); state.clearTopicAliases(); - state.getOutboundStore().resetSendQuota(); + state.resetSendQuota(); + state.clearCoreDeliveryInfo(); if (getVersion() == MQTTVersion.MQTT_5) { if (state.getClientSessionExpiryInterval() == 0) { @@ -231,7 +231,6 @@ MQTTStateManager getStateManager() { void clean(boolean enforceSecurity) throws Exception { subscriptionManager.clean(enforceSecurity); - mqttPublishManager.clean(); state.clear(); } diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionCallback.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionCallback.java index 9b80d5b66faf..5cd349748490 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionCallback.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionCallback.java @@ -28,7 +28,7 @@ public class MQTTSessionCallback implements SessionCallback { private final MQTTConnection connection; private final int defaultMaximumInFlightPublishMessages; - public MQTTSessionCallback(MQTTSession session, MQTTConnection connection, int defaultMaximumInFlightPublishMessages) throws Exception { + public MQTTSessionCallback(MQTTSession session, MQTTConnection connection, int defaultMaximumInFlightPublishMessages) { this.session = session; this.connection = connection; this.defaultMaximumInFlightPublishMessages = defaultMaximumInFlightPublishMessages; @@ -49,9 +49,9 @@ public int sendMessage(MessageReference ref, ServerConsumer consumer, int deliveryCount) { try { - session.getMqttPublishManager().sendMessage(ref.getMessage().toCore(), consumer, deliveryCount); + session.getMqttPublishManager().publishToClient(ref.getMessage().toCore(), consumer); } catch (Exception e) { - MQTTLogger.LOGGER.unableToSendMessage(ref, e); + MQTTLogger.LOGGER.unableToSendMessage(session.getState().getClientId(), ref, e); } return 1; } @@ -110,7 +110,7 @@ public boolean hasCredits(ServerConsumer consumerID, MessageReference ref) { * Therefore, enforce flow-control based on the number of pending QoS 1 & 2 messages */ int maxInFlightPublishMessages = connection.getReceiveMaximum() > 0 ? connection.getReceiveMaximum() : defaultMaximumInFlightPublishMessages; - if (ref != null && ref.isDurable() == true && maxInFlightPublishMessages > 0 && session.getState().getOutboundStore().getSendQuota() >= maxInFlightPublishMessages) { + if (ref != null && ref.isDurable() == true && maxInFlightPublishMessages > 0 && session.getState().getSendQuota() >= maxInFlightPublishMessages) { return false; } else { return true; diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionState.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionState.java index 8397abee1678..031373f1b030 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionState.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionState.java @@ -24,9 +24,9 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import io.netty.buffer.ByteBuf; import io.netty.handler.codec.mqtt.MqttProperties; @@ -35,9 +35,7 @@ import io.netty.handler.codec.mqtt.MqttTopicSubscription; import org.apache.activemq.artemis.api.core.ActiveMQBuffer; import org.apache.activemq.artemis.api.core.Message; -import org.apache.activemq.artemis.api.core.Pair; import org.apache.activemq.artemis.api.core.SimpleString; -import org.apache.activemq.artemis.core.config.WildcardConfiguration; import org.apache.activemq.artemis.core.message.impl.CoreMessage; import org.apache.activemq.artemis.core.postoffice.Address; import org.apache.activemq.artemis.core.postoffice.impl.AddressImpl; @@ -54,20 +52,27 @@ public class MQTTSessionState { private final String clientId; - private final ConcurrentMap subscriptions = new ConcurrentHashMap<>(); + private final ConcurrentMap subscriptionItems = new ConcurrentHashMap<>(); - // Used to store Packet ID of Publish QoS1 and QoS2 message. See spec: 4.3.3 QoS 2: Exactly once delivery. Method B. - private final Map messageRefStore = new ConcurrentHashMap<>(); - - private final ConcurrentMap> addressMessageMap = new ConcurrentHashMap<>(); + /** + * Records packet IDs of inbound QoS2 PUBLISH messages (client → broker). The ID is added when the broker + * receives the PUBLISH and removed when the client completes the handshake with PUBREL, preventing duplicate + * processing of the same message. + */ + protected PacketIdCache publishCache; - private final Set pubRec = new HashSet<>(); + /** + * Records packet IDs of outbound QoS2 messages (broker → client) that have reached the PUBREC stage. The + * ID is added when the broker receives PUBREC and removed when the client sends PUBCOMP, ensuring the broker can + * resume the handshake after a reconnect. + */ + protected PacketIdCache pubRecCache; private boolean attached = false; private long disconnectedTime = 0; - private final OutboundStore outboundStore = new OutboundStore(); + private PacketIdGenerator packetIdGenerator; private int clientSessionExpiryInterval; @@ -97,6 +102,11 @@ public class MQTTSessionState { private Map serverTopicAliases; + private Map coreDeliveryInfos; + + private static final AtomicIntegerFieldUpdater SEND_QUOTA_UPDATER = AtomicIntegerFieldUpdater.newUpdater(MQTTSessionState.class, "sendQuota"); + private volatile int sendQuota = 0; + public MQTTSessionState(String clientId) { this.clientId = clientId; } @@ -137,7 +147,12 @@ public MQTTSessionState(CoreMessage message) { MqttSubscriptionOption.RetainedHandlingPolicy retainedHandlingPolicy = MqttSubscriptionOption.RetainedHandlingPolicy.valueOf(buf.readInt()); Integer subscriptionId = buf.readNullableInt(); - subscriptions.put(topicName, new SubscriptionItem(new MqttTopicSubscription(topicName, new MqttSubscriptionOption(qos, nolocal, retainAsPublished, retainedHandlingPolicy)), subscriptionId)); + subscriptionItems.put(topicName, SubscriptionItem.of(new MqttTopicSubscription(topicName, new MqttSubscriptionOption(qos, nolocal, retainAsPublished, retainedHandlingPolicy)), subscriptionId)); + } + + if (buf.readable()) { + clientSessionExpiryInterval = buf.readInt(); + disconnectedTime = System.currentTimeMillis(); } } @@ -150,11 +165,16 @@ public void setSession(MQTTSession session) { } public synchronized void clear() throws Exception { - subscriptions.clear(); - messageRefStore.clear(); - addressMessageMap.clear(); - pubRec.clear(); - outboundStore.clear(); + subscriptionItems.clear(); + if (publishCache != null) { + publishCache.clear(); + } + if (pubRecCache != null) { + pubRecCache.clear(); + } + if (packetIdGenerator != null) { + packetIdGenerator.clear(); + } disconnectedTime = 0; if (willMessage != null) { willMessage.clear(); @@ -170,12 +190,12 @@ public synchronized void clear() throws Exception { clientTopicAliasMaximum = 0; } - public OutboundStore getOutboundStore() { - return outboundStore; - } - - public Set getPubRec() { - return pubRec; + public int generatePacketId() { + if (packetIdGenerator == null) { + packetIdGenerator = new PacketIdGenerator(); + } + int result = packetIdGenerator.generatePacketId(); + return result; } public boolean isAttached() { @@ -186,60 +206,32 @@ public void setAttached(boolean attached) { this.attached = attached; } - public Collection getSubscriptions() { - Collection result = new HashSet<>(); - for (SubscriptionItem item : subscriptions.values()) { - result.add(item.getSubscription()); - } - return result; + public Map getSubscriptionsPlusID() { + return new HashMap<>(subscriptionItems); } - public Map getSubscriptionsPlusID() { - return new HashMap<>(subscriptions); - } - - public boolean addSubscription(MqttTopicSubscription subscription, WildcardConfiguration wildcardConfiguration, Integer subscriptionIdentifier) throws Exception { - // synchronized to prevent race with removeSubscription - synchronized (subscriptions) { - addressMessageMap.putIfAbsent(MQTTUtil.getCoreAddressFromMqttTopic(subscription.topicFilter(), wildcardConfiguration), new ConcurrentHashMap<>()); - - SubscriptionItem existingSubscription = subscriptions.get(subscription.topicFilter()); - if (existingSubscription != null) { - if (subscription.qualityOfService().value() > existingSubscription.getSubscription().qualityOfService().value() - || !Objects.equals(subscriptionIdentifier, existingSubscription.getId())) { - existingSubscription.update(subscription, subscriptionIdentifier); - return true; - } else { - return false; - } - } else { - subscriptions.put(subscription.topicFilter(), new SubscriptionItem(subscription, subscriptionIdentifier)); - return true; - } - } + public Collection getSubscriptionItems() { + return new HashSet(subscriptionItems.values()); } - public void removeSubscription(String address) throws Exception { - // synchronized to prevent race with addSubscription - synchronized (subscriptions) { - subscriptions.remove(address); - addressMessageMap.remove(address); - } + public void addSubscription(SubscriptionItem item) { + subscriptionItems.put(item.getSubscription().topicFilter(), item); } - public MqttTopicSubscription getSubscription(String address) { - return subscriptions.get(address) != null ? subscriptions.get(address).getSubscription() : null; + public void removeSubscription(String topicFilter) throws Exception { + subscriptionItems.remove(topicFilter); } - public SubscriptionItem getSubscriptionPlusID(String address) { - return subscriptions.get(address); + public SubscriptionItem getSubscriptionItem(String topicFilter) { + return subscriptionItems.get(topicFilter); } public List getMatchingSubscriptionIdentifiers(String address) { String topic = MQTTUtil.getMqttTopicFromCoreAddress(address, session.getServer().getConfiguration().getWildcardConfiguration()); + Address topicToMatch = new AddressImpl(SimpleString.of(topic), MQTTUtil.MQTT_WILDCARD); List result = null; - for (SubscriptionItem item : subscriptions.values()) { - Integer matchingId = item.getMatchingId(topic); + for (SubscriptionItem item : subscriptionItems.values()) { + Integer matchingId = item.getMatchingId(topicToMatch); if (matchingId != null) { if (result == null) { result = new ArrayList<>(); @@ -394,16 +386,6 @@ public Integer getServerTopicAlias(String topicName) { return serverTopicAliases == null ? null : serverTopicAliases.get(topicName); } - void removeMessageRef(Integer mqttId) { - MQTTMessageInfo info = messageRefStore.remove(mqttId); - if (info != null) { - Map addressMap = addressMessageMap.get(info.getAddress()); - if (addressMap != null) { - addressMap.remove(info.getServerMessageId()); - } - } - } - public void clearTopicAliases() { if (clientTopicAliases != null) { clientTopicAliases.clear(); @@ -415,16 +397,44 @@ public void clearTopicAliases() { } } + public CoreDeliveryInfo getCoreDeliveryInfo(Integer packetId) { + return coreDeliveryInfos == null ? null : coreDeliveryInfos.get(packetId); + } + + public void putCoreDeliveryInfo(Integer packetId, CoreDeliveryInfo coreDeliveryInfo) { + if (coreDeliveryInfos == null) { + coreDeliveryInfos = new ConcurrentHashMap<>(); + } + coreDeliveryInfos.put(packetId, coreDeliveryInfo); + } + + public CoreDeliveryInfo removeCoreDeliveryInfo(Integer packetId) { + if (coreDeliveryInfos != null) { + return coreDeliveryInfos.remove(packetId); + } else { + return null; + } + } + + public boolean coreDeliveryInfoExists(Integer packetId) { + return coreDeliveryInfos == null ? false : coreDeliveryInfos.containsKey(packetId); + } + + public void clearCoreDeliveryInfo() { + if (coreDeliveryInfos != null) { + coreDeliveryInfos.clear(); + } + } + @Override public String toString() { return "MQTTSessionState[session=" + session + ", clientId=" + clientId + - ", subscriptions=" + subscriptions + - ", messageRefStore=" + messageRefStore + - ", addressMessageMap=" + addressMessageMap + - ", pubRec=" + pubRec + + ", subscriptionItems=" + subscriptionItems + + ", publishCache=" + publishCache + + ", pubRecCache=" + pubRecCache + ", attached=" + attached + - ", outboundStore=" + outboundStore + + ", packetIdGenerator=" + packetIdGenerator + ", disconnectedTime=" + disconnectedTime + ", sessionExpiryInterval=" + clientSessionExpiryInterval + ", isWill=" + isWill + @@ -438,104 +448,79 @@ public String toString() { "]@" + System.identityHashCode(this); } - public static class OutboundStore { - private final Map, Integer> artemisToMqttMessageMap = new HashMap<>(); - - private final Map> mqttToServerIds = new HashMap<>(); - - private final Object dataStoreLock = new Object(); - - private static final int INITIAL_ID = 0; - - private int currentId = INITIAL_ID; - - // track send quota independently because it's reset when the client disconnects, but other state must remain in tact - private int sendQuota = 0; - - private Pair generateKey(long messageId, long consumerID) { - return new Pair<>(messageId, consumerID); + public PacketIdCache getPublishCache() { + Objects.requireNonNull(session, "session is null"); + if (publishCache == null) { + publishCache = new PacketIdCache(session, PacketIdCache.TYPE.PUBLISH); } + return publishCache; + } - public int generateMqttId(long messageId, long consumerId) { - synchronized (dataStoreLock) { - Integer id = artemisToMqttMessageMap.get(generateKey(messageId, consumerId)); - if (id == null) { - final int start = currentId; - do { - // wrap around to the start if we reach the max - if (++currentId > MQTTUtil.TWO_BYTE_INT_MAX) { - currentId = INITIAL_ID; - } - // check to see if we looped all the way back around to where we started - if (start == currentId) { - // this detects an edge case where the same ID is acked & then generated again - if (currentId != INITIAL_ID && !mqttToServerIds.containsKey(currentId)) { - break; - } - throw MQTTBundle.BUNDLE.unableToGenerateID(); - } - } - while (mqttToServerIds.containsKey(currentId) || currentId == INITIAL_ID); - id = currentId; - } - return id; - } + public PacketIdCache getPubRecCache() { + Objects.requireNonNull(session, "session is null"); + if (pubRecCache == null) { + pubRecCache = new PacketIdCache(session, PacketIdCache.TYPE.PUBREC); } + return pubRecCache; + } - public void publish(int mqtt, long messageId, long consumerId) { - synchronized (dataStoreLock) { - Pair key = generateKey(messageId, consumerId); - artemisToMqttMessageMap.put(key, mqtt); - mqttToServerIds.put(mqtt, key); - sendQuota++; - } - } + public int getSendQuota() { + return sendQuota; + } - public Pair publishAckd(int mqtt) { - synchronized (dataStoreLock) { - Pair p = mqttToServerIds.remove(mqtt); - if (p != null) { - sendQuota--; - artemisToMqttMessageMap.remove(p); - } - return p; - } - } + public void incrementSendQuota() { + SEND_QUOTA_UPDATER.incrementAndGet(this); + } - public Pair publishReceived(int mqtt) { - return publishAckd(mqtt); - } + public void decrementSendQuota() { + SEND_QUOTA_UPDATER.decrementAndGet(this); + } - public void publishReleasedSent(int mqttId, long serverMessageId) { - synchronized (dataStoreLock) { - mqttToServerIds.put(mqttId, new Pair<>(serverMessageId, 0L)); - sendQuota++; - } - } + public void resetSendQuota() { + SEND_QUOTA_UPDATER.set(this, 0); + } - public Pair publishComplete(int mqtt) { - return publishAckd(mqtt); - } + private class PacketIdGenerator { + private static final int INITIAL_ID = 0; - public void clear() { - synchronized (dataStoreLock) { - artemisToMqttMessageMap.clear(); - mqttToServerIds.clear(); - currentId = INITIAL_ID; - sendQuota = 0; + private int currentId = INITIAL_ID; + + private int generatePacketId() { + final int start = currentId; + do { + // wrap around to the start if we reach the max + if (++currentId > MQTTUtil.TWO_BYTE_INT_MAX) { + currentId = INITIAL_ID; + } + // check to see if we looped all the way back around to where we started + if (start == currentId) { + // this detects an edge case where the same ID is acked & then generated again + if (currentId != INITIAL_ID && !packetIdInUse(currentId)) { + break; + } + throw MQTTBundle.BUNDLE.unableToGenerateID(); + } } + while (packetIdInUse(currentId) || currentId == INITIAL_ID); + return currentId; } - public int getSendQuota() { - synchronized (dataStoreLock) { - return sendQuota; - } + /** + * Checks to see if the packet ID is in use already for either QoS 1 or QoS 2 + */ + private boolean packetIdInUse(int packetId) { + // coreDeliveryInfoExists is redundant but is an O(1) short-circuit for the O(n) containsValue in packetIdCorrelationExists + return coreDeliveryInfoExists(packetId) || + (session != null && + session.getStateManager() != null && + session.getStateManager().packetIdCorrelationExistsForClient(clientId) && + session.getStateManager().packetIdCorrelationExists(clientId, packetId)) || + (pubRecCache != null && + pubRecCache.contains(packetId)); } - public void resetSendQuota() { - synchronized (dataStoreLock) { - sendQuota = 0; - } + private void clear() { + currentId = INITIAL_ID; } } @@ -560,44 +545,4 @@ public static WillStatus getStatus(byte status) { }; } } - - public static class SubscriptionItem { - private MqttTopicSubscription subscription; - private Integer id; - private Address address; - - public SubscriptionItem(MqttTopicSubscription subscription, Integer id) { - update(subscription, id); - } - - public MqttTopicSubscription getSubscription() { - return subscription; - } - - public Integer getId() { - return id; - } - - public Integer getMatchingId(String topic) { - if (id != null && new AddressImpl(SimpleString.of(topic), MQTTUtil.MQTT_WILDCARD).matches(address)) { - return id; - } else { - return null; - } - } - - private void update(MqttTopicSubscription newSub, Integer newId) { - if (newId != null && !newId.equals(id)) { - if (this.address == null || !subscription.topicFilter().equals(newSub.topicFilter())) { - String topicFilter = newSub.topicFilter(); - if (MQTTUtil.isSharedSubscription(topicFilter)) { - topicFilter = MQTTUtil.decomposeSharedSubscriptionTopicFilter(newSub.topicFilter()).getB(); - } - address = new AddressImpl(SimpleString.of(topicFilter), MQTTUtil.MQTT_WILDCARD); - } - } - subscription = newSub; - id = newId; - } - } } diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTStateManager.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTStateManager.java index 99e3d2265f32..fcd3d6b171e0 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTStateManager.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTStateManager.java @@ -32,8 +32,12 @@ import org.apache.activemq.artemis.api.core.QueueConfiguration; import org.apache.activemq.artemis.api.core.RoutingType; import org.apache.activemq.artemis.core.filter.impl.FilterImpl; +import org.apache.activemq.artemis.core.journal.RecordInfo; +import org.apache.activemq.artemis.core.journal.collections.JournalHashMapProvider; import org.apache.activemq.artemis.core.message.impl.CoreMessage; import org.apache.activemq.artemis.core.persistence.StorageManager; +import org.apache.activemq.artemis.core.persistence.impl.journal.JournalRecordIds; +import org.apache.activemq.artemis.core.persistence.impl.journal.OperationContextImpl; import org.apache.activemq.artemis.core.server.ActiveMQServer; import org.apache.activemq.artemis.core.server.MessageReference; import org.apache.activemq.artemis.core.server.Queue; @@ -44,12 +48,13 @@ public class MQTTStateManager { private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + private static final Map INSTANCES = new HashMap<>(); private final ActiveMQServer server; private final Map sessionStates = new ConcurrentHashMap<>(); - private final Queue sessionStore; - private static final Map INSTANCES = new HashMap<>(); + private Queue sessionStore; private final Map connectedClients = new ConcurrentHashMap<>(); private final boolean subscriptionPersistenceEnabled; + private final JournalHashMapProvider journalHashMapProvider; /* * Even though there may be multiple instances of MQTTProtocolManager (e.g. for MQTT on different ports) we only want @@ -73,37 +78,7 @@ public static synchronized void removeInstance(ActiveMQServer server) { private MQTTStateManager(ActiveMQServer server) throws Exception { this.server = server; this.subscriptionPersistenceEnabled = server.getConfiguration().isMqttSubscriptionPersistenceEnabled(); - if (subscriptionPersistenceEnabled) { - this.sessionStore = server.createQueue(QueueConfiguration.of(MQTTUtil.MQTT_SESSION_STORE).setRoutingType(RoutingType.ANYCAST).setLastValue(true).setDurable(true).setInternal(true).setAutoCreateAddress(true), true); - - // load subscription data from queue - try (LinkedListIterator iterator = sessionStore.browserIterator()) { - while (iterator.hasNext()) { - Message message = iterator.next().getMessage(); - if (!(message instanceof CoreMessage)) { - MQTTLogger.LOGGER.sessionStateMessageIncorrectType(message.getClass().getName()); - continue; - } - String clientId = message.getStringProperty(Message.HDR_LAST_VALUE_NAME); - if (clientId == null || clientId.isEmpty()) { - MQTTLogger.LOGGER.sessionStateMessageBadClientId(); - continue; - } - MQTTSessionState sessionState; - try { - sessionState = new MQTTSessionState((CoreMessage) message); - } catch (Exception e) { - MQTTLogger.LOGGER.errorDeserializingStateMessage(e); - continue; - } - sessionStates.put(clientId, sessionState); - } - } catch (NoSuchElementException ignored) { - // this could happen through paging browsing - } - } else { - this.sessionStore = null; - } + this.journalHashMapProvider = new JournalHashMapProvider<>(server.getStorageManager()::generateID, server.getStorageManager(), PacketIdCorrelationKey.getPersister(), JournalRecordIds.MQTT_PACKET_ID_CORRELATION, OperationContextImpl::getContext, null, server.getIoCriticalErrorListener()); } public void scanSessions() { @@ -149,9 +124,6 @@ public MQTTSessionState removeSessionState(String clientId) throws Exception { return null; } MQTTSessionState removed = sessionStates.remove(clientId); - if (removed != null && removed.getSubscriptions().size() > 0) { - removeDurableSubscriptionState(clientId); - } return removed; } @@ -171,9 +143,9 @@ public String toString() { return "MQTTSessionStateManager@" + Integer.toHexString(System.identityHashCode(this)); } - public void storeDurableSubscriptionState(MQTTSessionState state) throws Exception { + public void storeDurableState(MQTTSessionState state) throws Exception { if (subscriptionPersistenceEnabled) { - logger.debug("Adding durable MQTT subscription record for: {}", state.getClientId()); + logger.debug("Adding durable MQTT record for: {}", state.getClientId()); StorageManager storageManager = server.getStorageManager(); MQTTUtil.sendMessageDirectlyToQueue(storageManager, server.getPostOffice(), serializeState(state, storageManager.generateID()), sessionStore, null); } @@ -184,7 +156,7 @@ public static CoreMessage serializeState(MQTTSessionState state, long messageID) message.setAddress(MQTTUtil.MQTT_SESSION_STORE); message.setDurable(true); message.putStringProperty(Message.HDR_LAST_VALUE_NAME, state.getClientId()); - Map subscriptions = state.getSubscriptionsPlusID(); + Map subscriptions = state.getSubscriptionsPlusID(); ActiveMQBuffer buf = message.getBodyBuffer(); /* @@ -195,7 +167,7 @@ public static CoreMessage serializeState(MQTTSessionState state, long messageID) buf.writeInt(subscriptions.size()); logger.debug("Serializing {} subscriptions", subscriptions.size()); - for (MQTTSessionState.SubscriptionItem item : subscriptions.values()) { + for (SubscriptionItem item : subscriptions.values()) { MqttTopicSubscription sub = item.getSubscription(); buf.writeString(sub.topicFilter()); buf.writeInt(sub.option().qos().value()); @@ -205,6 +177,8 @@ public static CoreMessage serializeState(MQTTSessionState state, long messageID) buf.writeNullableInt(item.getId()); } + buf.writeInt(state.getClientSessionExpiryInterval()); + return message; } @@ -244,4 +218,70 @@ public MQTTConnection getConnectedClient(String clientId) { public Map getConnectedClients() { return connectedClients; } + + public void reload(RecordInfo recordInfo) { + if (recordInfo.userRecordType == JournalRecordIds.MQTT_PACKET_ID_CORRELATION) { + journalHashMapProvider.reload(recordInfo); + } + } + + public void putPacketIdCorrelation(String clientId, PacketIdCorrelationKey key, Integer packetId) { + journalHashMapProvider.getMap(clientId).put(key, packetId); + } + + public Integer getPacketIdCorrelation(String clientId, PacketIdCorrelationKey key) { + return journalHashMapProvider.getMap(clientId).get(key); + } + + public Integer removePacketIdCorrelation(String clientId, PacketIdCorrelationKey key, long transactionId) { + return journalHashMapProvider.getMap(clientId).remove(key, transactionId); + } + + public void clearPacketIdCorrelation(String clientId) { + journalHashMapProvider.getMap(clientId).clear(); + } + + public boolean packetIdCorrelationExists(String clientId, int packetId) { + return journalHashMapProvider.getMap(clientId).containsValue(packetId); + } + + public boolean packetIdCorrelationExistsForClient(String clientId) { + return journalHashMapProvider.containsMap(clientId); + } + + public int getPacketIdCorrelationSize(String clientId) { + return journalHashMapProvider.getMap(clientId).size(); + } + + public void init() throws Exception { + if (subscriptionPersistenceEnabled) { + this.sessionStore = server.createQueue(QueueConfiguration.of(MQTTUtil.MQTT_SESSION_STORE).setRoutingType(RoutingType.ANYCAST).setLastValue(true).setDurable(true).setInternal(true).setAutoCreateAddress(true), true); + + // load subscription data from queue + try (LinkedListIterator iterator = sessionStore.browserIterator()) { + while (iterator.hasNext()) { + Message message = iterator.next().getMessage(); + if (!(message instanceof CoreMessage)) { + MQTTLogger.LOGGER.sessionStateMessageIncorrectType(message.getClass().getName()); + continue; + } + String clientId = message.getStringProperty(Message.HDR_LAST_VALUE_NAME); + if (clientId == null || clientId.isEmpty()) { + MQTTLogger.LOGGER.sessionStateMessageBadClientId(); + continue; + } + MQTTSessionState sessionState; + try { + sessionState = new MQTTSessionState((CoreMessage) message); + } catch (Exception e) { + MQTTLogger.LOGGER.errorDeserializingStateMessage(e); + continue; + } + sessionStates.put(clientId, sessionState); + } + } catch (NoSuchElementException ignored) { + // this could happen through paging browsing + } + } + } } \ No newline at end of file diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSubscriptionManager.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSubscriptionManager.java index 6c65292a5cb1..4ddf208d740e 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSubscriptionManager.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSubscriptionManager.java @@ -28,7 +28,6 @@ import io.netty.handler.codec.mqtt.MqttTopicSubscription; import org.apache.activemq.artemis.api.core.ActiveMQQueueExistsException; import org.apache.activemq.artemis.api.core.ActiveMQSecurityException; -import org.apache.activemq.artemis.api.core.FilterConstants; import org.apache.activemq.artemis.api.core.QueueConfiguration; import org.apache.activemq.artemis.api.core.RoutingType; import org.apache.activemq.artemis.api.core.SimpleString; @@ -39,7 +38,8 @@ import org.apache.activemq.artemis.core.server.impl.AddressInfo; import org.apache.activemq.artemis.utils.CompositeAddress; -import static org.apache.activemq.artemis.core.protocol.mqtt.MQTTUtil.DOLLAR; +import static io.netty.handler.codec.mqtt.MqttSubscriptionOption.RetainedHandlingPolicy.SEND_AT_SUBSCRIBE; +import static io.netty.handler.codec.mqtt.MqttSubscriptionOption.RetainedHandlingPolicy.SEND_AT_SUBSCRIBE_IF_NOT_YET_EXISTS; import static org.apache.activemq.artemis.reader.MessageUtil.CONNECTION_ID_PROPERTY_NAME_STRING; public class MQTTSubscriptionManager { @@ -50,85 +50,74 @@ public class MQTTSubscriptionManager { private final ConcurrentMap consumerQoSLevels; - private final ConcurrentMap consumers; - - // We filter out certain messages (e.g. management messages, notifications) - private final SimpleString messageFilter; - - /* - * We can also filter out messages from any address starting with '$'. This is because MQTT clients can do silly - * things like subscribe to '#' which matches ever address on the broker. - */ - private final SimpleString messageFilterNoDollar; - private final char singleWord; private final char anyWords; - public MQTTSubscriptionManager(MQTTSession session, MQTTStateManager stateManager) { + private boolean started = false; + + public MQTTSubscriptionManager(MQTTSession session) { this.session = session; - this.stateManager = stateManager; - - singleWord = session.getServer().getConfiguration().getWildcardConfiguration().getSingleWord(); - anyWords = session.getServer().getConfiguration().getWildcardConfiguration().getAnyWords(); - - consumers = new ConcurrentHashMap<>(); - consumerQoSLevels = new ConcurrentHashMap<>(); - - // Create filter string to ignore certain messages - StringBuilder baseFilter = new StringBuilder(); - baseFilter.append("NOT ("); - baseFilter.append("(").append(FilterConstants.ACTIVEMQ_ADDRESS).append(" = '").append(session.getServer().getConfiguration().getManagementAddress()).append("')"); - baseFilter.append(" OR "); - baseFilter.append("(").append(FilterConstants.ACTIVEMQ_ADDRESS).append(" = '").append(session.getServer().getConfiguration().getManagementNotificationAddress()).append("')"); - - StringBuilder messageFilter = new StringBuilder(baseFilter); - messageFilter.append(")"); - this.messageFilter = SimpleString.of(messageFilter.toString()); - - // [MQTT-4.7.2-1] - StringBuilder messageFilterNoDollar = new StringBuilder(baseFilter); - messageFilterNoDollar.append(" OR "); - messageFilterNoDollar.append("(").append(FilterConstants.ACTIVEMQ_ADDRESS).append(" LIKE '").append(DOLLAR).append("%')"); - messageFilterNoDollar.append(")"); - this.messageFilterNoDollar = SimpleString.of(messageFilterNoDollar.toString()); + this.stateManager = session.getStateManager(); + this.singleWord = session.getServer().getConfiguration().getWildcardConfiguration().getSingleWord(); + this.anyWords = session.getServer().getConfiguration().getWildcardConfiguration().getAnyWords(); + this.consumerQoSLevels = new ConcurrentHashMap<>(); } - synchronized void start() throws Exception { - for (MqttTopicSubscription subscription : session.getState().getSubscriptions()) { - addSubscription(subscription, null, true); + void start() throws Exception { + MQTTSessionState state = session.getState(); + synchronized (state) { + if (started) { + return; + } + + for (SubscriptionItem item : state.getSubscriptionItems()) { + addSubscription(item.getSubscription(), null, true); + } + + // Re-send PUBREL for any QoS 2 messages where PUBREC was received but PUBCOMP wasn't + for (int packetId : state.getPubRecCache().getPacketIds()) { + session.getProtocolHandler().sendPubRel(packetId, MQTTReasonCodes.SUCCESS); + } + + started = true; } } private void addSubscription(MqttTopicSubscription subscription, Integer subscriptionIdentifier, boolean initialStart) throws Exception { - String rawTopicName = CompositeAddress.extractAddressName(subscription.topicFilter()); - String parsedTopicName = MQTTUtil.decomposeSharedSubscriptionTopicFilter(rawTopicName).getB(); - boolean isFullyQualified = CompositeAddress.isFullyQualified(subscription.topicFilter()); + final MQTTSessionState state = session.getState(); + final String topicFilter = subscription.topicFilter(); + final int qos = subscription.qualityOfService().value(); + final boolean isNoLocal = subscription.option().isNoLocal(); - Queue q = createQueueForSubscription(rawTopicName, parsedTopicName, isFullyQualified); + final String rawTopicName = CompositeAddress.extractAddressName(topicFilter); + final String parsedTopicName = MQTTUtil.decomposeSharedSubscriptionTopicFilter(rawTopicName).getB(); - int qos = subscription.qualityOfService().value(); + Queue q = createQueueForSubscription(rawTopicName, parsedTopicName, CompositeAddress.isFullyQualified(topicFilter)); try { if (initialStart) { - createConsumerForSubscriptionQueue(q, parsedTopicName, qos, subscription.option().isNoLocal(), null); + // it's safe to assume the subscription exists here because the caller references the same underlying map + state.getSubscriptionItem(topicFilter).setConsumer(createConsumer(q, qos, isNoLocal)); } else { - MqttTopicSubscription existingSubscription = session.getState().getSubscription(parsedTopicName); - if (existingSubscription == null) { - createConsumerForSubscriptionQueue(q, parsedTopicName, qos, subscription.option().isNoLocal(), null); + SubscriptionItem existingSub = state.getSubscriptionItem(topicFilter); + + if (existingSub == null) { + state.addSubscription(SubscriptionItem.of(subscription, subscriptionIdentifier, createConsumer(q, qos, isNoLocal))); } else { - Long existingConsumerId = consumers.get(parsedTopicName).getID(); - consumerQoSLevels.put(existingConsumerId, qos); - if (existingSubscription.option().isNoLocal() != subscription.option().isNoLocal()) { - createConsumerForSubscriptionQueue(q, parsedTopicName, qos, subscription.option().isNoLocal(), existingConsumerId); + ServerConsumer existingConsumer = existingSub.getConsumer(); + consumerQoSLevels.put(existingConsumer.getID(), qos); + if (existingSub.getSubscription().option().isNoLocal() != isNoLocal) { + existingSub.setConsumer(createConsumer(q, qos, isNoLocal, existingConsumer.getID())); + closeConsumer(existingConsumer); } + existingSub.update(subscription, subscriptionIdentifier); } - if (subscription.option().retainHandling() == MqttSubscriptionOption.RetainedHandlingPolicy.SEND_AT_SUBSCRIBE || (subscription.option().retainHandling() == MqttSubscriptionOption.RetainedHandlingPolicy.SEND_AT_SUBSCRIBE_IF_NOT_YET_EXISTS && existingSubscription == null)) { + MqttSubscriptionOption.RetainedHandlingPolicy retainedHandlingPolicy = subscription.option().retainHandling(); + if (retainedHandlingPolicy == SEND_AT_SUBSCRIBE || (retainedHandlingPolicy == SEND_AT_SUBSCRIBE_IF_NOT_YET_EXISTS && existingSub == null)) { session.getRetainMessageManager().addRetainedMessagesToQueue(q, parsedTopicName); } - - session.getState().addSubscription(subscription, session.getWildcardConfiguration(), subscriptionIdentifier); } } catch (Exception e) { // if anything broke during the creation of the consumer (or otherwise) then ensure the subscription queue @@ -137,12 +126,20 @@ private void addSubscription(MqttTopicSubscription subscription, Integer subscri } } - synchronized void stop() throws Exception { - for (ServerConsumer consumer : consumers.values()) { - consumer.setStarted(false); - consumer.disconnect(); - consumer.getQueue().removeConsumer(consumer); - consumer.close(false); + void stop() throws Exception { + MQTTSessionState state = session.getState(); + synchronized (state) { + for (SubscriptionItem item : state.getSubscriptionItems()) { + ServerConsumer consumer = item.getConsumer(); + if (consumer != null && !consumer.isClosed()) { + consumer.setStarted(false); + consumer.disconnect(); + consumer.getQueue().removeConsumer(consumer); + consumer.close(false); + } + item.setConsumer(null); + } + started = false; } } @@ -215,28 +212,38 @@ private SimpleString getMessageFilter(SimpleString addressName) { * By the time we get here wildcards in the MQTT topic filter have already been translated into their core * equivalents. This check is to enforce [MQTT-4.7.2-1]. */ + MQTTProtocolManager protocolManager = session.getProtocolManager(); if (addressName.startsWith(singleWord) || addressName.startsWith(anyWords)) { - return messageFilterNoDollar; + return protocolManager.getMessageFilterNoDollar(); } else { - return messageFilter; + return protocolManager.getMessageFilter(); } } - private void createConsumerForSubscriptionQueue(Queue queue, String topicFilter, int qos, boolean noLocal, Long existingConsumerId) throws Exception { + private ServerConsumer createConsumer(Queue queue, int qos, boolean noLocal) throws Exception { + return createConsumer(queue, qos, noLocal, null); + } + + private ServerConsumer createConsumer(Queue queue, int qos, boolean noLocal, Long existingConsumerId) throws Exception { long cid = Objects.requireNonNullElseGet(existingConsumerId, () -> session.getServer().getStorageManager().generateID()); // for noLocal support we use the MQTT *client id* rather than the connection ID, but we still use the existing property name - ServerConsumer consumer = session.getServerSession().createConsumer(cid, queue.getName(), noLocal ? SimpleString.of(CONNECTION_ID_PROPERTY_NAME_STRING + " <> '" + session.getState().getClientId() + "'") : null, false, false, -1); + SimpleString filterString = noLocal ? SimpleString.of(CONNECTION_ID_PROPERTY_NAME_STRING + " <> '" + session.getState().getClientId() + "'") : null; - ServerConsumer existingConsumer = consumers.put(topicFilter, consumer); - if (existingConsumer != null) { - existingConsumer.setStarted(false); - existingConsumer.close(false); - } + ServerConsumer consumer = session.getServerSession().createConsumer(cid, queue.getName(), filterString, false, false, -1); consumer.setStarted(true); consumerQoSLevels.put(cid, qos); + + return consumer; + } + + private void closeConsumer(ServerConsumer consumer) throws Exception { + if (consumer != null) { + consumer.setStarted(false); + consumer.close(false); + } } short[] removeSubscriptions(List topics, boolean enforceSecurity) throws Exception { @@ -250,7 +257,7 @@ short[] removeSubscriptions(List topics, boolean enforceSecurity) throws synchronized (state) { reasonCodes = new short[topics.size()]; for (int i = 0; i < topics.size(); i++) { - if (state.getSubscription(topics.get(i)) == null) { + if (state.getSubscriptionItem(topics.get(i)) == null) { reasonCodes[i] = MQTTReasonCodes.NO_SUBSCRIPTION_EXISTED; continue; } @@ -258,8 +265,9 @@ short[] removeSubscriptions(List topics, boolean enforceSecurity) throws short reasonCode = MQTTReasonCodes.SUCCESS; try { + SubscriptionItem item = state.getSubscriptionItem(topics.get(i)); + ServerConsumer removed = item != null ? item.getConsumer() : null; state.removeSubscription(topics.get(i)); - ServerConsumer removed = consumers.remove(MQTTUtil.decomposeSharedSubscriptionTopicFilter(topics.get(i)).getB()); if (removed != null) { removed.close(false); consumerQoSLevels.remove(removed.getID()); @@ -283,9 +291,9 @@ short[] removeSubscriptions(List topics, boolean enforceSecurity) throws } // deal with durable state after *all* requested subscriptions have been removed in memory - if (state.getSubscriptions().size() > 0) { + if (state.getSubscriptionItems().size() > 0) { // if there are some subscriptions left then update the state - stateManager.storeDurableSubscriptionState(state); + stateManager.storeDurableState(state); } else { // if there are no subscriptions left then remove the state entirely stateManager.removeDurableSubscriptionState(state.getClientId()); @@ -303,6 +311,7 @@ short[] removeSubscriptions(List topics, boolean enforceSecurity) throws int[] addSubscriptions(List subscriptions, Integer subscriptionIdentifier) throws Exception { MQTTSessionState state = session.getState(); synchronized (state) { + start(); int[] qos = new int[subscriptions.size()]; for (int i = 0; i < subscriptions.size(); i++) { @@ -338,7 +347,7 @@ int[] addSubscriptions(List subscriptions, Integer subscr } // store state after *all* requested subscriptions have been created in memory - stateManager.storeDurableSubscriptionState(state); + stateManager.storeDurableState(state); return qos; } @@ -350,9 +359,10 @@ Map getConsumerQoSLevels() { void clean(boolean enforceSecurity) throws Exception { List topics = new ArrayList<>(); - for (MqttTopicSubscription mqttTopicSubscription : session.getState().getSubscriptions()) { - topics.add(mqttTopicSubscription.topicFilter()); + for (SubscriptionItem item : session.getState().getSubscriptionItems()) { + topics.add(item.getSubscription().topicFilter()); } removeSubscriptions(topics, enforceSecurity); + stateManager.clearPacketIdCorrelation(session.getState().getClientId()); } } diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTUtil.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTUtil.java index 3316e55f827d..e4c4764f0fd7 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTUtil.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTUtil.java @@ -74,9 +74,9 @@ public class MQTTUtil { public static final boolean DURABLE_MESSAGES = true; - public static final boolean SESSION_AUTO_COMMIT_SENDS = true; + public static final boolean SESSION_AUTO_COMMIT_SENDS = false; - public static final boolean SESSION_AUTO_COMMIT_ACKS = true; + public static final boolean SESSION_AUTO_COMMIT_ACKS = false; public static final boolean SESSION_PREACKNOWLEDGE = false; @@ -98,10 +98,6 @@ public class MQTTUtil { public static final SimpleString MQTT_QOS_LEVEL_KEY = SimpleString.of("mqtt.qos.level"); - public static final SimpleString MQTT_MESSAGE_ID_KEY = SimpleString.of("mqtt.message.id"); - - public static final SimpleString MQTT_MESSAGE_TYPE_KEY = SimpleString.of("mqtt.message.type"); - public static final SimpleString MQTT_MESSAGE_RETAIN_KEY = SimpleString.of("mqtt.message.retain"); public static final SimpleString MQTT_MESSAGE_RETAIN_INITIAL_DISTRIBUTION_KEY = SimpleString.of("mqtt.message.retain.initial.distribution"); @@ -120,8 +116,6 @@ public class MQTTUtil { public static final SimpleString MQTT_CONTENT_TYPE_KEY = SimpleString.of("mqtt.content.type"); - public static final String QOS2_MANAGEMENT_QUEUE_PREFIX = DOLLAR + "sys.mqtt.queue.qos2."; - public static final String SHARED_SUBSCRIPTION_PREFIX = DOLLAR + "share/"; public static final long FOUR_BYTE_INT_MAX = Long.decode("0xFFFFFFFF"); // 4_294_967_295 @@ -303,118 +297,130 @@ public static void logMessage(MQTTSessionState state, MqttMessage message, boole log.append("): OUT >> "); } - if (message.fixedHeader() != null) { - log.append(message.fixedHeader().messageType().toString()); + String messageForLogging = getMessageForLogging(message, version); - if (message.variableHeader() instanceof MqttMessageIdVariableHeader) { - log.append("(" + ((MqttMessageIdVariableHeader) message.variableHeader()).messageId() + ")"); - } + if (messageForLogging != null) { + logger.trace(log.append(messageForLogging).toString()); + } + } + } - switch (message.fixedHeader().messageType()) { - case PUBLISH: - MqttPublishVariableHeader publishHeader = (MqttPublishVariableHeader) message.variableHeader(); - String topicName = publishHeader.topicName(); - if (topicName == null || topicName.isEmpty()) { - topicName = ""; - } - log.append("(" + publishHeader.packetId() + ")") - .append(" topic=" + topicName) - .append(", qos=" + message.fixedHeader().qosLevel().value()) - .append(", retain=" + message.fixedHeader().isRetain()) - .append(", dup=" + message.fixedHeader().isDup()) - .append(", remainingLength=" + message.fixedHeader().remainingLength()); - for (MqttProperties.MqttProperty property : ((MqttPublishMessage)message).variableHeader().properties().listAll()) { - Object value = property.value(); - if (value != null) { - if (value instanceof byte[] bytes) { - value = new String(bytes, StandardCharsets.UTF_8); - } else if (value instanceof ArrayList list && !list.isEmpty() && list.get(0) instanceof MqttProperties.StringPair) { - StringBuilder userProperties = new StringBuilder(); - userProperties.append("["); - for (MqttProperties.StringPair pair : (ArrayList) value) { - userProperties.append(pair.key).append(": ").append(pair.value).append(", "); - } - userProperties.delete(userProperties.length() - 2, userProperties.length()); - userProperties.append("]"); - value = userProperties.toString(); + public static String getMessageForLogging(MqttMessage message, MQTTVersion version) { + String result = null; + + if (message.fixedHeader() != null) { + StringBuilder log = new StringBuilder(); + log.append(message.fixedHeader().messageType().toString()); + + if (message.variableHeader() instanceof MqttMessageIdVariableHeader) { + log.append("(" + ((MqttMessageIdVariableHeader) message.variableHeader()).messageId() + ")"); + } + + switch (message.fixedHeader().messageType()) { + case PUBLISH: + MqttPublishVariableHeader publishHeader = (MqttPublishVariableHeader) message.variableHeader(); + String topicName = publishHeader.topicName(); + if (topicName == null || topicName.isEmpty()) { + topicName = ""; + } + log.append("(" + publishHeader.packetId() + ")") + .append(" topic=" + topicName) + .append(", qos=" + message.fixedHeader().qosLevel().value()) + .append(", retain=" + message.fixedHeader().isRetain()) + .append(", dup=" + message.fixedHeader().isDup()) + .append(", remainingLength=" + message.fixedHeader().remainingLength()); + for (MqttProperties.MqttProperty property : ((MqttPublishMessage) message).variableHeader().properties().listAll()) { + Object value = property.value(); + if (value != null) { + if (value instanceof byte[] bytes) { + value = new String(bytes, StandardCharsets.UTF_8); + } else if (value instanceof ArrayList list && !list.isEmpty() && list.get(0) instanceof MqttProperties.StringPair) { + StringBuilder userProperties = new StringBuilder(); + userProperties.append("["); + for (MqttProperties.StringPair pair : (ArrayList) value) { + userProperties.append(pair.key).append(": ").append(pair.value).append(", "); } + userProperties.delete(userProperties.length() - 2, userProperties.length()); + userProperties.append("]"); + value = userProperties.toString(); } - log.append(", " + formatCase(MqttPropertyType.valueOf(property.propertyId()).name()) + "=" + value); - } - log.append(", payload=" + getPayloadForLogging((MqttPublishMessage) message, 256)); - break; - case CONNECT: - // intentionally omit the username & password from the log - MqttConnectVariableHeader connectHeader = (MqttConnectVariableHeader) message.variableHeader(); - MqttConnectPayload payload = ((MqttConnectMessage)message).payload(); - log.append(" protocol=(").append(connectHeader.name()).append(", ").append(connectHeader.version()).append(")") - .append(", hasPassword=").append(connectHeader.hasPassword()) - .append(", isCleanStart=").append(connectHeader.isCleanSession()) - .append(", keepAliveTimeSeconds=").append(connectHeader.keepAliveTimeSeconds()) - .append(", clientIdentifier=").append(payload.clientIdentifier()) - .append(", hasUserName=").append(connectHeader.hasUserName()) - .append(", isWillFlag=").append(connectHeader.isWillFlag()); - if (connectHeader.isWillFlag()) { - log.append(", willQos=").append(connectHeader.willQos()) - .append(", isWillRetain=").append(connectHeader.isWillRetain()) - .append(", willTopic=").append(payload.willTopic()); - } - for (MqttProperties.MqttProperty property : connectHeader.properties().listAll()) { - log.append(", " + formatCase(MqttPropertyType.valueOf(property.propertyId()).name()) + "=" + property.value()); - } - break; - case CONNACK: - MqttConnAckVariableHeader connackHeader = (MqttConnAckVariableHeader) message.variableHeader(); - log.append(" connectReasonCode=").append(formatByte(connackHeader.connectReturnCode().byteValue())) - .append(", sessionPresent=").append(connackHeader.isSessionPresent()); - for (MqttProperties.MqttProperty property : connackHeader.properties().listAll()) { - log.append(", " + formatCase(MqttPropertyType.valueOf(property.propertyId()).name()) + "=" + property.value()); - } - break; - case SUBSCRIBE: - for (MqttTopicSubscription sub : ((MqttSubscribeMessage) message).payload().topicSubscriptions()) { - log.append("\n\ttopic: ").append(sub.topicName()) - .append(", qos: ").append(sub.qualityOfService()) - .append(", nolocal: ").append(sub.option().isNoLocal()) - .append(", retainHandling: ").append(sub.option().retainHandling()) - .append(", isRetainAsPublished: ").append(sub.option().isRetainAsPublished()); } - break; - case SUBACK: - for (Integer qos : ((MqttSubAckMessage) message).payload().grantedQoSLevels()) { - log.append("\n\t" + qos); - } - break; - case UNSUBSCRIBE: - for (String topic : ((MqttUnsubscribeMessage) message).payload().topics()) { - log.append("\n\t" + topic); - } - break; - case PUBACK: - break; - case PUBREC: - case PUBREL: - case PUBCOMP: - if (version == MQTTVersion.MQTT_5) { - MqttPubReplyMessageVariableHeader pubReplyVariableHeader = (MqttPubReplyMessageVariableHeader) message.variableHeader(); - log.append(" reasonCode=").append(formatByte(pubReplyVariableHeader.reasonCode())); - } - break; - case DISCONNECT: - if (version == MQTTVersion.MQTT_5) { - MqttReasonCodeAndPropertiesVariableHeader disconnectVariableHeader = (MqttReasonCodeAndPropertiesVariableHeader) message.variableHeader(); - log.append(" reasonCode=").append(formatByte(disconnectVariableHeader.reasonCode())); - } - break; - } - - logger.trace(log.toString()); + log.append(", " + formatCase(MqttPropertyType.valueOf(property.propertyId()).name()) + "=" + value); + } + log.append(", payload=" + getPayloadForLogging((MqttPublishMessage) message, 256)); + break; + case CONNECT: + // intentionally omit the username & password from the log + MqttConnectVariableHeader connectHeader = (MqttConnectVariableHeader) message.variableHeader(); + MqttConnectPayload payload = ((MqttConnectMessage) message).payload(); + log.append(" protocol=(").append(connectHeader.name()).append(", ").append(connectHeader.version()).append(")") + .append(", hasPassword=").append(connectHeader.hasPassword()) + .append(", isCleanStart=").append(connectHeader.isCleanSession()) + .append(", keepAliveTimeSeconds=").append(connectHeader.keepAliveTimeSeconds()) + .append(", clientIdentifier=").append(payload.clientIdentifier()) + .append(", hasUserName=").append(connectHeader.hasUserName()) + .append(", isWillFlag=").append(connectHeader.isWillFlag()); + if (connectHeader.isWillFlag()) { + log.append(", willQos=").append(connectHeader.willQos()) + .append(", isWillRetain=").append(connectHeader.isWillRetain()) + .append(", willTopic=").append(payload.willTopic()); + } + for (MqttProperties.MqttProperty property : connectHeader.properties().listAll()) { + log.append(", " + formatCase(MqttPropertyType.valueOf(property.propertyId()).name()) + "=" + property.value()); + } + break; + case CONNACK: + MqttConnAckVariableHeader connackHeader = (MqttConnAckVariableHeader) message.variableHeader(); + log.append(" connectReasonCode=").append(formatByte(connackHeader.connectReturnCode().byteValue())) + .append(", sessionPresent=").append(connackHeader.isSessionPresent()); + for (MqttProperties.MqttProperty property : connackHeader.properties().listAll()) { + log.append(", " + formatCase(MqttPropertyType.valueOf(property.propertyId()).name()) + "=" + property.value()); + } + break; + case SUBSCRIBE: + for (MqttTopicSubscription sub : ((MqttSubscribeMessage) message).payload().topicSubscriptions()) { + log.append("\n\ttopic: ").append(sub.topicName()) + .append(", qos: ").append(sub.qualityOfService()) + .append(", nolocal: ").append(sub.option().isNoLocal()) + .append(", retainHandling: ").append(sub.option().retainHandling()) + .append(", isRetainAsPublished: ").append(sub.option().isRetainAsPublished()); + } + break; + case SUBACK: + for (Integer qos : ((MqttSubAckMessage) message).payload().grantedQoSLevels()) { + log.append("\n\t" + qos); + } + break; + case UNSUBSCRIBE: + for (String topic : ((MqttUnsubscribeMessage) message).payload().topics()) { + log.append("\n\t" + topic); + } + break; + case PUBACK: + break; + case PUBREC: + case PUBREL: + case PUBCOMP: + if (version == MQTTVersion.MQTT_5) { + MqttPubReplyMessageVariableHeader pubReplyVariableHeader = (MqttPubReplyMessageVariableHeader) message.variableHeader(); + log.append(" reasonCode=").append(formatByte(pubReplyVariableHeader.reasonCode())); + } + break; + case DISCONNECT: + if (version == MQTTVersion.MQTT_5) { + MqttReasonCodeAndPropertiesVariableHeader disconnectVariableHeader = (MqttReasonCodeAndPropertiesVariableHeader) message.variableHeader(); + log.append(" reasonCode=").append(formatByte(disconnectVariableHeader.reasonCode())); + } + break; } + result = log.toString(); } + + return result; } private static String formatByte(byte bite) { - return String.format("0x%02X ", bite); + return String.format("0x%02X", bite); } private static String formatCase(String string) { diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/PacketIdCache.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/PacketIdCache.java new file mode 100644 index 000000000000..d584a2c82d04 --- /dev/null +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/PacketIdCache.java @@ -0,0 +1,116 @@ +/* + * 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.artemis.core.protocol.mqtt; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import org.apache.activemq.artemis.api.core.Pair; +import org.apache.activemq.artemis.api.core.SimpleString; +import org.apache.activemq.artemis.core.postoffice.DuplicateIDCache; +import org.apache.activemq.artemis.core.postoffice.PostOffice; +import org.apache.activemq.artemis.core.transaction.Transaction; +import org.apache.activemq.artemis.utils.ByteUtil; + +/** + * Tracks MQTT packet identifiers using a {@link DuplicateIDCache} to provide durable, duplicate-safe tracking across + * broker restarts for QoS2 message flows. + *

+ * QoS1 flows do not use this cache. + */ +public class PacketIdCache { + + private DuplicateIDCache cache; + private MQTTSession session; + private PostOffice postOffice; + private final SimpleString cacheName; + + public PacketIdCache(MQTTSession session, TYPE type) { + this.session = session; + this.postOffice = session.getServer().getPostOffice(); + this.cacheName = getCacheName(session.getServer().getInternalNamingPrefix(), session.getState().getClientId(), type); + } + + /* + * "Getting" the cache from the PostOffice automatically creates it if it doesn't exist. We want to avoid this for + * most operations. + */ + private void check() { + if (cache == null && durableStateExists()) { + cache = postOffice.getDuplicateIDCache(cacheName, MQTTUtil.TWO_BYTE_INT_MAX); + } + } + + private boolean durableStateExists() { + return postOffice.duplicateIDCacheExists(cacheName); + } + + public void add(int packetId, Transaction tx) throws Exception { + if (cache == null) { + cache = postOffice.getDuplicateIDCache(cacheName, MQTTUtil.TWO_BYTE_INT_MAX); + } + cache.addToCache(ByteUtil.intToBytes(packetId), tx); + } + + public boolean contains(int packetId) { + check(); + return cache != null && cache.contains(ByteUtil.intToBytes(packetId)); + } + + public boolean remove(int packetId) throws Exception { + check(); + return cache != null && cache.deleteFromCache(ByteUtil.intToBytes(packetId)); + } + + public int size() { + check(); + return cache != null ? cache.getMap().size() : 0; + } + + public List getPacketIds() { + check(); + if (cache == null) { + return Collections.emptyList(); + } + List result = new ArrayList<>(); + for (Pair entry : cache.getMap()) { + result.add(ByteUtil.bytesToInt(entry.getA())); + } + return result; + } + + public void clear() throws Exception { + postOffice.deleteDuplicateCache(cacheName); + cache = null; + } + + public static SimpleString getCacheName(String prefix, String clientId, TYPE type) { + return SimpleString.of(prefix).concat("mqtt.qos.").concat(type.type).concat('.').concat(clientId); + } + + public enum TYPE { + PUBLISH(SimpleString.of("publish")), + PUBREC(SimpleString.of("pubrec")); + + final SimpleString type; + + TYPE(SimpleString type) { + this.type = type; + } + } +} diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/PacketIdCorrelationKey.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/PacketIdCorrelationKey.java new file mode 100644 index 000000000000..6aafb6379e73 --- /dev/null +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/PacketIdCorrelationKey.java @@ -0,0 +1,133 @@ +/* + * 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.artemis.core.protocol.mqtt; + +import java.util.Objects; + +import org.apache.activemq.artemis.api.core.ActiveMQBuffer; +import org.apache.activemq.artemis.api.core.SimpleString; +import org.apache.activemq.artemis.core.journal.collections.AbstractHashMapPersister; +import org.apache.activemq.artemis.utils.BufferHelper; +import org.apache.activemq.artemis.utils.DataConstants; + +/** + * Composite key that maps a core message delivery to its MQTT packet ID. The MQTT specification requires that packet + * IDs for in-flight QoS 1 and QoS 2 messages are unique per client session, and that the same packet ID is reused if + * the message is redelivered (e.g. after a reconnect). This mapping is persisted in the journal so that a reconnecting + * client receives the same packet ID it was originally assigned. The key includes both the core message ID and the + * subscription address because overlapping subscriptions (e.g. {@code foo/bar} and {@code foo/#}) can cause the same + * core message to be delivered to the same client more than once, each through a different subscription address and + * with its own packet ID. + */ +public class PacketIdCorrelationKey { + + private static Persister persister = new Persister(); + + public static Persister getPersister() { + return persister; + } + + private long coreMessageId; + private SimpleString address; + + public static PacketIdCorrelationKey of(long coreMessageId, SimpleString address) { + return new PacketIdCorrelationKey(coreMessageId, address); + } + + private PacketIdCorrelationKey(long coreMessageId, SimpleString address) { + this.coreMessageId = coreMessageId; + this.address = address; + } + + public long getCoreMessageId() { + return coreMessageId; + } + + public SimpleString getAddress() { + return address; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof PacketIdCorrelationKey other)) { + return false; + } + return coreMessageId == other.coreMessageId && + Objects.equals(address, other.address); + } + + @Override + public int hashCode() { + return Objects.hash(coreMessageId, address); + } + + @Override + public String toString() { + return "PacketIdCorrelation[" + "coreMessageId=" + coreMessageId + ", address=" + address + "]"; + } + + private static class Persister extends AbstractHashMapPersister { + @Override + protected int getCollectionIdSize(String collectionID) { + return BufferHelper.sizeOfString(collectionID); + } + + @Override + protected void encodeCollectionId(ActiveMQBuffer buffer, String collectionID) { + buffer.writeString(collectionID); + } + + @Override + protected String decodeCollectionId(ActiveMQBuffer buffer) { + return buffer.readString(); + } + + @Override + protected int getKeySize(PacketIdCorrelationKey packetIdCorrelationKey) { + return DataConstants.SIZE_LONG + packetIdCorrelationKey.getAddress().sizeof(); + } + + @Override + protected void encodeKey(ActiveMQBuffer buffer, PacketIdCorrelationKey packetIdCorrelationKey) { + buffer.writeLong(packetIdCorrelationKey.getCoreMessageId()); + buffer.writeSimpleString(packetIdCorrelationKey.getAddress()); + } + + @Override + protected PacketIdCorrelationKey decodeKey(ActiveMQBuffer buffer) { + return PacketIdCorrelationKey.of(buffer.readLong(), buffer.readSimpleString()); + } + + @Override + protected int getValueSize(Integer mqttPacketId) { + return DataConstants.SIZE_INT; + } + + @Override + protected void encodeValue(ActiveMQBuffer buffer, Integer mqttPacketId) { + buffer.writeInt(mqttPacketId); + } + + @Override + protected Integer decodeValue(ActiveMQBuffer buffer, PacketIdCorrelationKey coreMessageId) { + return buffer.readInt(); + } + } +} diff --git a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/SubscriptionItem.java b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/SubscriptionItem.java new file mode 100644 index 000000000000..1f07450ef066 --- /dev/null +++ b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/SubscriptionItem.java @@ -0,0 +1,83 @@ +/* + * 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.artemis.core.protocol.mqtt; + +import io.netty.handler.codec.mqtt.MqttTopicSubscription; +import org.apache.activemq.artemis.api.core.SimpleString; +import org.apache.activemq.artemis.core.postoffice.Address; +import org.apache.activemq.artemis.core.postoffice.impl.AddressImpl; +import org.apache.activemq.artemis.core.server.ServerConsumer; + +public class SubscriptionItem { + + private MqttTopicSubscription subscription; + private Integer id; + private Address address; + private volatile ServerConsumer consumer; + + public static SubscriptionItem of(MqttTopicSubscription subscription, Integer id) { + return new SubscriptionItem(subscription, id, null); + } + + public static SubscriptionItem of(MqttTopicSubscription subscription, Integer id, ServerConsumer consumer) { + return new SubscriptionItem(subscription, id, consumer); + } + + private SubscriptionItem(MqttTopicSubscription subscription, Integer id, ServerConsumer consumer) { + update(subscription, id); + this.consumer = consumer; + } + + public MqttTopicSubscription getSubscription() { + return subscription; + } + + public Integer getId() { + return id; + } + + public ServerConsumer getConsumer() { + return consumer; + } + + public SubscriptionItem setConsumer(ServerConsumer consumer) { + this.consumer = consumer; + return this; + } + + public Integer getMatchingId(Address topicToMatch) { + if (id != null && topicToMatch.matches(address)) { + return id; + } else { + return null; + } + } + + public void update(MqttTopicSubscription newSub, Integer newId) { + if (newId != null && !newId.equals(id)) { + if (this.address == null || !subscription.topicFilter().equals(newSub.topicFilter())) { + String topicFilter = newSub.topicFilter(); + if (MQTTUtil.isSharedSubscription(topicFilter)) { + topicFilter = MQTTUtil.decomposeSharedSubscriptionTopicFilter(newSub.topicFilter()).getB(); + } + address = new AddressImpl(SimpleString.of(topicFilter), MQTTUtil.MQTT_WILDCARD); + } + } + subscription = newSub; + id = newId; + } +} \ No newline at end of file diff --git a/artemis-protocols/artemis-mqtt-protocol/src/test/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionStateTest.java b/artemis-protocols/artemis-mqtt-protocol/src/test/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionStateTest.java index 7e37bd3e9a1c..6b6eaa16c4f1 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/test/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionStateTest.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/test/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionStateTest.java @@ -30,8 +30,8 @@ public class MQTTSessionStateTest { public void testGenerateMqttIdOrder() { MQTTSessionState mqttSessionState = new MQTTSessionState(RandomUtil.randomUUIDString()); for (int i = 1; i <= MQTTUtil.TWO_BYTE_INT_MAX; i++) { - assertEquals(i, mqttSessionState.getOutboundStore().generateMqttId(RandomUtil.randomLong(), RandomUtil.randomLong())); - mqttSessionState.getOutboundStore().publish(i, RandomUtil.randomLong(), RandomUtil.randomLong()); + assertEquals(i, mqttSessionState.generatePacketId()); + mqttSessionState.putCoreDeliveryInfo(i, CoreDeliveryInfo.of(RandomUtil.randomLong(), PacketIdCorrelationKey.of(RandomUtil.randomLong(), RandomUtil.randomUUIDSimpleString()))); } } @@ -41,11 +41,11 @@ public void testGenerateMqttIdWithRandomAcks() { for (int i = 0; i < 10_000; i++) { int random = RandomUtil.randomInterval(1, MQTTUtil.TWO_BYTE_INT_MAX); // acknowledge a random ID - mqttSessionState.getOutboundStore().publishAckd(random); + mqttSessionState.removeCoreDeliveryInfo(random); // the ID that was acked is now the only one available so ensure generator finds it - assertEquals(random, mqttSessionState.getOutboundStore().generateMqttId(RandomUtil.randomLong(), RandomUtil.randomLong())); + assertEquals(random, mqttSessionState.generatePacketId()); // "publish" a new message with the ID to ensure the store is full for the next loop - mqttSessionState.getOutboundStore().publish(random, RandomUtil.randomLong(), RandomUtil.randomLong()); + mqttSessionState.putCoreDeliveryInfo(random, CoreDeliveryInfo.of(RandomUtil.randomLong(), PacketIdCorrelationKey.of(RandomUtil.randomLong(), RandomUtil.randomUUIDSimpleString()))); } } @@ -53,13 +53,13 @@ public void testGenerateMqttIdWithRandomAcks() { public void testGenerateMqttIdExhausted() { MQTTSessionState mqttSessionState = createMqttSessionStateWithFullOutboundStore(); // ensure we throw an exception when we try generate a new ID once the store is full - assertThrows(IllegalStateException.class, () -> mqttSessionState.getOutboundStore().generateMqttId(RandomUtil.randomLong(), RandomUtil.randomLong())); + assertThrows(IllegalStateException.class, () -> mqttSessionState.generatePacketId()); } private static MQTTSessionState createMqttSessionStateWithFullOutboundStore() { MQTTSessionState mqttSessionState = new MQTTSessionState(RandomUtil.randomUUIDString()); for (int i = 1; i <= MQTTUtil.TWO_BYTE_INT_MAX; i++) { - mqttSessionState.getOutboundStore().publish(i, RandomUtil.randomLong(), RandomUtil.randomLong()); + mqttSessionState.putCoreDeliveryInfo(i, CoreDeliveryInfo.of(RandomUtil.randomLong(), PacketIdCorrelationKey.of(RandomUtil.randomLong(), RandomUtil.randomUUIDSimpleString()))); } return mqttSessionState; } diff --git a/artemis-protocols/artemis-mqtt-protocol/src/test/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTStateManagerTest.java b/artemis-protocols/artemis-mqtt-protocol/src/test/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTStateManagerTest.java index 0b4b461ce717..21fdcc5b2db6 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/test/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTStateManagerTest.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/test/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTStateManagerTest.java @@ -26,6 +26,7 @@ import java.util.concurrent.atomic.AtomicReference; import org.apache.activemq.artemis.core.config.Configuration; +import org.apache.activemq.artemis.core.persistence.StorageManager; import org.apache.activemq.artemis.core.server.ActiveMQServer; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; @@ -43,8 +44,10 @@ public class MQTTStateManagerTest { public void testGetSessionStateNeverReturnsNullUnderConcurrentRemoval() throws Exception { final ActiveMQServer server = mock(ActiveMQServer.class); final Configuration configuration = mock(Configuration.class); + final StorageManager storageManager = mock(StorageManager.class); when(server.getConfiguration()).thenReturn(configuration); when(configuration.isMqttSubscriptionPersistenceEnabled()).thenReturn(false); + when(server.getStorageManager()).thenReturn(storageManager); final MQTTStateManager manager = MQTTStateManager.getInstance(server); try { diff --git a/artemis-protocols/artemis-mqtt-protocol/src/test/java/org/apache/activemq/artemis/core/protocol/mqtt/StateSerDeTest.java b/artemis-protocols/artemis-mqtt-protocol/src/test/java/org/apache/activemq/artemis/core/protocol/mqtt/StateSerDeTest.java index 1d5630e99a20..d82dd1501de5 100644 --- a/artemis-protocols/artemis-mqtt-protocol/src/test/java/org/apache/activemq/artemis/core/protocol/mqtt/StateSerDeTest.java +++ b/artemis-protocols/artemis-mqtt-protocol/src/test/java/org/apache/activemq/artemis/core/protocol/mqtt/StateSerDeTest.java @@ -44,17 +44,17 @@ public void testSerDe() throws Exception { RandomUtil.randomBoolean(), RandomUtil.randomBoolean(), MqttSubscriptionOption.RetainedHandlingPolicy.valueOf(RandomUtil.randomInterval(0, 3)))); - unserialized.addSubscription(sub, MQTTUtil.MQTT_WILDCARD, subscriptionIdentifier); + unserialized.addSubscription(SubscriptionItem.of(sub, subscriptionIdentifier)); } CoreMessage serializedState = MQTTStateManager.serializeState(unserialized, 0); MQTTSessionState deserialized = new MQTTSessionState(serializedState); assertEquals(unserialized.getClientId(), deserialized.getClientId()); - for (MQTTSessionState.SubscriptionItem unserializedItem : unserialized.getSubscriptionsPlusID().values()) { + for (SubscriptionItem unserializedItem : unserialized.getSubscriptionsPlusID().values()) { MqttTopicSubscription unserializedSub = unserializedItem.getSubscription(); Integer unserializedSubId = unserializedItem.getId(); - MQTTSessionState.SubscriptionItem deserializedEntry = deserialized.getSubscriptionPlusID(unserializedSub.topicFilter()); + SubscriptionItem deserializedEntry = deserialized.getSubscriptionItem(unserializedSub.topicFilter()); MqttTopicSubscription deserializedSub = deserializedEntry.getSubscription(); Integer deserializedSubId = deserializedEntry.getId(); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalRecordIds.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalRecordIds.java index b29ca6efe7fa..29530ebb7161 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalRecordIds.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalRecordIds.java @@ -105,4 +105,6 @@ public final class JournalRecordIds { public static final byte ADDRESS_SETTING_RECORD_JSON = 52; public static final byte ACK_RETRY = 53; + + public static final byte MQTT_PACKET_ID_CORRELATION = 54; } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/codec/AckRetry.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/codec/AckRetry.java index 9ab9653c57b5..2fc693b7e152 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/codec/AckRetry.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/codec/AckRetry.java @@ -131,11 +131,26 @@ public int hashCode() { return Objects.hash(nodeID, messageID); } - public static class Persister extends AbstractHashMapPersister { + public static class Persister extends AbstractHashMapPersister { private Persister() { } + @Override + protected int getCollectionIdSize(Long collectionID) { + return DataConstants.SIZE_LONG; + } + + @Override + protected void encodeCollectionId(ActiveMQBuffer buffer, Long collectionID) { + buffer.writeLong(collectionID); + } + + @Override + protected Long decodeCollectionId(ActiveMQBuffer buffer) { + return buffer.readLong(); + } + @Override protected int getKeySize(AckRetry key) { return DataConstants.SIZE_INT + diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/DuplicateIDCache.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/DuplicateIDCache.java index 2f0d3269a8dd..9ed47e692372 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/DuplicateIDCache.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/DuplicateIDCache.java @@ -39,7 +39,7 @@ public interface DuplicateIDCache { */ void addToCache(byte[] duplicateID, Transaction tx, boolean instantAdd) throws Exception; - void deleteFromCache(byte[] duplicateID) throws Exception; + boolean deleteFromCache(byte[] duplicateID) throws Exception; void load(List> ids) throws Exception; diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/PostOffice.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/PostOffice.java index f8ecb42f0d50..45c29671da68 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/PostOffice.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/PostOffice.java @@ -191,10 +191,14 @@ Pair redistribute(Message message, void processRoute(Message message, RoutingContext context, boolean direct) throws Exception; + boolean duplicateIDCacheExists(SimpleString address); + DuplicateIDCache getDuplicateIDCache(SimpleString address); DuplicateIDCache getDuplicateIDCache(SimpleString address, int idCacheSize); + void deleteDuplicateCache(SimpleString address) throws Exception; + void sendQueueInfoToQueue(SimpleString queueName, SimpleString address) throws Exception; Object getNotificationLock(); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/InMemoryDuplicateIDCache.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/InMemoryDuplicateIDCache.java index dccce30a31d9..c35e4618001d 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/InMemoryDuplicateIDCache.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/InMemoryDuplicateIDCache.java @@ -77,11 +77,11 @@ public void load(List> ids) throws Exception { } @Override - public void deleteFromCache(byte[] duplicateID) { - deleteFromCache(new ByteArray(duplicateID)); + public boolean deleteFromCache(byte[] duplicateID) { + return deleteFromCache(new ByteArray(duplicateID)); } - private void deleteFromCache(final ByteArray duplicateID) { + private boolean deleteFromCache(final ByteArray duplicateID) { if (logger.isTraceEnabled()) { logger.trace("deleting id = {}", describeID(duplicateID.bytes)); } @@ -102,8 +102,10 @@ private void deleteFromCache(final ByteArray duplicateID) { } } } + return true; } + return false; } private static String describeID(byte[] duplicateID) { diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/NoOpDuplicateIDCache.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/NoOpDuplicateIDCache.java index 5443de2b2fd2..9190487e383f 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/NoOpDuplicateIDCache.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/NoOpDuplicateIDCache.java @@ -51,8 +51,8 @@ public void addToCache(byte[] duplicateID, Transaction tx, boolean instantAdd) t } @Override - public void deleteFromCache(byte[] duplicateID) throws Exception { - + public boolean deleteFromCache(byte[] duplicateID) throws Exception { + return true; } @Override diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PersistentDuplicateIDCache.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PersistentDuplicateIDCache.java index bcccddfa4690..acb202395e36 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PersistentDuplicateIDCache.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PersistentDuplicateIDCache.java @@ -145,11 +145,11 @@ public synchronized void load(final List> ids) throws Excepti } @Override - public void deleteFromCache(byte[] duplicateID) throws Exception { - deleteFromCache(new ByteArray(duplicateID)); + public boolean deleteFromCache(byte[] duplicateID) throws Exception { + return deleteFromCache(new ByteArray(duplicateID)); } - private void deleteFromCache(final ByteArray duplicateID) throws Exception { + private boolean deleteFromCache(final ByteArray duplicateID) throws Exception { if (logger.isTraceEnabled()) { logger.trace("deleting id = {}", describeID(duplicateID.bytes)); } @@ -170,8 +170,10 @@ private void deleteFromCache(final ByteArray duplicateID) throws Exception { storageManager.deleteDuplicateID(recordID); } } + return true; } + return false; } private static String describeID(byte[] duplicateID) { @@ -322,15 +324,19 @@ public synchronized void clear() throws Exception { logger.debug("address = {} removing duplicate ID data", address); final int idsSize = ids.size(); if (idsSize > 0) { + boolean deleted = false; long tx = storageManager.generateID(); for (int i = 0; i < idsSize; i++) { final ObjLongPair id = ids.get(i); if (id.getA() != null) { assert id.getB() != NIL; storageManager.deleteDuplicateIDTransactional(tx, id.getB()); + deleted = true; } } - storageManager.commit(tx); + if (deleted) { + storageManager.commit(tx); + } } ids.clear(); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java index 6ee9a03a1a9e..1dd2ef5b428a 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java @@ -1051,7 +1051,8 @@ public synchronized Binding removeBinding(final SimpleString uniqueName, } } - private void deleteDuplicateCache(SimpleString address) throws Exception { + @Override + public void deleteDuplicateCache(SimpleString address) throws Exception { DuplicateIDCache cache = duplicateIDCaches.remove(address); if (cache != null) { @@ -1476,6 +1477,11 @@ private int resolveIdCacheSize(SimpleString address) { return Objects.requireNonNullElse(addressSettingsRepository.getMatch(address.toString()).getIDCacheSize(), idCacheSize); } + @Override + public boolean duplicateIDCacheExists(final SimpleString address) { + return duplicateIDCaches.containsKey(address); + } + @Override public DuplicateIDCache getDuplicateIDCache(final SimpleString address) { int resolvedIdCacheSize = resolveIdCacheSize(address); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/invm/InVMAcceptor.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/invm/InVMAcceptor.java index 0d258081af78..44dcb821a012 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/invm/InVMAcceptor.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/invm/InVMAcceptor.java @@ -180,7 +180,7 @@ public void notifyStop() { try { notificationService.sendNotification(notification); } catch (Exception e) { - ActiveMQServerLogger.LOGGER.failedToSendNotification(e); + ActiveMQServerLogger.LOGGER.failedToSendNotification(notification.toString(), e.getMessage()); } } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java index 7943f4e19763..1bd19470e876 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java @@ -900,7 +900,7 @@ public void notifyStop() { try { notificationService.sendNotification(notification); } catch (Exception e) { - ActiveMQServerLogger.LOGGER.failedToSendNotification(e); + ActiveMQServerLogger.LOGGER.failedToSendNotification(notification.toString(), e.getMessage()); } } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/RemotingService.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/RemotingService.java index 652c89fd86fe..d1e0a3cf7a60 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/RemotingService.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/RemotingService.java @@ -75,6 +75,8 @@ default int getConnectionCount() { boolean removeOutgoingInterceptor(BaseInterceptor interceptor); + void clearInterceptors(); + void notifyStop(); /** The Prepare stop will close all the connections however it will use the one used by storage manager */ diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java index f367ddf2d427..ef30d9b4064f 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/server/impl/RemotingServiceImpl.java @@ -608,7 +608,11 @@ public synchronized ReusableLatch getConnectionCountLatch() { @Override public void loadProtocolServices(List protocolServices) { for (ProtocolManagerFactory protocolManagerFactory : protocolMap.values()) { - protocolManagerFactory.loadProtocolServices(this.server, protocolServices); + try { + protocolManagerFactory.loadProtocolServices(this.server, protocolServices); + } catch (Exception e) { + logger.warn("Unable to load protocol services for: {}", protocolManagerFactory.getProtocols(), e); + } } } @@ -880,6 +884,13 @@ public boolean removeOutgoingInterceptor(final BaseInterceptor interceptor) { } } + @Override + public void clearInterceptors() { + outgoingInterceptors.clear(); + incomingInterceptors.clear(); + updateProtocols(); + } + private ClusterConnection lookupClusterConnection(TransportConfiguration acceptorConfig) { String clusterConnectionName = (String) acceptorConfig.getParams().get(org.apache.activemq.artemis.core.remoting.impl.netty.TransportConstants.CLUSTER_CONNECTION); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQMessageBundle.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQMessageBundle.java index 0ca2902c56f1..35cf342bc07c 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQMessageBundle.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQMessageBundle.java @@ -130,7 +130,7 @@ public interface ActiveMQMessageBundle { @Message(id = 229026, value = "Backup Server was not yet in sync with live") ActiveMQIllegalStateException backupServerNotInSync(); - @Message(id = 229027, value = "Could not find reference on consumer ID={}, messageId = {} queue = {}") + @Message(id = 229027, value = "Could not find reference on consumerId = {}, messageId = {} queue = {}") ActiveMQIllegalStateException consumerNoReference(Long id, Long messageID, SimpleString name); @Message(id = 229028, value = "Consumer {} doesn't exist on the server") diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java index 4f3369007761..f27e0df700f0 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java @@ -882,8 +882,8 @@ void slowConsumerDetected(String sessionID, @LogMessage(id = 222229, value = "Failed to perform rollback", level = LogMessage.Level.WARN) void failedToPerformRollback(IllegalStateException e); - @LogMessage(id = 222230, value = "Failed to send notification", level = LogMessage.Level.WARN) - void failedToSendNotification(Exception e); + @LogMessage(id = 222230, value = "Failed to send notification: {}; Exception message: {}", level = LogMessage.Level.WARN) + void failedToSendNotification(String notification, String exceptionMessage); @LogMessage(id = 222231, value = "Failed to flush outstanding data from the connection", level = LogMessage.Level.WARN) void failedToFlushOutstandingDataFromTheConnection(Throwable e); @@ -1550,4 +1550,10 @@ void slowConsumerDetected(String sessionID, @LogMessage(id = 224164, value = "Failed to recover stored configuration for divert named '{}': {}. To repair this record create a new divert with the same name via the management API.", level = LogMessage.Level.WARN) void failedToRecoverStoredDivertConfiguration(String divertName, String divert); + + @LogMessage(id = 224165, value = "Server is stopping. Unable to process redelivery during rollback; ref: {}; transaction: {}; exception message: {}", level = LogMessage.Level.WARN) + void unableToProcessRedeliveryDuringRollback(String messageRef, String transaction, String exceptionMessage); + + @LogMessage(id = 224166, value = "Server is stopping. Unable to delete unreferenced message with id={}.", level = LogMessage.Level.WARN) + void unableToDeleteMessageDuringShutdown(long messageId); } \ No newline at end of file diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java index ee7e22817a0a..049028ee8f7c 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java @@ -3711,7 +3711,11 @@ private void loadProtocolServices() { private void startProtocolServices() throws Exception { for (ProtocolManagerFactory protocolManagerFactory : protocolManagerFactories) { - protocolManagerFactory.loadProtocolServices(this, protocolServices); + try { + protocolManagerFactory.loadProtocolServices(this, protocolServices); + } catch (Exception e) { + logger.warn("Unable to load protocol services for: {}", protocolManagerFactory.getProtocols(), e); + } } for (ActiveMQComponent protocolComponent : protocolServices) { diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java index 04a47ad3aa4e..387e25aa71ff 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java @@ -49,6 +49,7 @@ import org.apache.activemq.artemis.api.core.ActiveMQException; import org.apache.activemq.artemis.api.core.ActiveMQNullRefException; import org.apache.activemq.artemis.api.core.ActiveMQQueueExistsException; +import org.apache.activemq.artemis.api.core.ActiveMQShutdownException; import org.apache.activemq.artemis.api.core.Message; import org.apache.activemq.artemis.api.core.Pair; import org.apache.activemq.artemis.api.core.QueueConfiguration; @@ -4021,6 +4022,8 @@ public void postAcknowledge(final MessageReference ref, AckReason reason, boolea // There is a startup check to remove non referenced messages case these deletes fail try { storageManager.deleteMessage(message.getMessageID()); + } catch (ActiveMQShutdownException e) { + ActiveMQServerLogger.LOGGER.unableToDeleteMessageDuringShutdown(message.getMessageID()); } catch (Exception e) { ActiveMQServerLogger.LOGGER.cannotFindMessageOnJournal(message.getMessageID(), e); } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/RefsOperation.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/RefsOperation.java index 8ef9afffcab5..800bd65a8644 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/RefsOperation.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/RefsOperation.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; +import org.apache.activemq.artemis.api.core.ActiveMQShutdownException; import org.apache.activemq.artemis.api.core.Message; import org.apache.activemq.artemis.api.core.RefCountMessage; import org.apache.activemq.artemis.core.paging.cursor.PagedReference; @@ -117,6 +118,8 @@ public void afterRollback(final Transaction tx) { ackedRefs.add(ref); } rollbackRedelivery(tx, ref, timeBase, queueMap); + } catch (ActiveMQShutdownException e) { + ActiveMQServerLogger.LOGGER.unableToProcessRedeliveryDuringRollback(ref.toString(), tx.toString(), e.getMessage()); } catch (Exception e) { ActiveMQServerLogger.LOGGER.errorCheckingDLQ(e); } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java index 01764ae8e589..be1940f73543 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java @@ -513,10 +513,11 @@ private void sendSessionNotification(final CoreNotificationType type) throws Exc props.putSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME, SimpleString.of(stableRemotingConnection.getProtocolName())); props.putSimpleStringProperty(ManagementHelper.HDR_ADDRESS, managementService.getManagementNotificationAddress()); props.putIntProperty(ManagementHelper.HDR_DISTANCE, 0); + Notification notification = new Notification(null, type, props); try { - managementService.sendNotification(new Notification(null, type, props)); + managementService.sendNotification(notification); } catch (Exception e) { - ActiveMQServerLogger.LOGGER.failedToSendNotification(e); + ActiveMQServerLogger.LOGGER.failedToSendNotification(notification.toString(), e.getMessage()); } }); } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/protocol/AbstractProtocolManagerFactory.java b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/protocol/AbstractProtocolManagerFactory.java index 96be40f88061..955ed86dff01 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/protocol/AbstractProtocolManagerFactory.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/protocol/AbstractProtocolManagerFactory.java @@ -46,7 +46,7 @@ protected List

internalFilterInterceptors(Class

type, List services) { + public void loadProtocolServices(ActiveMQServer server, List services) throws Exception { } @Override diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/protocol/ProtocolManagerFactory.java b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/protocol/ProtocolManagerFactory.java index d61f0a09ad28..b16f7e34601e 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/protocol/ProtocolManagerFactory.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/protocol/ProtocolManagerFactory.java @@ -50,7 +50,7 @@ ProtocolManager createProtocolManager(ActiveMQServer server, String getModuleName(); - void loadProtocolServices(ActiveMQServer server, List services); + void loadProtocolServices(ActiveMQServer server, List services) throws Exception; /** * Provides an entry point for the server to trigger the protocol manager factory to update its protocol services diff --git a/docs/user-manual/versions.adoc b/docs/user-manual/versions.adoc index 3b6653a77790..c1259f303bc7 100644 --- a/docs/user-manual/versions.adoc +++ b/docs/user-manual/versions.adoc @@ -28,6 +28,18 @@ https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12315920&versio * Due to https://issues.apache.org/jira/browse/ARTEMIS-6128[ARTEMIS-6128] the `Alder32` and `fileAlder32` properties have been deprecated in favor of `Adler32` and `fileAdler32` respectively. * Due to https://issues.apache.org/jira/browse/ARTEMIS-6172[ARTEMIS-6172] the HTTP-specific transport parameters deprecated by https://issues.apache.org/jira/browse/ARTEMIS-5819[ARTEMIS-5819] (`httpClientIdleTime`, `httpClientIdleScanPeriod`, `httpResponseTime`, `httpServerScanPeriod`) are restored. These are required so HTTP-tunneled Core connections can create response slots for server-initiated packets (for example cluster topology updates) without starving replies to client requests. +* Due to https://issues.apache.org/jira/browse/ARTEMIS-6189[ARTEMIS-6189] MQTT QoS 1 and QoS 2 message delivery has been redesigned for resiliency across broker restarts and client reconnects. +Additional persistent state is now tracked via duplicate ID caches (named `mqtt.qos.publish.` and `mqtt.qos.pubrec.`) and packet ID correlation records. +These are internal implementation details, but they will be visible in data tool output. ++ +The per-client internal management queues previously used for QoS 2 PUBREL persistence (named `$sys.mqtt.queue.qos2.`) are no longer used. +It is recommended to allow all in-flight QoS 1 and QoS 2 flows to complete before upgrading so that QoS flow state information is not orphaned in the existing management queues. +Manual clean-up of these queues may be required after upgrading. ++ +Due to the additional persistent state there will be a performance impact. +Of course, this is a traditional trade-off for increased reliability. +The exact impact will depend on your specific use-case. +Generally speaking, the recommendation is to use the lowest QoS possible to avoid unnecessary overhead. == Version 2.55.0 diff --git a/etc/checkstyle.xml b/etc/checkstyle.xml index de6f3ab67c91..2063e4ce6079 100644 --- a/etc/checkstyle.xml +++ b/etc/checkstyle.xml @@ -88,6 +88,9 @@ under the License. + + + diff --git a/pom.xml b/pom.xml index c6e435b89139..23ad397c4f0f 100644 --- a/pom.xml +++ b/pom.xml @@ -174,6 +174,7 @@ 4.13.2 2.3.9 1.2.5 + 1.3.17 42.7.13 1.21.4 4.46.0 diff --git a/tests/artemis-test-support/src/main/java/org/apache/activemq/artemis/tests/unit/core/server/impl/fakes/FakePostOffice.java b/tests/artemis-test-support/src/main/java/org/apache/activemq/artemis/tests/unit/core/server/impl/fakes/FakePostOffice.java index 66e7a3638964..63b28a5c4bb5 100644 --- a/tests/artemis-test-support/src/main/java/org/apache/activemq/artemis/tests/unit/core/server/impl/fakes/FakePostOffice.java +++ b/tests/artemis-test-support/src/main/java/org/apache/activemq/artemis/tests/unit/core/server/impl/fakes/FakePostOffice.java @@ -204,6 +204,10 @@ public DuplicateIDCache getDuplicateIDCache(final SimpleString address, int idSi return DuplicateIDCaches.inMemory(address, idSize); } + @Override + public void deleteDuplicateCache(SimpleString address) throws Exception { + } + @Override public Collection getMatchingBindings(final SimpleString address) { @@ -293,6 +297,11 @@ public RoutingStatus route(Message message, RoutingContext context, boolean dire public void processRoute(Message message, RoutingContext context, boolean direct) throws Exception { } + @Override + public boolean duplicateIDCacheExists(SimpleString address) { + return false; + } + @Override public RoutingStatus route(Message message, boolean direct) throws Exception { return RoutingStatus.OK; diff --git a/tests/compatibility-tests/src/main/resources/ackManager/ackRetryEncoding.groovy b/tests/compatibility-tests/src/main/resources/ackManager/ackRetryEncoding.groovy new file mode 100644 index 000000000000..7b99b0c19598 --- /dev/null +++ b/tests/compatibility-tests/src/main/resources/ackManager/ackRetryEncoding.groovy @@ -0,0 +1,78 @@ +package ackManager + +/* + * 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. + */ + +import org.apache.activemq.artemis.api.core.ActiveMQBuffers +import org.apache.activemq.artemis.core.persistence.impl.journal.codec.AckRetry +import org.apache.activemq.artemis.core.journal.collections.JournalHashMap +import org.apache.activemq.artemis.core.server.impl.AckReason +import org.apache.activemq.artemis.tests.compatibility.GroovyRun + +import java.nio.file.Files +import java.nio.file.Paths + +file = arg[0] +method = arg[1] + +if (method.equals("write")) { + def persister = AckRetry.getPersister() + def buffer = ActiveMQBuffers.dynamicBuffer(1024) + + buffer.writeInt(3) + + def ack1 = new AckRetry("test-node-id-1", 12345L, AckReason.NORMAL) + def rec1 = new JournalHashMap.MapRecord(100L, 1L, ack1, ack1) + persister.encode(buffer, rec1) + + def ack2 = new AckRetry("test-node-id-2", 67890L, AckReason.EXPIRED) + def rec2 = new JournalHashMap.MapRecord(200L, 2L, ack2, ack2) + persister.encode(buffer, rec2) + + def ack3 = new AckRetry(null, 99999L, AckReason.NORMAL) + def rec3 = new JournalHashMap.MapRecord(300L, 3L, ack3, ack3) + persister.encode(buffer, rec3) + + byte[] bytes = new byte[buffer.readableBytes()] + buffer.readBytes(bytes) + + Files.write(Paths.get(file), bytes) +} else { + byte[] bytes = Files.readAllBytes(Paths.get(file)) + + def buffer = ActiveMQBuffers.wrappedBuffer(bytes) + + int count = buffer.readInt() + GroovyRun.assertEquals(3, count) + + def persister = AckRetry.getPersister() + + def rec1 = persister.decode(buffer, null, null) + GroovyRun.assertEquals("test-node-id-1", rec1.key.getNodeID()) + GroovyRun.assertEquals(12345L, rec1.key.getMessageID()) + GroovyRun.assertEquals(1L, rec1.id) + + def rec2 = persister.decode(buffer, null, null) + GroovyRun.assertEquals("test-node-id-2", rec2.key.getNodeID()) + GroovyRun.assertEquals(67890L, rec2.key.getMessageID()) + GroovyRun.assertEquals(2L, rec2.id) + + def rec3 = persister.decode(buffer, null, null) + GroovyRun.assertNull(rec3.key.getNodeID()) + GroovyRun.assertEquals(99999L, rec3.key.getMessageID()) + GroovyRun.assertEquals(3L, rec3.id) +} diff --git a/tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/AckRetryCompatibilityTest.java b/tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/AckRetryCompatibilityTest.java new file mode 100644 index 000000000000..82ce1309b0fb --- /dev/null +++ b/tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/AckRetryCompatibilityTest.java @@ -0,0 +1,48 @@ +/* + * 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.artemis.tests.compatibility; + +import static org.apache.activemq.artemis.tests.compatibility.GroovyRun.ARTEMIS_2_44_0; +import static org.apache.activemq.artemis.tests.compatibility.GroovyRun.SNAPSHOT; + +import java.io.File; + +import org.apache.activemq.artemis.tests.compatibility.base.ClasspathBase; +import org.junit.jupiter.api.Test; + +public class AckRetryCompatibilityTest extends ClasspathBase { + + @Test + public void testAckRetryEncoding_2_44_0_versus_Snapshot() throws Exception { + ClassLoader two_44_classloader = getClasspath(ARTEMIS_2_44_0); + ClassLoader snapshot = getClasspath(SNAPSHOT); + testAckRetryEncodeDecode(two_44_classloader, snapshot); + } + + @Test + public void testAckRetryEncodingSnapshot() throws Exception { + ClassLoader snapshot = getClasspath(SNAPSHOT); + testAckRetryEncodeDecode(snapshot, snapshot); + } + + private void testAckRetryEncodeDecode(ClassLoader senderLoader, ClassLoader receiverLoader) throws Exception { + File file = File.createTempFile("ackRetry", ".bin", serverFolder); + evaluate(senderLoader, "ackManager/ackRetryEncoding.groovy", file.getAbsolutePath(), "write"); + evaluate(receiverLoader, "ackManager/ackRetryEncoding.groovy", file.getAbsolutePath(), "read"); + } +} diff --git a/tests/integration-tests/pom.xml b/tests/integration-tests/pom.xml index fddff750a416..7987103b4ab9 100644 --- a/tests/integration-tests/pom.xml +++ b/tests/integration-tests/pom.xml @@ -195,6 +195,11 @@ org.eclipse.paho.mqttv5.client test + + com.hivemq + hivemq-mqtt-client + test + jakarta.resource jakarta.resource-api diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/connect/AckManagerTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/connect/AckManagerTest.java index c237b0de4398..04124e8fbaae 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/connect/AckManagerTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/connect/AckManagerTest.java @@ -184,16 +184,16 @@ public void testDirectACK() throws Throwable { logger.info("Repeating {}", repeat); AckManager ackManager = AckManagerProvider.getManager(server1); - Map>> sortedRetries = ackManager.sortRetries(); + Map>> sortedRetries = ackManager.sortRetries(); assertEquals(1, sortedRetries.size()); - LongObjectHashMap> acksOnAddress = sortedRetries.get(c1s1.getAddress()); + LongObjectHashMap> acksOnAddress = sortedRetries.get(c1s1.getAddress()); assertEquals(2, acksOnAddress.size()); - JournalHashMap acksOnc1s1 = acksOnAddress.get(c1s1.getID()); - JournalHashMap acksOnc2s2 = acksOnAddress.get(c2s2.getID()); + JournalHashMap acksOnc1s1 = acksOnAddress.get(c1s1.getID()); + JournalHashMap acksOnc2s2 = acksOnAddress.get(c2s2.getID()); assertEquals(numberOfAcksC1, acksOnc1s1.size()); assertEquals(numberOfAcksC2, acksOnc2s2.size()); @@ -217,11 +217,11 @@ public void testDirectACK() throws Throwable { AckManager ackManager = AckManagerProvider.getManager(server1); ackManager.start(); - Map>> sortedRetries = ackManager.sortRetries(); + Map>> sortedRetries = ackManager.sortRetries(); assertEquals(1, sortedRetries.size()); - LongObjectHashMap> acksOnAddress = sortedRetries.get(c1s1.getAddress()); - JournalHashMap acksOnc1s1 = acksOnAddress.get(c1s1.getID()); - JournalHashMap acksOnc2s2 = acksOnAddress.get(c2s2.getID()); + LongObjectHashMap> acksOnAddress = sortedRetries.get(c1s1.getAddress()); + JournalHashMap acksOnc1s1 = acksOnAddress.get(c1s1.getID()); + JournalHashMap acksOnc2s2 = acksOnAddress.get(c2s2.getID()); Wait.assertEquals(0, () -> acksOnc1s1.size()); Wait.assertEquals(0, () -> acksOnc2s2.size()); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/JournalHashMapTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/JournalHashMapTest.java index 3321e768dec5..5e2dd9814765 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/JournalHashMapTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/journal/JournalHashMapTest.java @@ -24,8 +24,11 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicLong; +import java.util.function.IntFunction; +import java.util.function.Supplier; import org.apache.activemq.artemis.api.core.ActiveMQBuffer; +import org.apache.activemq.artemis.api.core.SimpleString; import org.apache.activemq.artemis.core.io.nio.NIOSequentialFileFactory; import org.apache.activemq.artemis.core.journal.IOCompletion; import org.apache.activemq.artemis.core.journal.Journal; @@ -87,6 +90,18 @@ public void deleteMapRecordTx(long txid, long id) throws Exception { @Test public void testHashMap() throws Exception { + doTestHashMap(new LongPersister(), 1L, i -> (long) i, RandomUtil::randomLong); + } + + @Test + public void testHashMapString() throws Exception { + doTestHashMap(new StringPersister(), "collection-1", i -> "key-" + i, () -> RandomUtil.randomAlphaNumericString(10)); + } + + private void doTestHashMap(AbstractHashMapPersister persister, + I collectionId, + IntFunction keyProducer, + Supplier valueProducer) throws Exception { ExecutorService service = Executors.newFixedThreadPool(10); runAfter(service::shutdownNow); OrderedExecutorFactory executorFactory = new OrderedExecutorFactory(service); @@ -102,22 +117,21 @@ public void testHashMap() throws Exception { AtomicLong sequence = new AtomicLong(1); - JournalHashMapProvider journalHashMapProvider = new JournalHashMapProvider(sequence::incrementAndGet, new JournalManager(journal), new LongPersister(), (byte)3, OperationContextImpl::getContext, l -> null, (e, m, f) -> { + JournalHashMapProvider journalHashMapProvider = new JournalHashMapProvider<>(sequence::incrementAndGet, new JournalManager(journal), persister, (byte)3, OperationContextImpl::getContext, l -> null, (e, m, f) -> { e.printStackTrace(); }); - JournalHashMap journalHashMap = journalHashMapProvider.getMap(1); + JournalHashMap journalHashMap = journalHashMapProvider.getMap(collectionId); - for (long i = 0; i < 1000; i++) { - journalHashMap.put(i, RandomUtil.randomLong()); + for (int i = 0; i < 1000; i++) { + journalHashMap.put(keyProducer.apply(i), valueProducer.get()); } - /// repeating to make sure the remove works fine - for (long i = 0; i < 1000; i++) { - journalHashMap.put(i, RandomUtil.randomLong()); + // repeating to make sure the remove works fine + for (int i = 0; i < 1000; i++) { + journalHashMap.put(keyProducer.apply(i), valueProducer.get()); } - journal.flush(); journal.stop(); @@ -126,35 +140,46 @@ public void testHashMap() throws Exception { journal.start(); - List recordInfos = new ArrayList<>(); List preparedTransactions = new ArrayList<>(); journal.load(recordInfos, preparedTransactions, (a, b, c) -> { }, true); - List> records = new ArrayList<>(); recordInfos.forEach(r -> { assertEquals((byte)3, r.userRecordType); journalHashMapProvider.reload(r); }); - List> existingLists = journalHashMapProvider.getMaps(); + List> existingLists = journalHashMapProvider.getMaps(); assertEquals(1, existingLists.size()); - JournalHashMap reloadedList = existingLists.get(0); + JournalHashMap reloadedList = existingLists.get(0); assertEquals(journalHashMap.size(), reloadedList.size()); journalHashMap.forEach((a, b) -> assertEquals(b, reloadedList.get(a))); - } - - private static class LongPersister extends AbstractHashMapPersister { + private static class LongPersister extends AbstractHashMapPersister { @Override public byte getID() { return 0; } + @Override + protected int getCollectionIdSize(Long collectionID) { + return DataConstants.SIZE_LONG; + } + + @Override + protected void encodeCollectionId(ActiveMQBuffer buffer, Long collectionID) { + buffer.writeLong(collectionID); + } + + @Override + protected Long decodeCollectionId(ActiveMQBuffer buffer) { + return buffer.readLong(); + } + @Override protected int getKeySize(Long key) { return DataConstants.SIZE_LONG; @@ -186,4 +211,61 @@ protected Long decodeValue(ActiveMQBuffer buffer, Long key) { return buffer.readLong(); } } + + private static class StringPersister extends AbstractHashMapPersister { + + @Override + public byte getID() { + return 0; + } + + private int sizeOfString(String s) { + return DataConstants.SIZE_INT + s.length() * DataConstants.SIZE_CHAR; + } + + @Override + protected int getCollectionIdSize(String collectionID) { + return sizeOfString(collectionID); + } + + @Override + protected void encodeCollectionId(ActiveMQBuffer buffer, String collectionID) { + buffer.writeSimpleString(SimpleString.of(collectionID)); + } + + @Override + protected String decodeCollectionId(ActiveMQBuffer buffer) { + return buffer.readSimpleString().toString(); + } + + @Override + protected int getKeySize(String key) { + return sizeOfString(key); + } + + @Override + protected void encodeKey(ActiveMQBuffer buffer, String key) { + buffer.writeSimpleString(SimpleString.of(key)); + } + + @Override + protected String decodeKey(ActiveMQBuffer buffer) { + return buffer.readSimpleString().toString(); + } + + @Override + protected int getValueSize(String value) { + return sizeOfString(value); + } + + @Override + protected void encodeValue(ActiveMQBuffer buffer, String value) { + buffer.writeSimpleString(SimpleString.of(value)); + } + + @Override + protected String decodeValue(ActiveMQBuffer buffer, String key) { + return buffer.readSimpleString().toString(); + } + } } diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MQTTTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MQTTTest.java index 03cbcb7a1ebe..ecc773282bc0 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MQTTTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MQTTTest.java @@ -62,7 +62,6 @@ import org.apache.activemq.artemis.core.postoffice.QueueBinding; import org.apache.activemq.artemis.core.protocol.mqtt.MQTTUtil; import org.apache.activemq.artemis.core.server.ActiveMQServer; -import org.apache.activemq.artemis.core.server.Queue; import org.apache.activemq.artemis.core.server.impl.AddressInfo; import org.apache.activemq.artemis.core.settings.impl.AddressSettings; import org.apache.activemq.artemis.json.JsonArray; @@ -100,7 +99,6 @@ public class MQTTTest extends MQTTTestSupport { private static final String AMQP_URI = "tcp://localhost:61616"; - @Override public void configureBroker() throws Exception { super.configureBroker(); @@ -320,30 +318,6 @@ public void testSendAtMostOnceReceiveExactlyOnce() throws Exception { provider.disconnect(); } - @Test - @Timeout(120) - public void testManagementQueueMessagesAreAckd() throws Exception { - String clientId = "test.client.id"; - final MQTTClientProvider provider = getMQTTClientProvider(); - provider.setClientId(clientId); - initializeConnection(provider); - provider.subscribe("foo", EXACTLY_ONCE); - for (int i = 0; i < NUM_MESSAGES; i++) { - String payload = "Test Message: " + i; - provider.publish("foo", payload.getBytes(), EXACTLY_ONCE); - byte[] message = provider.receive(5000); - assertNotNull(message, "Should get a message"); - assertEquals(payload, new String(message)); - } - - final Queue queue = server.locateQueue(SimpleString.of(MQTTUtil.QOS2_MANAGEMENT_QUEUE_PREFIX + clientId)); - - Wait.waitFor(() -> queue.getMessageCount() == 0, 1000, 100); - - assertEquals(0, queue.getMessageCount()); - provider.disconnect(); - } - @Test @Timeout(120) public void testSendAtLeastOnceReceiveExactlyOnce() throws Exception { @@ -2265,6 +2239,7 @@ public void autoDestroyAddress() throws Exception { @Test @Timeout(60) public void testAutoDeleteRetainedQueue() throws Exception { + final int MESSAGE_COUNT = 3; final String TOPIC = "/abc/123"; final String RETAINED_QUEUE = MQTTUtil.getCoreRetainAddressFromMqttTopic(TOPIC, server.getConfiguration().getWildcardConfiguration()); final MQTTClientProvider publisher = getMQTTClientProvider(); @@ -2278,22 +2253,22 @@ public void testAutoDeleteRetainedQueue() throws Exception { String RETAINED = "retained"; publisher.publish(TOPIC, RETAINED.getBytes(), AT_LEAST_ONCE, true); + subscriber.subscribe(TOPIC, AT_LEAST_ONCE); + + byte[] msg = subscriber.receive(5000); + assertNotNull(msg); + assertEquals(RETAINED, new String(msg)); + List messages = new ArrayList<>(); - for (int i = 0; i < 10; i++) { + for (int i = 0; i < MESSAGE_COUNT; i++) { messages.add("TEST MESSAGE:" + i); } - subscriber.subscribe(TOPIC, AT_LEAST_ONCE); - - for (int i = 0; i < 10; i++) { + for (int i = 0; i < MESSAGE_COUNT; i++) { publisher.publish(TOPIC, messages.get(i).getBytes(), AT_LEAST_ONCE); } - byte[] msg = subscriber.receive(5000); - assertNotNull(msg); - assertEquals(RETAINED, new String(msg)); - - for (int i = 0; i < 10; i++) { + for (int i = 0; i < MESSAGE_COUNT; i++) { msg = subscriber.receive(5000); assertNotNull(msg); assertEquals(messages.get(i), new String(msg)); @@ -2314,15 +2289,15 @@ public void testAutoDeleteRetainedQueue() throws Exception { subscriber.subscribe(TOPIC, AT_LEAST_ONCE); - for (int i = 0; i < 10; i++) { - publisher.publish(TOPIC, messages.get(i).getBytes(), AT_LEAST_ONCE); - } - msg = subscriber.receive(5000); assertNotNull(msg); assertEquals(RETAINED, new String(msg)); - for (int i = 0; i < 10; i++) { + for (int i = 0; i < MESSAGE_COUNT; i++) { + publisher.publish(TOPIC, messages.get(i).getBytes(), AT_LEAST_ONCE); + } + + for (int i = 0; i < MESSAGE_COUNT; i++) { msg = subscriber.receive(5000); assertNotNull(msg); assertEquals(messages.get(i), new String(msg)); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MqttClusterRemoteSubscribeTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MqttClusterRemoteSubscribeTest.java index 267b1398b812..04f1a6ea6ca1 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MqttClusterRemoteSubscribeTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MqttClusterRemoteSubscribeTest.java @@ -16,14 +16,21 @@ */ package org.apache.activemq.artemis.tests.integration.mqtt; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import org.apache.activemq.artemis.api.core.QueueConfiguration; import org.apache.activemq.artemis.api.core.RoutingType; +import org.apache.activemq.artemis.api.core.SimpleString; +import org.apache.activemq.artemis.api.core.client.ActiveMQClient; +import org.apache.activemq.artemis.api.core.client.ClientConsumer; +import org.apache.activemq.artemis.api.core.client.ClientMessage; +import org.apache.activemq.artemis.api.core.client.ClientSession; +import org.apache.activemq.artemis.api.core.client.ClientSessionFactory; +import org.apache.activemq.artemis.api.core.client.MessageHandler; +import org.apache.activemq.artemis.api.core.client.ServerLocator; +import org.apache.activemq.artemis.api.core.management.CoreNotificationType; +import org.apache.activemq.artemis.api.core.management.ManagementHelper; import org.apache.activemq.artemis.core.config.CoreAddressConfiguration; import org.apache.activemq.artemis.core.config.WildcardConfiguration; import org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolManager; @@ -43,6 +50,10 @@ import org.fusesource.mqtt.client.Topic; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + public class MqttClusterRemoteSubscribeTest extends ClusterTestBase { @Override @@ -248,11 +259,30 @@ public void useSameClientIdAndMulticastSubscribeRemoteQueue() throws Exception { final String ANYCAST_TOPIC = "anycast/test/1/some/la"; final String subClientId = "subClientId"; final String pubClientId = "pubClientId"; + final String notifications = "notifications"; setupServers(ANYCAST_TOPIC); startServers(0, 1); + ServerLocator locator = addServerLocator(ActiveMQClient.createServerLocator("tcp://localhost:61617")); + ClientSessionFactory sf = createSessionFactory(locator); + ClientSession session = addClientSession(sf.createSession(false, true, true)); + servers[1].createQueue(QueueConfiguration.of(notifications).setAddress(servers[1].getConfiguration().getManagementNotificationAddress()).setRoutingType(RoutingType.MULTICAST)); + ClientConsumer consumer = session.createConsumer(notifications); + AtomicInteger notificationCount = new AtomicInteger(0); + consumer.setMessageHandler(new MessageHandler() { + @Override + public void onMessage(ClientMessage message) { + CoreNotificationType type = CoreNotificationType.valueOf(message.getSimpleStringProperty(ManagementHelper.HDR_NOTIFICATION_TYPE).toString()); + SimpleString protocol = message.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + if (type == CoreNotificationType.SESSION_CREATED && protocol.equals(SimpleString.of("MQTT"))) { + notificationCount.incrementAndGet(); + } + } + }); + session.start(); + BlockingConnection subConnection1 = null; BlockingConnection subConnection2 = null; BlockingConnection pubConnection = null; @@ -263,6 +293,7 @@ public void useSameClientIdAndMulticastSubscribeRemoteQueue() throws Exception { subConnection1 = retrieveMQTTConnection("tcp://localhost:61616", subClientId); Wait.assertEquals(1, locateMQTTPM(servers[0]).getStateManager().getConnectedClients()::size); + Wait.assertEquals(1, () -> notificationCount.get()); subConnection2 = retrieveMQTTConnection("tcp://localhost:61617", subClientId); pubConnection = retrieveMQTTConnection("tcp://localhost:61616", pubClientId); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/PahoMQTTQOS2SecurityTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/PahoMQTTQOS2SecurityTest.java index f01222176403..628be58f0a56 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/PahoMQTTQOS2SecurityTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/PahoMQTTQOS2SecurityTest.java @@ -16,8 +16,9 @@ */ package org.apache.activemq.artemis.tests.integration.mqtt; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.CountDownLatch; import org.apache.activemq.artemis.core.protocol.mqtt.MQTTUtil; import org.apache.activemq.artemis.core.security.Role; @@ -34,9 +35,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; -import java.util.HashSet; -import java.util.Set; -import java.util.concurrent.CountDownLatch; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; public class PahoMQTTQOS2SecurityTest extends MQTTTestSupport { @@ -134,7 +134,7 @@ public void testSendQoS2UnauthorizedNotStorePublish() throws Exception { } catch (MqttException e) { // ignore } - assertEquals(0, getSessions().get(clientID).getPubRec().size()); + assertEquals(0, getSessions().get(clientID).getPublishCache().size()); producer.close(); } diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/MQTT5Test.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/MQTT5Test.java index dcae8935a29b..1a85c84e8bb9 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/MQTT5Test.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/MQTT5Test.java @@ -728,7 +728,7 @@ public void testConnectionStealingOnMultipleAcceptors() throws Exception { Wait.assertEquals(1, () -> getSessionStates().size(), 2000, 100); assertNotNull(getSessionStates().get(CLIENT_ID)); - assertFalse(client.isConnected()); + Wait.assertFalse(() -> client.isConnected(), 2000, 100); client.close(); client2.disconnect(); @@ -913,13 +913,17 @@ public void testFlowControlAfterDisconnect() throws Exception { // ensure subscriber got message and ack was blocked assertTrue(subscriberLatch.await(500, TimeUnit.MILLISECONDS)); assertTrue(interceptorBlockedLatch.await(500, TimeUnit.MILLISECONDS)); - Wait.assertEquals(1L, () -> mqttSessionState.getOutboundStore().getSendQuota(), 2000, 10); + Wait.assertEquals(1L, () -> mqttSessionState.getSendQuota(), 2000, 10); pendingCountCheckLatch.countDown(); // disconnect subscriber - subscriber.disconnect(); + try { + subscriber.disconnect(); + } catch (Exception e) { + // ignore + } Wait.assertFalse(() -> mqttSessionState.isAttached(), 2000, 50); - assertEquals(0, mqttSessionState.getOutboundStore().getSendQuota()); + assertEquals(0, mqttSessionState.getSendQuota()); assertEquals(1L, subscriptionQueue.getMessageCount()); assertEquals(0L, subscriptionQueue.getMessagesAcknowledged()); assertEquals(0L, subscriptionQueue.getConsumerCount()); @@ -1002,6 +1006,65 @@ public void testSubscriptionQueueRoutingType() throws Exception { assertEquals(RoutingType.MULTICAST, getSubscriptionQueue(topic, clientID).getRoutingType()); } + /** + * This test stresses the synchronization between the broker sending the CONNACK and starting the SubscriptionManager + * and the client sending a SUBSCRIBE. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testConcurrentReconnectAndResubscribe() throws Exception { + final int clientCount = 100; + final int subsPerClient = 100; + final int resubscribeCount = 10; + AtomicBoolean failed = new AtomicBoolean(false); + ExecutorService executorService = Executors.newFixedThreadPool(clientCount); + runAfter(executorService::shutdownNow); + CountDownLatch latch = new CountDownLatch(clientCount); + + for (int c = 0; c < clientCount; c++) { + final String clientId = "client-" + c; + final int clientIndex = c; + executorService.submit(() -> { + MqttClient client = null; + try { + client = createPahoClient(clientId); + MqttConnectionOptions options = new MqttConnectionOptionsBuilder() + .cleanStart(false) + .sessionExpiryInterval(300L) + .build(); + + MqttSubscription[] subs = new MqttSubscription[subsPerClient]; + for (int s = 0; s < subsPerClient; s++) { + subs[s] = new MqttSubscription("topic/" + clientIndex + "/" + s, AT_LEAST_ONCE); + } + + for (int i = 0; i < resubscribeCount; i++) { + client.connect(options); + client.subscribe(subs); + client.disconnect(); + } + client.close(); + } catch (Exception e) { + logger.error("Client {} failed: {}", clientId, e.getMessage(), e); + failed.set(true); + } finally { + if (client != null) { + try { + client.disconnect(); + client.close(); + } catch (MqttException e) { + // ignore + } + } + latch.countDown(); + } + }); + } + + latch.await(); + assertFalse(failed.get()); + } + @Test @Timeout(DEFAULT_TIMEOUT_SEC) public void testPublishWithDelimiterInTopicNameAndWildcardSubscription() throws Exception { diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/MQTT5TestSupport.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/MQTT5TestSupport.java index ce19a1e76e1e..bdc39ff06ea0 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/MQTT5TestSupport.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/MQTT5TestSupport.java @@ -37,7 +37,9 @@ import org.apache.activemq.artemis.api.core.SimpleString; import org.apache.activemq.artemis.api.core.TransportConfiguration; import org.apache.activemq.artemis.core.config.Configuration; +import org.apache.activemq.artemis.core.postoffice.DuplicateIDCache; import org.apache.activemq.artemis.core.protocol.mqtt.MQTTInterceptor; +import org.apache.activemq.artemis.core.protocol.mqtt.PacketIdCache; import org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolManager; import org.apache.activemq.artemis.core.protocol.mqtt.MQTTSessionState; import org.apache.activemq.artemis.core.protocol.mqtt.MQTTUtil; @@ -55,6 +57,11 @@ import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager; import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; import org.apache.activemq.artemis.utils.ClassloadingUtil; +import org.apache.activemq.artemis.utils.Wait; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.config.LoggerConfig; import org.eclipse.paho.mqttv5.client.IMqttToken; import org.eclipse.paho.mqttv5.client.MqttAsyncClient; import org.eclipse.paho.mqttv5.client.MqttCallback; @@ -73,6 +80,14 @@ import static org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME; public class MQTT5TestSupport extends ActiveMQTestBase { + + // The Paho MQTT client logging adds noise during QoS2 operations + private static final java.util.logging.Logger PAHO_LOGGER; + static { + PAHO_LOGGER = java.util.logging.Logger.getLogger("org.eclipse.paho.mqttv5.client.internal.ClientState"); + PAHO_LOGGER.setLevel(java.util.logging.Level.WARNING); + } + protected static final String TCP = "tcp"; protected static final String WS = "ws"; protected static final String SSL = "ssl"; @@ -158,15 +173,36 @@ public void setUp() throws Exception { exceptions.clear(); startBroker(); createJMSConnection(); + if (isProtocolLoggingEnabled()) { + enableProtocolLogging(); + } } @Override @AfterEach public void tearDown() throws Exception { stopBroker(); + if (isProtocolLoggingEnabled()) { + disableProtocolLogging(); + } super.tearDown(); } + public void enableProtocolLogging() { + LoggerContext ctx = (LoggerContext) LogManager.getContext(false); + org.apache.logging.log4j.core.config.Configuration config = ctx.getConfiguration(); + LoggerConfig loggerConfig = new LoggerConfig(MQTTUtil.class.getName(), Level.TRACE, true); + config.addLogger(MQTTUtil.class.getName(), loggerConfig); + ctx.updateLoggers(); + } + + public void disableProtocolLogging() { + LoggerContext ctx = (LoggerContext) LogManager.getContext(false); + org.apache.logging.log4j.core.config.Configuration config = ctx.getConfiguration(); + config.removeLogger(MQTTUtil.class.getName()); + ctx.updateLoggers(); + } + public void configureBroker() throws Exception { super.setUp(); server = createServerForMQTT(); @@ -305,6 +341,10 @@ public boolean isMutualSsl() { return false; } + public boolean isProtocolLoggingEnabled() { + return false; + } + protected interface Task { void run() throws Exception; @@ -351,6 +391,46 @@ protected void setAcceptorProperty(String property) throws Exception { server.getRemotingService().createAcceptor(MQTT_PROTOCOL_NAME, "tcp://localhost:" + port + "?protocols=MQTT;" + property).start(); } + protected DuplicateIDCache getPubCache(String clientId) { + return getCache(clientId, PacketIdCache.TYPE.PUBLISH); + } + + protected int getPubCacheSize(String clientId) { + return getCacheSize(getPubCache(clientId)); + } + + protected DuplicateIDCache getPubRecCache(String clientId) { + return getCache(clientId, PacketIdCache.TYPE.PUBREC); + } + + protected int getPubRecCacheSize(String clientId) { + return getCacheSize(getPubRecCache(clientId)); + } + + private int getCacheSize(DuplicateIDCache cache) { + return cache == null ? 0 : cache.getMap().size(); + } + + private DuplicateIDCache getCache(String clientId, PacketIdCache.TYPE type) { + SimpleString cacheName = PacketIdCache.getCacheName(server.getInternalNamingPrefix(), clientId, type); + if (server.getPostOffice().duplicateIDCacheExists(cacheName)) { + return server.getPostOffice().getDuplicateIDCache(cacheName); + } else { + return null; + } + } + + protected static void reconnectSafely(MqttClient subscriber) throws Exception { + Wait.waitFor(() -> { + try { + subscriber.reconnect(); + return true; + } catch (MqttException e) { + return false; + } + }); + } + /* * From the Paho MQTT client's JavaDoc: * diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/QoS1SubscriberResiliencyTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/QoS1SubscriberResiliencyTest.java new file mode 100644 index 000000000000..e4eaa71e2428 --- /dev/null +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/QoS1SubscriberResiliencyTest.java @@ -0,0 +1,243 @@ +/* + * 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.artemis.tests.integration.mqtt5; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import io.netty.handler.codec.mqtt.MqttMessageType; +import io.netty.handler.codec.mqtt.MqttPublishMessage; +import org.apache.activemq.artemis.core.protocol.mqtt.MQTTInterceptor; +import org.apache.activemq.artemis.utils.RandomUtil; +import org.apache.activemq.artemis.utils.Wait; +import org.eclipse.paho.mqttv5.client.MqttClient; +import org.eclipse.paho.mqttv5.client.MqttConnectionOptions; +import org.eclipse.paho.mqttv5.client.MqttConnectionOptionsBuilder; +import org.eclipse.paho.mqttv5.common.MqttMessage; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests for QoS 1 protocol resiliency with subscriber reconnections or broker restarts. + *

+ * QoS 1 Protocol Flow (broker sending to subscriber): + *

    + *
  1. Broker sends PUBLISH (QoS=1)
    2. + *
  2. Subscriber sends PUBACK
    3. + *
+ * These tests verify that the protocol maintains at-least-once delivery semantics when either clients + * reconnect or the broker is restarted at each stage of the QoS 1 flow. + */ +public class QoS1SubscriberResiliencyTest extends MQTT5TestSupport { + + protected static final long DEFAULT_TIMEOUT_SEC = 10; + + @Override + public boolean isProtocolLoggingEnabled() { + return true; + } + + /** + * Verifies that the broker will re-use the same packet ID if it sends a PUBLISH but fails to receive the + * corresponding PUBACK. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS1BrokerRestartBeforePubAckSent() throws Exception { + testQoS1FailureBeforePubAckSent(true); + } + + /** + * Same test as {@link testQoS1BrokerRestartBeforePubAckSent} but disconnecting the client instead of restarting the + * broker. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS1ClientDisconnectBeforePubAckSent() throws Exception { + testQoS1FailureBeforePubAckSent(false); + } + + public void testQoS1FailureBeforePubAckSent(boolean restart) throws Exception { + final String TOPIC = "test/resiliency"; + final String SUBSCRIBER_CLIENT_ID = "subscriber"; + final String PUBLISHER_CLIENT_ID = "publisher"; + + // Set up interceptor to block the *second* incoming PUBACK. + // We allow 1 PUBACK so the packet ID goes up to 2 for testing. + final CountDownLatch pubAckLatch = new CountDownLatch(1); + AtomicInteger pubAckCount = new AtomicInteger(0); + final CountDownLatch stopLatch = new CountDownLatch(1); + MQTTInterceptor pubAckInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBACK && pubAckCount.incrementAndGet() > 1) { + pubAckLatch.countDown(); + logger.info("Blocking incoming {}", packet.fixedHeader().messageType()); + try { + stopLatch.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + return false; + } + logger.info("Allowing incoming {}", packet.fixedHeader().messageType()); + return true; + }; + server.getRemotingService().addIncomingInterceptor(pubAckInterceptor); + + // Consumer with persistent session + MqttClient subscriber = createPahoClient(SUBSCRIBER_CLIENT_ID); + subscriber.setCallback(new DefaultMqttCallback() { + @Override + public void messageArrived(String topic, MqttMessage message) throws Exception { + logger.info("messageArrived({}, {})", topic, message); + } + }); + MqttConnectionOptions subscriberOptions = new MqttConnectionOptionsBuilder() + .cleanStart(false) + .sessionExpiryInterval(300L) + .build(); + subscriber.connect(subscriberOptions); + subscriber.subscribe(TOPIC, 1); + + // Producer + MqttClient producer = createPahoClient(PUBLISHER_CLIENT_ID); + producer.connect(); + + // Send 2 messages to ensure the packet ID is preserved by the broker. + // If we just send 1 message it won't be clear if the broker just started generating packet IDs from scratch. + producer.publish(TOPIC, RandomUtil.randomBytes(), 1, false); + producer.publish(TOPIC, RandomUtil.randomBytes(), 1, false); + + producer.disconnect(); + producer.close(); + + assertTrue(pubAckLatch.await(5, TimeUnit.SECONDS)); + + stopLatch.countDown(); + if (restart) { + server.stop(); + waitForServerToStop(server); + server.start(); + waitForServerToStart(server); + } else { + server.getRemotingService().clearInterceptors(); + server.getActiveMQServerControl().closeConnectionWithID(server.getActiveMQServerControl().listConnectionIDs()[0]); + } + + assertTrue(getProtocolManager().getStateManager().packetIdCorrelationExists(SUBSCRIBER_CLIENT_ID, 2)); + + CountDownLatch packetIdLatch = new CountDownLatch(1); + MQTTInterceptor pubInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBLISH) { + if (((MqttPublishMessage)packet).variableHeader().packetId() == 2 && ((MqttPublishMessage)packet).fixedHeader().isDup()) { + packetIdLatch.countDown(); + } + } + return true; + }; + server.getRemotingService().addOutgoingInterceptor(pubInterceptor); + + reconnectSafely(subscriber); + + assertTrue(packetIdLatch.await(5, TimeUnit.SECONDS), "Didn't find a duplicate PUBLISH with the expected packet id"); + + Wait.assertEquals(0L, () -> getSubscriptionQueue(TOPIC, SUBSCRIBER_CLIENT_ID).getMessageCount(), 500, 25); + assertEquals(0, getProtocolManager().getStateManager().getPacketIdCorrelationSize(SUBSCRIBER_CLIENT_ID)); + + subscriber.disconnect(); + subscriber.close(); + } + + /** + * Verifies that after a complete QoS 1 protocol exchange (PUBLISH, PUBACK), a broker restart does + * not cause re-delivery. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS1BrokerRestartAfterPubAckSent() throws Exception { + testQoS1FailureAfterPubAckSent(true); + } + + /** + * Same test as {@link testQoS1BrokerRestartAfterPubAckSent} but disconnecting the client instead of restarting the + * broker. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS1ClientDisconnectAfterPubAckSent() throws Exception { + testQoS1FailureAfterPubAckSent(false); + } + + public void testQoS1FailureAfterPubAckSent(boolean restart) throws Exception { + final String TOPIC = "test/resiliency"; + final String SUBSCRIBER_CLIENT_ID = "subscriber"; + final String PUBLISHER_CLIENT_ID = "publisher"; + AtomicInteger messageCount = new AtomicInteger(0); + + // Subscriber with persistent session + MqttClient subscriber = createPahoClient(SUBSCRIBER_CLIENT_ID); + subscriber.setCallback(new DefaultMqttCallback() { + @Override + public void messageArrived(String topic, MqttMessage message) throws Exception { + messageCount.incrementAndGet(); + logger.info("messageArrived({}, {})", topic, message); + } + }); + MqttConnectionOptions subscriberOptions = new MqttConnectionOptionsBuilder() + .cleanStart(false) + .sessionExpiryInterval(300L) + .build(); + subscriber.connect(subscriberOptions); + subscriber.subscribe(TOPIC, 1); + + MqttClient producer = createPahoClient(PUBLISHER_CLIENT_ID); + producer.connect(); + producer.publish(TOPIC, RandomUtil.randomBytes(), 1, false); + producer.disconnect(); + producer.close(); + + Wait.assertEquals(1L, () -> messageCount.get(), 500, 25); + Wait.assertEquals(1L, () -> getSubscriptionQueue(TOPIC, SUBSCRIBER_CLIENT_ID).getMessagesAcknowledged(), 500, 25); + Wait.assertEquals(0L, () -> getSubscriptionQueue(TOPIC, SUBSCRIBER_CLIENT_ID).getMessageCount(), 500, 25); + + assertEquals(0, getProtocolManager().getStateManager().getPacketIdCorrelationSize(SUBSCRIBER_CLIENT_ID)); + + if (restart) { + server.stop(); + waitForServerToStop(server); + server.start(); + waitForServerToStart(server); + } else { + server.getRemotingService().clearInterceptors(); + server.getActiveMQServerControl().closeConnectionWithID(server.getActiveMQServerControl().listConnectionIDs()[0]); + } + + reconnectSafely(subscriber); + + // Check for any unexpected re-delivery + assertFalse(Wait.waitFor(() -> messageCount.get() > 1, 500, 25)); + + assertEquals(0, getProtocolManager().getStateManager().getPacketIdCorrelationSize(SUBSCRIBER_CLIENT_ID)); + + subscriber.disconnect(); + subscriber.close(); + } +} diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/QoS2PublisherResiliencyTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/QoS2PublisherResiliencyTest.java new file mode 100644 index 000000000000..66d09dcd2141 --- /dev/null +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/QoS2PublisherResiliencyTest.java @@ -0,0 +1,535 @@ +/* + * 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.artemis.tests.integration.mqtt5; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import io.netty.handler.codec.mqtt.MqttMessageType; +import org.apache.activemq.artemis.api.core.QueueConfiguration; +import org.apache.activemq.artemis.api.core.RoutingType; +import org.apache.activemq.artemis.core.protocol.mqtt.MQTTInterceptor; +import org.apache.activemq.artemis.utils.RandomUtil; +import org.apache.activemq.artemis.utils.ReusableLatch; +import org.apache.activemq.artemis.utils.Wait; +import org.eclipse.paho.mqttv5.client.MqttClient; +import org.eclipse.paho.mqttv5.client.MqttConnectionOptions; +import org.eclipse.paho.mqttv5.client.MqttConnectionOptionsBuilder; +import org.eclipse.paho.mqttv5.client.MqttDisconnectResponse; +import org.eclipse.paho.mqttv5.common.MqttException; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests for QoS 2 protocol resiliency with publisher reconnections or broker restarts. + *

+ * QoS 2 Protocol Flow (publisher sending to broker): + *

    + *
  1. Publisher sends PUBLISH (QoS=2)
    2. + *
  2. Broker sends PUBREC
    3. + *
  3. Publisher sends PUBREL
    4. + *
  4. Broker sends PUBCOMP
    5. + *
+ * These tests verify that the protocol maintains exactly-once delivery semantics when either clients + * reconnect or the broker is restarted at each stage of the QoS 2 flow. + */ +public class QoS2PublisherResiliencyTest extends MQTT5TestSupport { + + protected static final long DEFAULT_TIMEOUT_SEC = 10; + + @Override + public boolean isProtocolLoggingEnabled() { + return true; + } + + /** + * Verifies that resending the same message via QoS2 doesn't result in duplicates when the broker is restarted after + * it receives the PUBLISH but before it sends the PUBREC. In this circumstance the client will reconnect and send + * the PUBLISH again with the same packet ID and the dup flag set to true. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2BrokerRestartBeforePubRecSent() throws Exception { + testQoS2FailureBeforePubRecSent(true); + } + + /** + * Same test as {@link testQoS2BrokerRestartBeforePubRecSent} but disconnecting the client instead of restarting the + * broker. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2ClientDisconnectBeforePubRecSent() throws Exception { + testQoS2FailureBeforePubRecSent(false); + } + + public void testQoS2FailureBeforePubRecSent(boolean restart) throws Exception { + final String TOPIC = RandomUtil.randomUUIDString(); + final String CLIENTID = "publisher"; + final CountDownLatch publishLatch = new CountDownLatch(1); + final CountDownLatch stopLatch = new CountDownLatch(1); + final CountDownLatch pubCompLatch = new CountDownLatch(1); + + // Simulate a subscription queue + server.createQueue(QueueConfiguration.of(TOPIC) + .setAddress(TOPIC) + .setRoutingType(RoutingType.MULTICAST) + .setDurable(true)); + + // Set up interceptor block the initial PUBLISH + MQTTInterceptor pubRecInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBLISH) { + publishLatch.countDown(); + logger.info("Blocking incoming {}", packet.fixedHeader().messageType()); + try { + stopLatch.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + return false; + } + logger.info("Allowing incoming {}", packet.fixedHeader().messageType()); + return true; + }; + server.getRemotingService().addIncomingInterceptor(pubRecInterceptor); + + // Producer with persistent session + MqttClient publisher = createPahoClient(CLIENTID); + publisher.setCallback(new DefaultMqttCallback() { + @Override + public void disconnected(MqttDisconnectResponse disconnectResponse) { + logger.info("{} disconnected", CLIENTID); + } + }); + MqttConnectionOptions producerOptions = new MqttConnectionOptionsBuilder() + .cleanStart(false) + .sessionExpiryInterval(300L) + .build(); + publisher.connect(producerOptions); + + assertNull(getPubCache(CLIENTID)); + + // Send message async as it will block waiting for a PUBREC that won't come + CompletableFuture.runAsync(() -> { + try { + publisher.publish(TOPIC, RandomUtil.randomBytes(), EXACTLY_ONCE, false); + } catch (MqttException e) { + logger.info(e.getMessage()); + } + }); + + assertTrue(publishLatch.await(5, TimeUnit.SECONDS)); + stopLatch.countDown(); + if (restart) { + server.stop(); + waitForServerToStop(server); + server.start(); + waitForServerToStart(server); + } else { + server.getRemotingService().clearInterceptors(); + server.getActiveMQServerControl().closeConnectionWithID(server.getActiveMQServerControl().listConnectionIDs()[0]); + } + + MQTTInterceptor pubCompInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBCOMP) { + pubCompLatch.countDown(); + } + return true; + }; + server.getRemotingService().addOutgoingInterceptor(pubCompInterceptor); + + assertNull(getPubCache(CLIENTID)); + + // The client will automatically re-initiate the QoS2 protocol after reconnecting since it never got a PUBREC + reconnectSafely(publisher); + + // Wait for the PUBCOMP to confirm QoS2 protocol is done + assertTrue(pubCompLatch.await(5, TimeUnit.SECONDS)); + + // Verify only one message is in the queue despite the QoS2 interruption + Wait.assertEquals(1L, () -> server.locateQueue(TOPIC).getMessageCount(), 500, 25); + + publisher.disconnect(); + + assertEquals(0, getPubCacheSize(CLIENTID)); + + // connect again to clean the session which will completely remove the cache from memory and disk + publisher.connect(new MqttConnectionOptionsBuilder().cleanStart(true).sessionExpiryInterval(0L).build()); + assertNull(getPubCache(CLIENTID)); + publisher.disconnect(); + publisher.close(); + } + + /** + * Verifies that resending the same message via QoS2 doesn't result in duplicates when the broker is restarted after + * it receives the PUBLISH and sends the PUBREC but before the client receives the PUBREC. In this circumstance the + * client will reconnect and send the PUBLISH again with the same packet ID and the dup flag set to true. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2BrokerRestartAfterPubRecSent() throws Exception { + testQoS2FailureAfterPubRecSent(true); + } + + /** + * Same test as {@link testQoS2BrokerRestartAfterPubRecSent} but disconnecting the client instead of restarting the + * broker. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2ClientDisconnectAfterPubRecSent() throws Exception { + testQoS2FailureAfterPubRecSent(false); + } + + public void testQoS2FailureAfterPubRecSent(boolean restart) throws Exception { + final String TOPIC = RandomUtil.randomUUIDString(); + final String CLIENTID = "publisher"; + final CountDownLatch pubRecLatch = new CountDownLatch(1); + final CountDownLatch pubCompLatch = new CountDownLatch(1); + + // Simulate a subscription queue + server.createQueue(QueueConfiguration.of(TOPIC) + .setAddress(TOPIC) + .setRoutingType(RoutingType.MULTICAST) + .setDurable(true)); + + // Set up interceptor block the initial PUBREC + MQTTInterceptor pubRecInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBREC) { + pubRecLatch.countDown(); + logger.info("Blocking outgoing {}", packet.fixedHeader().messageType()); + return false; + } + + logger.info("Allowing outgoing {}", packet.fixedHeader().messageType()); + return true; + }; + server.getRemotingService().addOutgoingInterceptor(pubRecInterceptor); + + // Producer with persistent session + MqttClient publisher = createPahoClient(CLIENTID); + publisher.setCallback(new DefaultMqttCallback() { + @Override + public void disconnected(MqttDisconnectResponse disconnectResponse) { + logger.info("{} disconnected", CLIENTID); + } + }); + MqttConnectionOptions producerOptions = new MqttConnectionOptionsBuilder() + .cleanStart(false) + .sessionExpiryInterval(300L) + .build(); + publisher.connect(producerOptions); + + assertEquals(0, server.locateQueue(TOPIC).getMessageCount()); + assertNull(getPubCache(CLIENTID)); + + // Send message async as it will block waiting for a PUBREC that won't come + CompletableFuture.runAsync(() -> { + try { + publisher.publish(TOPIC, RandomUtil.randomBytes(), EXACTLY_ONCE, false); + } catch (MqttException e) { + logger.info(e.getMessage()); + } + }); + assertTrue(pubRecLatch.await(5, TimeUnit.SECONDS)); + + if (restart) { + server.stop(); + waitForServerToStop(server); + server.start(); + waitForServerToStart(server); + } else { + server.getRemotingService().clearInterceptors(); + server.getActiveMQServerControl().closeConnectionWithID(server.getActiveMQServerControl().listConnectionIDs()[0]); + } + + MQTTInterceptor pubCompInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBCOMP) { + pubCompLatch.countDown(); + } + return true; + }; + server.getRemotingService().addOutgoingInterceptor(pubCompInterceptor); + + assertEquals(1, getPubCacheSize(CLIENTID)); + + // The client will automatically re-initiate the QoS2 protocol after reconnecting since it never got a PUBREC + reconnectSafely(publisher); + + // Wait for the PUBCOMP to confirm QoS2 protocol is done + assertTrue(pubCompLatch.await(5, TimeUnit.SECONDS)); + + // Verify only one message is in the queue despite the QoS2 interruption + Wait.assertEquals(1L, () -> server.locateQueue(TOPIC).getMessageCount(), 500, 25); + + publisher.disconnect(); + + assertEquals(0, getPubCacheSize(CLIENTID)); + + // connect again to clean the session which will completely remove the cache from memory and disk + publisher.connect(new MqttConnectionOptionsBuilder().cleanStart(true).sessionExpiryInterval(0L).build()); + assertNull(getPubCache(CLIENTID)); + publisher.disconnect(); + publisher.close(); + } + + /** + * Verifies that resending the same message via QoS2 doesn't result in duplicates when the broker is restarted after + * it receives the PUBREL but before it sends the PUBCOMP. In this circumstance the client will reconnect and send + * the PUBREL again with the same packet ID. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2BrokerRestartBeforePubCompSent() throws Exception { + testQoS2FailureBeforePubCompSent(true); + } + + /** + * Same test as {@link testQoS2BrokerRestartBeforePubCompSent} but disconnecting the client instead of restarting the + * broker. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2ClientDisconnectBeforePubCompSent() throws Exception { + testQoS2FailureBeforePubCompSent(false); + } + + public void testQoS2FailureBeforePubCompSent(boolean restart) throws Exception { + final String TOPIC = RandomUtil.randomUUIDString(); + final String CLIENTID = "publisher"; + final CountDownLatch pubRelLatch = new CountDownLatch(1); + final CountDownLatch stopLatch = new CountDownLatch(1); + final CountDownLatch pubCompLatch = new CountDownLatch(1); + + // Simulate a subscription queue + server.createQueue(QueueConfiguration.of(TOPIC) + .setAddress(TOPIC) + .setRoutingType(RoutingType.MULTICAST) + .setDurable(true)); + + // Set up interceptor block the initial PUBREL + MQTTInterceptor pubRelInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBREL) { + pubRelLatch.countDown(); + logger.info("Blocking incoming {}", packet.fixedHeader().messageType()); + try { + assertTrue(stopLatch.await(5, TimeUnit.SECONDS)); + } catch (Exception e) { + throw new RuntimeException(e); + } + return false; + } + logger.info("Allowing incoming {}", packet.fixedHeader().messageType()); + return true; + }; + server.getRemotingService().addIncomingInterceptor(pubRelInterceptor); + + // Producer with persistent session + MqttClient publisher = createPahoClient(CLIENTID); + publisher.setCallback(new DefaultMqttCallback() { + @Override + public void disconnected(MqttDisconnectResponse disconnectResponse) { + logger.info("{} disconnected", CLIENTID); + } + }); + MqttConnectionOptions producerOptions = new MqttConnectionOptionsBuilder() + .cleanStart(false) + .sessionExpiryInterval(300L) + .build(); + publisher.connect(producerOptions); + + assertNull(getPubCache(CLIENTID)); + + // Send message async as it will block waiting for a PUBCOMP that won't come + CompletableFuture.runAsync(() -> { + try { + publisher.publish(TOPIC, RandomUtil.randomBytes(), EXACTLY_ONCE, false); + } catch (MqttException e) { + logger.info(e.getMessage()); + } + }); + assertTrue(pubRelLatch.await(5, TimeUnit.SECONDS)); + + stopLatch.countDown(); + + if (restart) { + server.stop(); + waitForServerToStop(server); + server.start(); + waitForServerToStart(server); + } else { + server.getRemotingService().clearInterceptors(); + server.getActiveMQServerControl().closeConnectionWithID(server.getActiveMQServerControl().listConnectionIDs()[0]); + } + + MQTTInterceptor pubCompInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBCOMP) { + pubCompLatch.countDown(); + } + return true; + }; + server.getRemotingService().addOutgoingInterceptor(pubCompInterceptor); + + assertEquals(1, getPubCacheSize(CLIENTID)); + + // The client will automatically re-initiate the QoS2 protocol after reconnecting since it never got a PUBCOMP + reconnectSafely(publisher); + + // Wait for the PUBCOMP to confirm QoS2 protocol is done + assertTrue(pubCompLatch.await(5, TimeUnit.SECONDS)); + + // Verify only one message is in the queue despite the QoS2 interruption + Wait.assertEquals(1L, () -> server.locateQueue(TOPIC).getMessageCount(), 500, 25); + + publisher.disconnect(); + + assertEquals(0, getPubCacheSize(CLIENTID)); + + // connect again to clean the session which will completely remove the cache from memory and disk + publisher.connect(new MqttConnectionOptionsBuilder().cleanStart(true).sessionExpiryInterval(0L).build()); + assertNull(getPubCache(CLIENTID)); + publisher.disconnect(); + publisher.close(); + } + + /** + * Verifies that resending the same message via QoS2 doesn't result in duplicates when the broker is restarted after + * it receives the PUBREL and sends the PUBCOMP but before the client receives the PUBCOMP. In this circumstance the + * client will reconnect and send the PUBREL again with the same packet ID. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2BrokerRestartAfterPubCompSent() throws Exception { + testQoS2FailureAfterPubCompSent(true); + } + + /** + * Same test as {@link testQoS2BrokerRestartAfterPubCompSent} but disconnecting the client instead of restarting the + * broker. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2ClientDisconnectAfterPubCompSent() throws Exception { + testQoS2FailureAfterPubCompSent(false); + } + + public void testQoS2FailureAfterPubCompSent(boolean restart) throws Exception { + final String TOPIC = RandomUtil.randomUUIDString(); + final String CLIENTID = "publisher"; + final ReusableLatch pubCompLatch = new ReusableLatch(1); + + // Simulate a subscription queue + server.createQueue(QueueConfiguration.of(TOPIC) + .setAddress(TOPIC) + .setRoutingType(RoutingType.MULTICAST) + .setDurable(true)); + + // Set up interceptor block the initial PUBCOMP + MQTTInterceptor initialPubCompInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBCOMP) { + pubCompLatch.countDown(); + logger.info("Blocking outgoing {}", packet.fixedHeader().messageType()); + return false; + } + + logger.info("Allowing outgoing {}", packet.fixedHeader().messageType()); + return true; + }; + server.getRemotingService().addOutgoingInterceptor(initialPubCompInterceptor); + + // Producer with persistent session + MqttClient publisher = createPahoClient(CLIENTID); + publisher.setCallback(new DefaultMqttCallback() { + @Override + public void disconnected(MqttDisconnectResponse disconnectResponse) { + logger.info("{} disconnected", CLIENTID); + } + }); + MqttConnectionOptions producerOptions = new MqttConnectionOptionsBuilder() + .cleanStart(false) + .sessionExpiryInterval(300L) + .build(); + publisher.connect(producerOptions); + + assertEquals(0, server.locateQueue(TOPIC).getMessageCount()); + assertNull(getPubCache(CLIENTID)); + + // Send message async as it will block waiting for a PUBREC that won't come + CompletableFuture.runAsync(() -> { + try { + publisher.publish(TOPIC, RandomUtil.randomBytes(), EXACTLY_ONCE, false); + } catch (MqttException e) { + logger.info(e.getMessage()); + } + }); + assertTrue(pubCompLatch.await(5, TimeUnit.SECONDS)); + + if (restart) { + server.stop(); + waitForServerToStop(server); + server.start(); + waitForServerToStart(server); + } else { + server.getRemotingService().clearInterceptors(); + server.getActiveMQServerControl().closeConnectionWithID(server.getActiveMQServerControl().listConnectionIDs()[0]); + } + + pubCompLatch.countUp(); + MQTTInterceptor pubCompInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBCOMP) { + pubCompLatch.countDown(); + } + return true; + }; + server.getRemotingService().addOutgoingInterceptor(pubCompInterceptor); + + if (restart) { + assertNull(getPubCache(CLIENTID)); + } else { + assertEquals(0, getPubCacheSize(CLIENTID)); + } + + // The client will automatically re-initiate the QoS2 protocol after reconnecting since it never got a PUBCOMP + reconnectSafely(publisher); + + // Wait for the PUBCOMP to confirm QoS2 protocol is done + assertTrue(pubCompLatch.await(5, TimeUnit.SECONDS)); + + // Verify only one message is in the queue despite the QoS2 interruption + Wait.assertEquals(1L, () -> server.locateQueue(TOPIC).getMessageCount(), 500, 25); + + publisher.disconnect(); + + if (restart) { + assertNull(getPubCache(CLIENTID)); + publisher.close(); + } else { + assertEquals(0, getPubCacheSize(CLIENTID)); + + // connect again to clean the session which will completely remove the cache from memory and disk + publisher.connect(new MqttConnectionOptionsBuilder().cleanStart(true).sessionExpiryInterval(0L).build()); + assertNull(getPubCache(CLIENTID)); + publisher.disconnect(); + publisher.close(); + } + } +} diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/QoS2SubscriberResiliencyTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/QoS2SubscriberResiliencyTest.java new file mode 100644 index 000000000000..353449100ac2 --- /dev/null +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/QoS2SubscriberResiliencyTest.java @@ -0,0 +1,678 @@ +/* + * 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.artemis.tests.integration.mqtt5; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import io.netty.handler.codec.mqtt.MqttMessageType; +import io.netty.handler.codec.mqtt.MqttPublishMessage; +import org.apache.activemq.artemis.core.protocol.mqtt.MQTTInterceptor; +import org.apache.activemq.artemis.utils.ByteUtil; +import org.apache.activemq.artemis.utils.RandomUtil; +import org.apache.activemq.artemis.utils.Wait; +import org.eclipse.paho.mqttv5.client.MqttClient; +import org.eclipse.paho.mqttv5.client.MqttConnectionOptions; +import org.eclipse.paho.mqttv5.client.MqttConnectionOptionsBuilder; +import org.eclipse.paho.mqttv5.common.MqttMessage; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests for QoS 2 protocol resiliency with subscriber reconnections or broker restarts. + *

+ * QoS 2 Protocol Flow (broker sending to subscriber): + *

    + *
  1. Broker sends PUBLISH (QoS=2)
    2. + *
  2. Subscriber sends PUBREC
    3. + *
  3. Broker sends PUBREL
    4. + *
  4. Subscriber sends PUBCOMP
    5. + *
+ * These tests verify that the protocol maintains exactly-once delivery semantics when either clients + * reconnect or the broker is restarted at each stage of the QoS 2 flow. + */ +public class QoS2SubscriberResiliencyTest extends MQTT5TestSupport { + + protected static final long DEFAULT_TIMEOUT_SEC = 10; + + @Override + public boolean isProtocolLoggingEnabled() { + return true; + } + + /** + * Verifies that the broker will re-use the same packet ID if it sends a PUBLISH but fails to receive the + * corresponding PUBREC. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2BrokerRestartBeforePubRecSent() throws Exception { + testQoS2FailureBeforePubRecSent(true); + } + + /** + * Same test as {@link testQoS2BrokerRestartBeforePubRecSent} but disconnecting the client instead of restarting the + * broker. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2ClientDisconnectBeforePubRecSent() throws Exception { + testQoS2FailureBeforePubRecSent(false); + } + + public void testQoS2FailureBeforePubRecSent(boolean restart) throws Exception { + final String TOPIC = "test/resiliency"; + final String SUBSCRIBER_CLIENT_ID = "subscriber"; + final String PUBLISHER_CLIENT_ID = "publisher"; + + // Set up interceptor to block the *second* incoming PUBREC. + // Allow 1 PUBREC so the packet ID goes up to 2 for testing. + final CountDownLatch pubRecLatch = new CountDownLatch(1); + AtomicInteger pubRecCount = new AtomicInteger(0); + final CountDownLatch stopLatch = new CountDownLatch(1); + MQTTInterceptor pubRecInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBREC && pubRecCount.incrementAndGet() > 1) { + pubRecLatch.countDown(); + logger.info("Blocking incoming {}", packet.fixedHeader().messageType()); + try { + stopLatch.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + return false; + } + logger.info("Allowing incoming {}", packet.fixedHeader().messageType()); + return true; + }; + server.getRemotingService().addIncomingInterceptor(pubRecInterceptor); + + // Consumer with persistent session + MqttClient subscriber = createPahoClient(SUBSCRIBER_CLIENT_ID); + subscriber.setCallback(new DefaultMqttCallback() { + @Override + public void messageArrived(String topic, MqttMessage message) throws Exception { + logger.info("messageArrived({}, {})", topic, message); + } + }); + MqttConnectionOptions subscriberOptions = new MqttConnectionOptionsBuilder() + .cleanStart(false) + .sessionExpiryInterval(300L) + .build(); + subscriber.connect(subscriberOptions); + subscriber.subscribe(TOPIC, EXACTLY_ONCE); + + // Producer + MqttClient producer = createPahoClient(PUBLISHER_CLIENT_ID); + producer.connect(); + + // Send 2 messages to ensure the packet ID is preserved by the broker. + // If we just send 1 message it won't be clear if the broker just started generating packet IDs from scratch. + producer.publish(TOPIC, RandomUtil.randomBytes(), EXACTLY_ONCE, false); + producer.publish(TOPIC, RandomUtil.randomBytes(), EXACTLY_ONCE, false); + + Wait.assertEquals(0, () -> getPubCacheSize(PUBLISHER_CLIENT_ID)); + + producer.disconnect(); + producer.close(); + + assertNull(getPubCache(PUBLISHER_CLIENT_ID)); + + assertTrue(pubRecLatch.await(5, TimeUnit.SECONDS)); + stopLatch.countDown(); + + if (restart) { + server.stop(); + waitForServerToStop(server); + server.start(); + waitForServerToStart(server); + } else { + server.getRemotingService().clearInterceptors(); + Wait.assertEquals(0, () -> server.getRemotingService().getConnections().size(), 1000, 10); + } + + assertTrue(getProtocolManager().getStateManager().packetIdCorrelationExists(SUBSCRIBER_CLIENT_ID, 2)); + + final CountDownLatch pubCompLatch = new CountDownLatch(1); + MQTTInterceptor pubCompInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBCOMP) { + pubCompLatch.countDown(); + } + return true; + }; + server.getRemotingService().addIncomingInterceptor(pubCompInterceptor); + + CountDownLatch packetIdLatch = new CountDownLatch(1); + MQTTInterceptor pubInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBLISH) { + if (((MqttPublishMessage)packet).variableHeader().packetId() == 2 && ((MqttPublishMessage)packet).fixedHeader().isDup()) { + packetIdLatch.countDown(); + } + } + return true; + }; + server.getRemotingService().addOutgoingInterceptor(pubInterceptor); + + reconnectSafely(subscriber); + + assertTrue(packetIdLatch.await(5, TimeUnit.SECONDS), "Didn't find a duplicate PUBLISH with the expected packet id"); + assertTrue(pubCompLatch.await(5, TimeUnit.SECONDS)); + + Wait.assertEquals(0L, () -> getSubscriptionQueue(TOPIC, SUBSCRIBER_CLIENT_ID).getMessageCount(), 500, 25); + + assertEquals(0, getProtocolManager().getStateManager().getPacketIdCorrelationSize(SUBSCRIBER_CLIENT_ID)); + Wait.assertEquals(0, () -> getPubRecCacheSize(SUBSCRIBER_CLIENT_ID)); + + subscriber.disconnect(); + + // connect again to clean the session which will completely remove the cache from memory and disk + subscriber.connect(new MqttConnectionOptionsBuilder().cleanStart(true).sessionExpiryInterval(0L).build()); + assertNull(getPubRecCache(SUBSCRIBER_CLIENT_ID)); + subscriber.disconnect(); + subscriber.close(); + } + + /** + * Verifies that the broker correctly completes the QoS 2 flow if it receives the PUBREC but the corresponding + * PUBREL is not sent to the consumer before the broker restarts. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2BrokerRestartAfterPubRecSent() throws Exception { + testQoS2FailureAfterPubRecSent(true); + } + + /** + * Same test as {@link testQoS2BrokerRestartAfterPubRecSent} but disconnecting the client instead of restarting the + * broker. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2ClientDisconnectAfterPubRecSent() throws Exception { + testQoS2FailureAfterPubRecSent(false); + } + + public void testQoS2FailureAfterPubRecSent(boolean restart) throws Exception { + final String TOPIC = "test/resiliency"; + final String SUBSCRIBER_CLIENT_ID = "subscriber"; + final String PUBLISHER_CLIENT_ID = "publisher"; + final CountDownLatch pubRelLatch = new CountDownLatch(1); + final CountDownLatch pubCompLatch = new CountDownLatch(1); + AtomicInteger messageCount = new AtomicInteger(0); + + // Block the outgoing PUBREL so the broker has processed PUBREC but the consumer never receives PUBREL + MQTTInterceptor pubRelInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBREL) { + pubRelLatch.countDown(); + logger.info("Blocking outgoing {}", packet.fixedHeader().messageType()); + return false; + } + logger.info("Allowing outgoing {}", packet.fixedHeader().messageType()); + return true; + }; + server.getRemotingService().addOutgoingInterceptor(pubRelInterceptor); + + // Consumer with persistent session + MqttClient subscriber = createPahoClient(SUBSCRIBER_CLIENT_ID); + subscriber.setCallback(new DefaultMqttCallback() { + @Override + public void messageArrived(String topic, MqttMessage message) throws Exception { + messageCount.incrementAndGet(); + logger.info("messageArrived({}, {})", topic, message); + } + }); + MqttConnectionOptions subscriberOptions = new MqttConnectionOptionsBuilder() + .cleanStart(false) + .sessionExpiryInterval(300L) + .build(); + subscriber.connect(subscriberOptions); + subscriber.subscribe(TOPIC, EXACTLY_ONCE); + + // Producer + MqttClient producer = createPahoClient(PUBLISHER_CLIENT_ID); + producer.connect(); + + producer.publish(TOPIC, RandomUtil.randomBytes(), EXACTLY_ONCE, false); + + Wait.assertEquals(0, () -> getPubCacheSize(PUBLISHER_CLIENT_ID)); + + producer.disconnect(); + producer.close(); + + assertNull(getPubCache(PUBLISHER_CLIENT_ID)); + + assertTrue(pubRelLatch.await(5, TimeUnit.SECONDS)); + + if (restart) { + server.stop(); + waitForServerToStop(server); + server.start(); + waitForServerToStart(server); + } else { + server.getRemotingService().clearInterceptors(); + server.getActiveMQServerControl().closeConnectionWithID(server.getActiveMQServerControl().listConnectionIDs()[0]); + } + + assertEquals(0, getProtocolManager().getStateManager().getPacketIdCorrelationSize(SUBSCRIBER_CLIENT_ID)); + assertTrue(getPubRecCache(SUBSCRIBER_CLIENT_ID).contains(ByteUtil.intToBytes(1))); + + MQTTInterceptor pubCompInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBCOMP) { + pubCompLatch.countDown(); + } + return true; + }; + server.getRemotingService().addIncomingInterceptor(pubCompInterceptor); + + reconnectSafely(subscriber); + + assertTrue(pubCompLatch.await(5, TimeUnit.SECONDS)); + + Wait.assertEquals(0L, () -> getSubscriptionQueue(TOPIC, SUBSCRIBER_CLIENT_ID).getMessageCount(), 500, 25); + + assertEquals(0, getProtocolManager().getStateManager().getPacketIdCorrelationSize(SUBSCRIBER_CLIENT_ID)); + Wait.assertEquals(0, () -> getPubRecCacheSize(SUBSCRIBER_CLIENT_ID)); + assertEquals(1, messageCount.get()); + + subscriber.disconnect(); + + // connect again to clean the session which will completely remove the cache from memory and disk + subscriber.connect(new MqttConnectionOptionsBuilder().cleanStart(true).sessionExpiryInterval(0L).build()); + assertNull(getPubRecCache(SUBSCRIBER_CLIENT_ID)); + subscriber.disconnect(); + subscriber.close(); + } + + /** + * Verifies that the broker correctly completes the QoS 2 flow if it sends the PUBREL but fails to receive the + * corresponding PUBCOMP before the broker restarts. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2BrokerRestartAfterPubRelSent() throws Exception { + testQoS2FailureAfterPubRelSent(true); + } + + /** + * Same test as {@link testQoS2BrokerRestartAfterPubRelSent} but disconnecting the client instead of restarting the + * broker. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2ClientDisconnectAfterPubRelSent() throws Exception { + testQoS2FailureAfterPubRelSent(false); + } + + public void testQoS2FailureAfterPubRelSent(boolean restart) throws Exception { + final String TOPIC = "test/resiliency"; + final String SUBSCRIBER_CLIENT_ID = "subscriber"; + final String PUBLISHER_CLIENT_ID = "publisher"; + final CountDownLatch pubCompBlockedLatch = new CountDownLatch(1); + final CountDownLatch stopLatch = new CountDownLatch(1); + final CountDownLatch pubCompLatch = new CountDownLatch(1); + AtomicInteger messageCount = new AtomicInteger(0); + + // Block the incoming PUBCOMP so the broker has sent PUBREL but never processes the PUBCOMP + MQTTInterceptor pubCompBlocker = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBCOMP) { + pubCompBlockedLatch.countDown(); + try { + stopLatch.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + return false; + } + return true; + }; + server.getRemotingService().addIncomingInterceptor(pubCompBlocker); + + // Consumer with persistent session + MqttClient subscriber = createPahoClient(SUBSCRIBER_CLIENT_ID); + subscriber.setCallback(new DefaultMqttCallback() { + @Override + public void messageArrived(String topic, MqttMessage message) throws Exception { + messageCount.incrementAndGet(); + logger.info("messageArrived({}, {})", topic, message); + } + }); + MqttConnectionOptions subscriberOptions = new MqttConnectionOptionsBuilder() + .cleanStart(false) + .sessionExpiryInterval(300L) + .build(); + subscriber.connect(subscriberOptions); + subscriber.subscribe(TOPIC, EXACTLY_ONCE); + + // Producer + MqttClient producer = createPahoClient(PUBLISHER_CLIENT_ID); + producer.connect(); + + producer.publish(TOPIC, RandomUtil.randomBytes(), EXACTLY_ONCE, false); + + producer.disconnect(); + producer.close(); + + assertTrue(pubCompBlockedLatch.await(5, TimeUnit.SECONDS)); + stopLatch.countDown(); + + if (restart) { + server.stop(); + waitForServerToStop(server); + server.start(); + waitForServerToStart(server); + } else { + server.getRemotingService().clearInterceptors(); + Wait.assertEquals(0, () -> server.getRemotingService().getConnections().size(), 1000, 10); + } + + assertEquals(0, getProtocolManager().getStateManager().getPacketIdCorrelationSize(SUBSCRIBER_CLIENT_ID)); + assertTrue(getPubRecCache(SUBSCRIBER_CLIENT_ID).contains(ByteUtil.intToBytes(1))); + + MQTTInterceptor pubCompInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBCOMP) { + pubCompLatch.countDown(); + } + return true; + }; + server.getRemotingService().addIncomingInterceptor(pubCompInterceptor); + + reconnectSafely(subscriber); + + assertTrue(pubCompLatch.await(5, TimeUnit.SECONDS)); + + Wait.assertEquals(0L, () -> getSubscriptionQueue(TOPIC, SUBSCRIBER_CLIENT_ID).getMessageCount(), 500, 25); + + assertEquals(0, getProtocolManager().getStateManager().getPacketIdCorrelationSize(SUBSCRIBER_CLIENT_ID)); + Wait.assertEquals(0, () -> getPubRecCacheSize(SUBSCRIBER_CLIENT_ID)); + assertEquals(1, messageCount.get()); + + subscriber.disconnect(); + + // connect again to clean the session which will completely remove the cache from memory and disk + subscriber.connect(new MqttConnectionOptionsBuilder().cleanStart(true).sessionExpiryInterval(0L).build()); + assertNull(getPubRecCache(SUBSCRIBER_CLIENT_ID)); + subscriber.disconnect(); + subscriber.close(); + } + + /** + * Verifies that after a complete QoS 2 protocol exchange (PUBLISH, PUBREC, PUBREL, PUBCOMP), a broker restart does + * not cause re-delivery. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2BrokerRestartAfterPubCompSent() throws Exception { + testQoS2FailureAfterPubCompSent(true); + } + + /** + * Same test as {@link testQoS2BrokerRestartAfterPubCompSent} but disconnecting the client instead of restarting the + * broker. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2ClientDisconnectAfterPubCompSent() throws Exception { + testQoS2FailureAfterPubCompSent(false); + } + + public void testQoS2FailureAfterPubCompSent(boolean restart) throws Exception { + final String TOPIC = "test/resiliency"; + final String SUBSCRIBER_CLIENT_ID = "subscriber"; + final String PUBLISHER_CLIENT_ID = "publisher"; + final CountDownLatch pubCompLatch = new CountDownLatch(1); + AtomicInteger messageCount = new AtomicInteger(0); + + // Consumer with persistent session + MqttClient subscriber = createPahoClient(SUBSCRIBER_CLIENT_ID); + subscriber.setCallback(new DefaultMqttCallback() { + @Override + public void messageArrived(String topic, MqttMessage message) throws Exception { + messageCount.incrementAndGet(); + logger.info("messageArrived({}, {})", topic, message); + } + }); + MqttConnectionOptions subscriberOptions = new MqttConnectionOptionsBuilder() + .cleanStart(false) + .sessionExpiryInterval(300L) + .build(); + subscriber.connect(subscriberOptions); + subscriber.subscribe(TOPIC, EXACTLY_ONCE); + + // Track PUBCOMPs to know when both QoS 2 flows are complete + MQTTInterceptor pubCompInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBCOMP) { + pubCompLatch.countDown(); + } + return true; + }; + server.getRemotingService().addIncomingInterceptor(pubCompInterceptor); + + // Producer + MqttClient producer = createPahoClient(PUBLISHER_CLIENT_ID); + producer.connect(); + + producer.publish(TOPIC, RandomUtil.randomBytes(), EXACTLY_ONCE, false); + + producer.disconnect(); + producer.close(); + + // Wait for both QoS 2 flows to complete + assertTrue(pubCompLatch.await(5, TimeUnit.SECONDS)); + Wait.assertEquals(0L, () -> getSubscriptionQueue(TOPIC, SUBSCRIBER_CLIENT_ID).getMessageCount(), 500, 25); + + assertEquals(0, getProtocolManager().getStateManager().getPacketIdCorrelationSize(SUBSCRIBER_CLIENT_ID)); + assertEquals(0, getPubRecCacheSize(SUBSCRIBER_CLIENT_ID)); + + if (restart) { + server.stop(); + waitForServerToStop(server); + server.start(); + waitForServerToStart(server); + } else { + server.getRemotingService().clearInterceptors(); + server.getActiveMQServerControl().closeConnectionWithID( + server.getActiveMQServerControl().listConnectionIDs()[0]); + } + + int countBeforeReconnect = messageCount.get(); + reconnectSafely(subscriber); + + // Verify no unexpected re-delivery + assertFalse(Wait.waitFor(() -> messageCount.get() > countBeforeReconnect, 500, 25), "Unexpected message delivered after restart"); + Wait.assertEquals(0L, () -> getSubscriptionQueue(TOPIC, SUBSCRIBER_CLIENT_ID).getMessageCount(), 500, 25); + + assertEquals(0, getProtocolManager().getStateManager().getPacketIdCorrelationSize(SUBSCRIBER_CLIENT_ID)); + if (restart) { + assertNull(getPubRecCache(SUBSCRIBER_CLIENT_ID)); + } else { + assertEquals(0, getPubRecCacheSize(SUBSCRIBER_CLIENT_ID)); + } + + subscriber.disconnect(); + + // connect again to clean the session which will completely remove the cache from memory and disk + subscriber.connect(new MqttConnectionOptionsBuilder().cleanStart(true).sessionExpiryInterval(0L).build()); + assertNull(getPubRecCache(SUBSCRIBER_CLIENT_ID)); + subscriber.disconnect(); + subscriber.close(); + } + + /** + * Verifies that incomplete QoS 2 flow state (packet ID correlations) is removed when a client with session expiry + * interval 0 is disconnected before PUBREC is processed. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2PacketIdCorrelationsRemovedOnSessionExpiryZeroDisconnect() throws Exception { + final String TOPIC = "test/resiliency"; + final String SUBSCRIBER_CLIENT_ID = "subscriber"; + final String PUBLISHER_CLIENT_ID = "publisher"; + + // Block the incoming PUBREC so the packet ID correlation is never removed by the normal QoS 2 flow. + // Returning false from an incoming interceptor disconnects the client. + final CountDownLatch pubRecLatch = new CountDownLatch(1); + MQTTInterceptor pubRecInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBREC) { + pubRecLatch.countDown(); + return false; + } + return true; + }; + server.getRemotingService().addIncomingInterceptor(pubRecInterceptor); + + MqttClient subscriber = createPahoClient(SUBSCRIBER_CLIENT_ID); + subscriber.setCallback(new DefaultMqttCallback() { + @Override + public void messageArrived(String topic, MqttMessage message) throws Exception { + } + }); + subscriber.connect(new MqttConnectionOptionsBuilder().cleanStart(true).sessionExpiryInterval(0L).build()); + subscriber.subscribe(TOPIC, EXACTLY_ONCE); + + MqttClient producer = createPahoClient(PUBLISHER_CLIENT_ID); + producer.connect(); + producer.publish(TOPIC, RandomUtil.randomBytes(), EXACTLY_ONCE, false); + Wait.assertEquals(0, () -> getPubCacheSize(PUBLISHER_CLIENT_ID)); + producer.disconnect(); + producer.close(); + + // Wait for PUBREC to be intercepted (which disconnects the subscriber) + assertTrue(pubRecLatch.await(5, TimeUnit.SECONDS)); + server.getRemotingService().clearInterceptors(); + Wait.assertEquals(0, () -> server.getRemotingService().getConnections().size(), 1000, 10); + + // Since session expiry is 0, the session was cleaned up on disconnect. + // The packet ID correlations should have been removed. + assertEquals(0, getProtocolManager().getStateManager().getPacketIdCorrelationSize(SUBSCRIBER_CLIENT_ID)); + + subscriber.close(); + } + + /** + * Verifies that incomplete QoS 2 flow state (packet ID correlations) is removed when a session expires via the + * session expiry scanner. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2PacketIdCorrelationsRemovedOnSessionExpiry() throws Exception { + final String TOPIC = "test/resiliency"; + final String SUBSCRIBER_CLIENT_ID = "subscriber"; + final String PUBLISHER_CLIENT_ID = "publisher"; + + final CountDownLatch pubRecLatch = new CountDownLatch(1); + MQTTInterceptor pubRecInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBREC) { + pubRecLatch.countDown(); + return false; + } + return true; + }; + server.getRemotingService().addIncomingInterceptor(pubRecInterceptor); + + MqttClient subscriber = createPahoClient(SUBSCRIBER_CLIENT_ID); + subscriber.setCallback(new DefaultMqttCallback() { + @Override + public void messageArrived(String topic, MqttMessage message) throws Exception { + } + }); + subscriber.connect(new MqttConnectionOptionsBuilder().cleanStart(true).sessionExpiryInterval(1L).build()); + subscriber.subscribe(TOPIC, EXACTLY_ONCE); + + MqttClient producer = createPahoClient(PUBLISHER_CLIENT_ID); + producer.connect(); + producer.publish(TOPIC, RandomUtil.randomBytes(), EXACTLY_ONCE, false); + Wait.assertEquals(0, () -> getPubCacheSize(PUBLISHER_CLIENT_ID)); + producer.disconnect(); + producer.close(); + + assertTrue(pubRecLatch.await(5, TimeUnit.SECONDS)); + server.getRemotingService().clearInterceptors(); + Wait.assertEquals(0, () -> server.getRemotingService().getConnections().size(), 1000, 10); + + // Packet ID correlations should exist since PUBREC was never processed + assertTrue(getProtocolManager().getStateManager().getPacketIdCorrelationSize(SUBSCRIBER_CLIENT_ID) > 0); + + // Wait for the session to expire then trigger the scanner + Thread.sleep(1500); + scanSessions(); + + // The session expiry scanner should have cleaned up the packet ID correlations + assertEquals(0, getProtocolManager().getStateManager().getPacketIdCorrelationSize(SUBSCRIBER_CLIENT_ID)); + assertNull(getSessionStates().get(SUBSCRIBER_CLIENT_ID)); + + subscriber.close(); + } + + /** + * Verifies that incomplete QoS 2 flow state (packet ID correlations) is removed when a client reconnects with + * clean start = true. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2PacketIdCorrelationsRemovedOnCleanStartReconnect() throws Exception { + final String TOPIC = "test/resiliency"; + final String SUBSCRIBER_CLIENT_ID = "subscriber"; + final String PUBLISHER_CLIENT_ID = "publisher"; + + final CountDownLatch pubRecLatch = new CountDownLatch(1); + MQTTInterceptor pubRecInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBREC) { + pubRecLatch.countDown(); + return false; + } + return true; + }; + server.getRemotingService().addIncomingInterceptor(pubRecInterceptor); + + MqttClient subscriber = createPahoClient(SUBSCRIBER_CLIENT_ID); + subscriber.setCallback(new DefaultMqttCallback() { + @Override + public void messageArrived(String topic, MqttMessage message) throws Exception { + } + }); + subscriber.connect(new MqttConnectionOptionsBuilder().cleanStart(true).sessionExpiryInterval(300L).build()); + subscriber.subscribe(TOPIC, EXACTLY_ONCE); + + MqttClient producer = createPahoClient(PUBLISHER_CLIENT_ID); + producer.connect(); + producer.publish(TOPIC, RandomUtil.randomBytes(), EXACTLY_ONCE, false); + Wait.assertEquals(0, () -> getPubCacheSize(PUBLISHER_CLIENT_ID)); + producer.disconnect(); + producer.close(); + + assertTrue(pubRecLatch.await(5, TimeUnit.SECONDS)); + server.getRemotingService().clearInterceptors(); + Wait.assertEquals(0, () -> server.getRemotingService().getConnections().size(), 1000, 10); + + // Packet ID correlations should exist since PUBREC was never processed + assertTrue(getProtocolManager().getStateManager().getPacketIdCorrelationSize(SUBSCRIBER_CLIENT_ID) > 0); + + // Reconnect with clean start to discard previous session state + subscriber.connect(new MqttConnectionOptionsBuilder().cleanStart(true).sessionExpiryInterval(0L).build()); + + // The clean start should have cleared the packet ID correlations + assertEquals(0, getProtocolManager().getStateManager().getPacketIdCorrelationSize(SUBSCRIBER_CLIENT_ID)); + + subscriber.disconnect(); + subscriber.close(); + } +} diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/QoSTests.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/QoSTests.java index 92ff22721dbc..80fe0da0ae11 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/QoSTests.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/QoSTests.java @@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -27,6 +28,19 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import com.hivemq.client.mqtt.MqttGlobalPublishFilter; +import com.hivemq.client.mqtt.datatypes.MqttQos; +import com.hivemq.client.mqtt.mqtt5.Mqtt5BlockingClient; +import com.hivemq.client.mqtt.mqtt5.Mqtt5Client; +import com.hivemq.client.mqtt.mqtt5.Mqtt5ClientConfig; +import com.hivemq.client.mqtt.mqtt5.advanced.interceptor.qos2.Mqtt5IncomingQos2Interceptor; +import com.hivemq.client.mqtt.mqtt5.message.publish.Mqtt5Publish; +import com.hivemq.client.mqtt.mqtt5.message.publish.pubcomp.Mqtt5PubCompBuilder; +import com.hivemq.client.mqtt.mqtt5.message.publish.pubrec.Mqtt5PubRecBuilder; +import com.hivemq.client.mqtt.mqtt5.message.publish.pubrec.Mqtt5PubRecReasonCode; +import com.hivemq.client.mqtt.mqtt5.message.publish.pubrel.Mqtt5PubRel; +import io.reactivex.schedulers.Schedulers; +import io.netty.channel.epoll.EpollEventLoopGroup; import io.netty.handler.codec.mqtt.MqttMessageType; import io.netty.handler.codec.mqtt.MqttPubAckMessage; import io.netty.handler.codec.mqtt.MqttPubReplyMessageVariableHeader; @@ -38,6 +52,7 @@ import org.apache.activemq.artemis.core.protocol.mqtt.MQTTUtil; import org.apache.activemq.artemis.core.server.Queue; import org.apache.activemq.artemis.tests.integration.mqtt5.MQTT5TestSupport; +import org.apache.activemq.artemis.utils.ByteUtil; import org.apache.activemq.artemis.utils.RandomUtil; import org.apache.activemq.artemis.tests.util.Wait; import org.eclipse.paho.mqttv5.client.MqttClient; @@ -368,13 +383,8 @@ public void testQoS2PubRel() throws Exception { MQTTInterceptor incomingInterceptor = (packet, connection) -> { if (packet.fixedHeader().messageType() == MqttMessageType.PUBCOMP) { - try { - // ensure the message is still in the management queue before we get the PUBCOMP from the client - Wait.assertEquals(1L, () -> server.locateQueue(MQTTUtil.QOS2_MANAGEMENT_QUEUE_PREFIX + CONSUMER_ID).getMessageCount(), 2000, 100); - Wait.assertEquals(1L, () -> server.locateQueue(MQTTUtil.QOS2_MANAGEMENT_QUEUE_PREFIX + CONSUMER_ID).getDeliveringCount(), 2000, 100); - } catch (Exception e) { - return false; - } + // ensure the packet ID is stored in the PUBREC cache before we get the PUBCOMP from the client + assertTrue(getPubRecCache(CONSUMER_ID).contains(ByteUtil.intToBytes(packetId.get()))); // ensure the ids match so we know this is the "corresponding" PUBCOMP for the previous PUBLISH assertEquals(packetId.get(), ((MqttPubReplyMessageVariableHeader)packet.variableHeader()).messageId()); @@ -691,4 +701,96 @@ public void testQoS2WithExpiration2() throws Exception { assertTrue(ackLatch.await(messageExpiryInterval * 2, TimeUnit.SECONDS)); Wait.assertEquals(1, () -> getSubscriptionQueue(TOPIC, CONSUMER_ID).getMessagesExpired()); } + + /* + * [MQTT-4.3.3-4] In the QoS 2 delivery protocol, the sender MUST send a PUBREL packet when it receives a PUBREC + * packet from the receiver with a Reason Code value less than 0x80. + * + * The converse: when the broker (sender) receives a PUBREC with Reason Code >= 0x80, it MUST NOT send PUBREL. + * Instead, it should treat the PUBLISH as acknowledged (i.e. discard it without sending PUBREL). + * + * This test uses the HiveMQ MQTT client as the Paho client doesn't support the ability to simulate a PUBREC with + * a reason code greater than 0x80. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testQoS2PubRecError() throws Exception { + final String TOPIC = RandomUtil.randomUUIDString(); + final String CONSUMER_ID = "consumer"; + final AtomicBoolean pubRelSent = new AtomicBoolean(false); + final CountDownLatch pubRecReceived = new CountDownLatch(1); + final AtomicBoolean correlationExistedDuringDelivery = new AtomicBoolean(false); + + MQTTInterceptor incomingInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBREC) { + pubRecReceived.countDown(); + } + return true; + }; + + MQTTInterceptor outgoingInterceptor = (packet, connection) -> { + if (packet.fixedHeader().messageType() == MqttMessageType.PUBLISH) { + correlationExistedDuringDelivery.set(getProtocolManager().getStateManager().getPacketIdCorrelationSize(CONSUMER_ID) > 0); + } + if (packet.fixedHeader().messageType() == MqttMessageType.PUBREL) { + pubRelSent.set(true); + } + return true; + }; + + server.getRemotingService().addIncomingInterceptor(incomingInterceptor); + server.getRemotingService().addOutgoingInterceptor(outgoingInterceptor); + + EpollEventLoopGroup hivemqEventLoop = new EpollEventLoopGroup(); + try { + Mqtt5BlockingClient consumer = Mqtt5Client.builder() + .identifier(CONSUMER_ID) + .serverHost("localhost") + .serverPort(getPort()) + .executorConfig() + .nettyExecutor(hivemqEventLoop) + .applyExecutorConfig() + .advancedConfig() + .interceptors() + .incomingQos2Interceptor(new Mqtt5IncomingQos2Interceptor() { + @Override + public void onPublish(Mqtt5ClientConfig clientConfig, Mqtt5Publish publish, Mqtt5PubRecBuilder pubRecBuilder) { + pubRecBuilder.reasonCode(Mqtt5PubRecReasonCode.UNSPECIFIED_ERROR); + } + + @Override + public void onPubRel(Mqtt5ClientConfig clientConfig, Mqtt5PubRel pubRel, Mqtt5PubCompBuilder pubCompBuilder) { + } + }) + .applyInterceptors() + .applyAdvancedConfig() + .buildBlocking(); + consumer.connect(); + Mqtt5BlockingClient.Mqtt5Publishes publishes = consumer.publishes(MqttGlobalPublishFilter.ALL); + consumer.subscribeWith() + .topicFilter(TOPIC) + .qos(MqttQos.EXACTLY_ONCE) + .send(); + + MqttClient producer = createPahoClient("producer"); + producer.connect(); + producer.publish(TOPIC, RandomUtil.randomUUIDString().getBytes(), 2, false); + producer.disconnect(); + producer.close(); + + assertTrue(pubRecReceived.await(5, TimeUnit.SECONDS), "PUBREC was not received by the broker"); + assertTrue(correlationExistedDuringDelivery.get(), "Packet ID correlation should exist during delivery"); + Wait.assertEquals(0L, () -> getSubscriptionQueue(TOPIC, CONSUMER_ID).getMessageCount(), 2000, 100); + Wait.assertEquals(1L, () -> getSubscriptionQueue(TOPIC, CONSUMER_ID).getMessagesAcknowledged(), 2000, 100); + Wait.assertEquals(0, () -> getProtocolManager().getStateManager().getPacketIdCorrelationSize(CONSUMER_ID), 2000, 100); + assertFalse(pubRelSent.get(), "PUBREL should not be sent in response to an error PUBREC"); + assertNull(getPubRecCache(CONSUMER_ID)); + + publishes.close(); + consumer.disconnect(); + } finally { + hivemqEventLoop.shutdownGracefully(0, 0, TimeUnit.MILLISECONDS).sync(); + Schedulers.shutdown(); + } + } } diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/ConnAckTests.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/ConnAckTests.java index 0960275cd9d8..b270b2e28c9e 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/ConnAckTests.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/ConnAckTests.java @@ -151,6 +151,17 @@ public void testConnackWhenCleanStartFalse() throws Exception { result = consumer.connectWithResult(options); assertTrue(result.getSessionPresent()); assertTrue(getListOfCodes(result.getResponse().getReasonCodes()).contains(MqttReturnCode.RETURN_CODE_SUCCESS)); + consumer.disconnect(); + + server.stop(); + waitForServerToStop(server); + server.start(); + waitForServerToStart(server); + + result = consumer.connectWithResult(options); + assertTrue(result.getSessionPresent()); + assertTrue(getListOfCodes(result.getResponse().getReasonCodes()).contains(MqttReturnCode.RETURN_CODE_SUCCESS)); + consumer.disconnect(); } /* diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/ConnectTests.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/ConnectTests.java index d236a56d88d7..e28614a31abe 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/ConnectTests.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/ConnectTests.java @@ -485,7 +485,35 @@ public void testKeepAlive() throws Exception { */ @Test @Timeout(DEFAULT_TIMEOUT_SEC) - public void testMaxPacketSize() throws Exception { + public void testMaxPacketSizeQoS2() throws Exception { + testMaxPacketSize(2); + } + + /* + * [MQTT-3.1.2-24] The Server MUST NOT send packets exceeding Maximum Packet Size to the Client. + * + * [MQTT-3.1.2-25] Where a Packet is too large to send, the Server MUST discard it without sending it and then behave + * as if it had completed sending that Application Message. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testMaxPacketSizeQoS1() throws Exception { + testMaxPacketSize(1); + } + + /* + * [MQTT-3.1.2-24] The Server MUST NOT send packets exceeding Maximum Packet Size to the Client. + * + * [MQTT-3.1.2-25] Where a Packet is too large to send, the Server MUST discard it without sending it and then behave + * as if it had completed sending that Application Message. + */ + @Test + @Timeout(DEFAULT_TIMEOUT_SEC) + public void testMaxPacketSizeQoS0() throws Exception { + testMaxPacketSize(0); + } + + private void testMaxPacketSize(int qos) throws Exception { final String CONSUMER_ID = RandomUtil.randomUUIDString(); final String TOPIC = this.getTopicName(); final long SIZE = 1500; @@ -502,11 +530,11 @@ public void testMaxPacketSize() throws Exception { .build(); consumer.setCallback(new LatchedMqttCallback(latch)); consumer.connect(options); - consumer.subscribe(TOPIC, 2); + consumer.subscribe(TOPIC, qos); MqttClient producer = createPahoClient(RandomUtil.randomUUIDString()); producer.connect(); - producer.publish(TOPIC, bytes, 2, false); + producer.publish(TOPIC, bytes, qos, false); producer.disconnect(); producer.close(); Wait.assertEquals(1L, () -> getSubscriptionQueue(TOPIC, CONSUMER_ID).getMessagesAdded(), 2000, 100); @@ -516,6 +544,9 @@ public void testMaxPacketSize() throws Exception { // the broker should acknowledge the message since it exceeded the client's max packet size Wait.assertEquals(1L, () -> getSubscriptionQueue(TOPIC, CONSUMER_ID).getMessagesAcknowledged(), 2000, 100); + Wait.assertEquals(0L, () -> getSubscriptionQueue(TOPIC, CONSUMER_ID).getMessageCount(), 2000, 100); + Wait.assertEquals(0, () -> getProtocolManager().getStateManager().getPacketIdCorrelationSize(CONSUMER_ID), 2000, 100); + Wait.assertNull(() -> getPubRecCache(CONSUMER_ID), 2000, 100); consumer.disconnect(); consumer.close(); } diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/replication/RepeatStartBackupTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/replication/RepeatStartBackupTest.java index bd4d0d0ed1a6..5c77312d83fd 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/replication/RepeatStartBackupTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/replication/RepeatStartBackupTest.java @@ -277,11 +277,11 @@ private void validateAckManager(ActiveMQServer server, long queueIdOnServerLive, int messagesSent) { AckManager liveManager = AckManagerProvider.getManager(server); - Map>> sortedRetries = liveManager.sortRetries(); + Map>> sortedRetries = liveManager.sortRetries(); assertEquals(1, sortedRetries.size()); - LongObjectHashMap> retryAddress = sortedRetries.get(SimpleString.of(queueName)); - JournalHashMap journalHashMapBackup = retryAddress.get(queueIdOnServerLive); + LongObjectHashMap> retryAddress = sortedRetries.get(SimpleString.of(queueName)); + JournalHashMap journalHashMapBackup = retryAddress.get(queueIdOnServerLive); assertEquals(messagesSent, journalHashMapBackup.size()); } diff --git a/tests/soak-tests/pom.xml b/tests/soak-tests/pom.xml index 5f5ea7aa2f9b..078ddbfeb7a7 100644 --- a/tests/soak-tests/pom.xml +++ b/tests/soak-tests/pom.xml @@ -188,7 +188,11 @@ spring-jms test - + + com.hivemq + hivemq-mqtt-client + test + diff --git a/tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/mqtt/resiliency/QoS2CombinedResiliencySoakTest.java b/tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/mqtt/resiliency/QoS2CombinedResiliencySoakTest.java new file mode 100644 index 000000000000..99a7c753ce27 --- /dev/null +++ b/tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/mqtt/resiliency/QoS2CombinedResiliencySoakTest.java @@ -0,0 +1,188 @@ +/* + * 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.artemis.tests.soak.mqtt.resiliency; + +import java.lang.invoke.MethodHandles; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; + +import com.hivemq.client.mqtt.MqttGlobalPublishFilter; +import com.hivemq.client.mqtt.datatypes.MqttQos; +import com.hivemq.client.mqtt.mqtt5.Mqtt5BlockingClient; +import org.apache.activemq.artemis.utils.TestParameters; +import org.apache.activemq.artemis.utils.Wait; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class QoS2CombinedResiliencySoakTest extends QoS2ResiliencySoakTestSupport { + + private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + private static final String TEST_NAME = "QOS2_COMBINED_RESILIENCY_SOAK"; + + private static final int NUM_PUBLISHERS = TestParameters.testProperty(TEST_NAME, "NUM_PUBLISHERS", 5); + private static final int NUM_SUBSCRIBERS = TestParameters.testProperty(TEST_NAME, "NUM_SUBSCRIBERS", 5); + private static final int NUM_MESSAGES = TestParameters.testProperty(TEST_NAME, "NUM_MESSAGES", 2_000); + private static final int RESTART_PAUSE = TestParameters.testProperty(TEST_NAME, "RESTART_PAUSE", 2_000); + private static final int TIMEOUT_SECONDS = TestParameters.testProperty(TEST_NAME, "TIMEOUT_SECONDS", 180); + + @Test + @Timeout(value = 5, unit = TimeUnit.MINUTES) + public void testQoS2CombinedResiliency() throws Exception { + disableProtocolLogging(); + final String PUB_CLIENT_ID_PREFIX = "pub-"; + final String SUB_CLIENT_ID_PREFIX = "sub-"; + + logger.info("{} publishers, {} subscribers, {} messages/publisher, {} total expected per subscriber", NUM_PUBLISHERS, NUM_SUBSCRIBERS, NUM_MESSAGES, NUM_PUBLISHERS * NUM_MESSAGES); + + // create and subscribe all subscribers + final List subscribers = new ArrayList<>(NUM_SUBSCRIBERS); + runAfter(() -> subscribers.forEach(c -> { + try { + c.disconnect(); + } catch (Exception ignored) { + } + })); + + final Map> receivedPerSubscriber = new HashMap<>(); + final Map> duplicatesPerSubscriber = new HashMap<>(); + final AtomicLong lastReceiveTime = new AtomicLong(System.currentTimeMillis()); + + for (int i = 0; i < NUM_SUBSCRIBERS; i++) { + String clientId = SUB_CLIENT_ID_PREFIX + i; + Mqtt5BlockingClient subscriber = createHiveMQClient(clientId, true); + final Set received = ConcurrentHashMap.newKeySet(NUM_PUBLISHERS * NUM_MESSAGES); + receivedPerSubscriber.put(clientId, received); + final Set duplicates = ConcurrentHashMap.newKeySet(); + duplicatesPerSubscriber.put(clientId, duplicates); + subscriber.toAsync().publishes(MqttGlobalPublishFilter.ALL, publish -> { + String payload = new String(publish.getPayloadAsBytes(), StandardCharsets.UTF_8); + if (!received.add(payload)) { + logger.warn("Subscriber {} received duplicate: {}", clientId, payload); + duplicates.add(payload); + } + lastReceiveTime.set(System.currentTimeMillis()); + }); + subscriber.connectWith() + .cleanStart(false) + .sessionExpiryInterval(300) + .send(); + subscriber.subscribeWith() + .topicFilter(TOPIC) + .qos(MqttQos.EXACTLY_ONCE) + .send(); + subscribers.add(subscriber); + assertNotNull(getSubscriptionQueue(TOPIC, clientId)); + logger.info("Subscriber {} connected and subscribed", clientId); + } + + // create and connect publishers + final List publishers = new ArrayList<>(); + runAfter(() -> publishers.forEach(c -> { + try { + c.disconnect(); + } catch (Exception ignored) { + } + })); + for (int i = 0; i < NUM_PUBLISHERS; i++) { + String clientId = PUB_CLIENT_ID_PREFIX + i; + Mqtt5BlockingClient publisher = createHiveMQClient(clientId, true); + publisher.connectWith() + .cleanStart(false) + .sessionExpiryInterval(300) + .send(); + publishers.add(publisher); + logger.info("Publisher {} connected", clientId); + } + + // start publisher tasks + PublishResult publishResult = startPublishing(publishers, NUM_MESSAGES); + + // start broker restart task + BrokerRestartTask restartTask = startBrokerRestartTask(RESTART_PAUSE, () -> { + logger.info("==========="); + for (Map.Entry> entry : receivedPerSubscriber.entrySet()) { + logger.info("Subscriber {} received {}/{} messages", entry.getKey(), entry.getValue().size(), NUM_PUBLISHERS * NUM_MESSAGES); + } + logger.info("Last message received {}ms ago.", System.currentTimeMillis() - lastReceiveTime.get()); + logger.info("==========="); + }); + + // wait for all publishers to finish sending + publishResult.executor().shutdown(); + assertTrue(publishResult.executor().awaitTermination(TIMEOUT_SECONDS, TimeUnit.SECONDS), "Publishers did not finish in time"); + logger.info("All publishers finished. Total messages sent: {}. Publish errors: {}", publishResult.sentMessages().size(), publishResult.publishErrors().get()); + + // wait for all subscribers to receive all messages + final long STALL_TIMEOUT_MS = 20_000; + Wait.assertTrue(() -> { + if (System.currentTimeMillis() - lastReceiveTime.get() > STALL_TIMEOUT_MS) { + for (Map.Entry> entry : receivedPerSubscriber.entrySet()) { + logger.warn("Subscriber {} received {}/{} messages", entry.getKey(), entry.getValue().size(), NUM_PUBLISHERS * NUM_MESSAGES); + } + throw new AssertionError("No subscriber has received a message in " + STALL_TIMEOUT_MS / 1000 + " seconds"); + } + for (Map.Entry> duplicates : duplicatesPerSubscriber.entrySet()) { + assertEquals(0, duplicates.getValue().size(), "Subscriber " + duplicates.getKey() + " received duplicates: " + duplicates.getValue()); + } + for (Set received : receivedPerSubscriber.values()) { + if (received.size() < NUM_PUBLISHERS * NUM_MESSAGES) { + return false; + } + } + return true; + }, TIMEOUT_SECONDS * 1000L, 100); + + disableProtocolLogging(); + + // stop reconnection task, ensure broker is running + stopBrokerRestartTask(restartTask); + + // verify all expected messages received with no duplicates + for (Mqtt5BlockingClient subscriber : subscribers) { + String clientId = getClientId(subscriber); + assertEquals(0, duplicatesPerSubscriber.get(clientId).size(), "Subscriber " + clientId + " received duplicates: " + duplicatesPerSubscriber.get(clientId)); + assertEquals(NUM_PUBLISHERS * NUM_MESSAGES, receivedPerSubscriber.get(clientId).size(), "Subscriber " + clientId + " didn't receive: " + getMissingMessages(publishResult.sentMessages(), receivedPerSubscriber.get(clientId))); + assertEquals(0L, getSubscriptionQueue(TOPIC, clientId).getMessageCount(), "Subscription queue for " + clientId + " has incorrect message count"); + assertEquals(0, getProtocolManager().getStateManager().getPacketIdCorrelationSize(clientId)); + assertEquals(0, getSubCacheSize(clientId)); + cleanDisconnect(subscriber); + assertNull(getSubCache(clientId), "Sub cache should be null after clean start for " + clientId); + } + + for (Mqtt5BlockingClient publisher : publishers) { + String clientId = getClientId(publisher); + Wait.assertEquals(0, () -> getPubCacheSize(clientId), 5000, 100); + cleanDisconnect(publisher); + assertNull(getPubCache(clientId), "Pub cache should be null after clean start for " + clientId); + } + } +} \ No newline at end of file diff --git a/tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/mqtt/resiliency/QoS2PublisherResiliencySoakTest.java b/tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/mqtt/resiliency/QoS2PublisherResiliencySoakTest.java new file mode 100644 index 000000000000..ac6dd3e6422e --- /dev/null +++ b/tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/mqtt/resiliency/QoS2PublisherResiliencySoakTest.java @@ -0,0 +1,153 @@ +/* + * 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.artemis.tests.soak.mqtt.resiliency; + +import java.lang.invoke.MethodHandles; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import com.hivemq.client.mqtt.MqttGlobalPublishFilter; +import com.hivemq.client.mqtt.datatypes.MqttQos; +import com.hivemq.client.mqtt.mqtt5.Mqtt5BlockingClient; +import org.apache.activemq.artemis.utils.TestParameters; +import org.apache.activemq.artemis.utils.Wait; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class QoS2PublisherResiliencySoakTest extends QoS2ResiliencySoakTestSupport { + + private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + private static final String TEST_NAME = "QOS2_PUBLISHER_RESILIENCY_SOAK"; + + private static final int NUM_PUBLISHERS = TestParameters.testProperty(TEST_NAME, "NUM_PUBLISHERS", 5); + private static final int NUM_MESSAGES = TestParameters.testProperty(TEST_NAME, "NUM_MESSAGES", 10_000); + private static final int RESTART_PAUSE = TestParameters.testProperty(TEST_NAME, "RESTART_PAUSE", 2_000); + private static final int TIMEOUT_SECONDS = TestParameters.testProperty(TEST_NAME, "TIMEOUT_SECONDS", 180); + + @Test + @Timeout(value = 5, unit = TimeUnit.MINUTES) + public void testQoS2PublisherResiliency() throws Exception { + disableProtocolLogging(); + final String PUB_CLIENT_ID_PREFIX = "pub-"; + final String SUB_CLIENT_ID = "sub"; + + // create subscription queue for consuming messages later + Mqtt5BlockingClient subscriber = createHiveMQClient(SUB_CLIENT_ID, false); + subscriber.connectWith() + .cleanStart(false) + .sessionExpiryInterval(300) + .send(); + subscriber.subscribeWith() + .topicFilter(TOPIC) + .qos(MqttQos.AT_LEAST_ONCE) + .send(); + subscriber.disconnect(); + + assertNotNull(getSubscriptionQueue(TOPIC, SUB_CLIENT_ID)); + + logger.info("{} publishers, {} messages/publisher, {} total expected", NUM_PUBLISHERS, NUM_MESSAGES, NUM_PUBLISHERS * NUM_MESSAGES); + + // create and connect publishers + final List publishers = new ArrayList<>(); + runAfter(() -> publishers.forEach(c -> { + try { + c.disconnect(); + } catch (Exception ignored) { + } + })); + for (int i = 0; i < NUM_PUBLISHERS; i++) { + String clientId = PUB_CLIENT_ID_PREFIX + i; + Mqtt5BlockingClient publisher = createHiveMQClient(clientId, true); + publisher.connectWith() + .cleanStart(false) + .sessionExpiryInterval(300) + .send(); + publishers.add(publisher); + logger.info("Publisher {} connected", clientId); + } + + // start broker restart task + BrokerRestartTask restartTask = startBrokerRestartTask(RESTART_PAUSE, () -> { + logger.info("==========="); + logger.info("Subscription queue received {}/{} messages", getSubscriptionQueue(TOPIC, SUB_CLIENT_ID).getMessageCount(), NUM_PUBLISHERS * NUM_MESSAGES); + logger.info("==========="); + }); + + // enableProtocolLogging(); + + // start publisher tasks + PublishResult publishResult = startPublishing(publishers, NUM_MESSAGES); + + // wait for all publishers to finish + publishResult.executor().shutdown(); + assertTrue(publishResult.executor().awaitTermination(TIMEOUT_SECONDS, TimeUnit.SECONDS), "Publishers did not finish in time"); + logger.info("All publishers finished. Total messages sent: {}. Publish errors: {}", publishResult.sentMessages().size(), publishResult.publishErrors().get()); + + disableProtocolLogging(); + + // stop restart task and ensure broker is running + stopBrokerRestartTask(restartTask); + + final long messageCount = getSubscriptionQueue(TOPIC, SUB_CLIENT_ID).getMessageCount(); + + // reconnect subscriber to verify there are no duplicates + Set consumedMessages = ConcurrentHashMap.newKeySet(NUM_MESSAGES * NUM_PUBLISHERS); + AtomicInteger duplicateCount = new AtomicInteger(0); + subscriber.toAsync().publishes(MqttGlobalPublishFilter.ALL, publish -> { + String payload = new String(publish.getPayloadAsBytes(), StandardCharsets.UTF_8); + if (consumedMessages.contains(payload)) { + logger.warn("Duplicate message: {}", payload); + duplicateCount.incrementAndGet(); + } + consumedMessages.add(payload); + publishResult.sentMessages().remove(payload); + }); + + subscriber.connectWith() + .cleanStart(false) + .sessionExpiryInterval(300) + .send(); + + Wait.waitFor(() -> consumedMessages.size() == messageCount); + + cleanDisconnect(subscriber); + + assertEquals(0, duplicateCount.get()); + assertEquals(0, publishResult.sentMessages().size(), "These messages were published, but were not on the broker: " + publishResult.sentMessages()); + assertEquals(NUM_PUBLISHERS * NUM_MESSAGES, consumedMessages.size()); + + for (Mqtt5BlockingClient publisher : publishers) { + String clientId = getClientId(publisher); + Wait.assertEquals(0, () -> getPubCacheSize(clientId), 5000, 100); + cleanDisconnect(publisher); + assertNull(getPubCache(clientId), "Pub cache should be null after clean start for " + clientId); + } + } +} \ No newline at end of file diff --git a/tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/mqtt/resiliency/QoS2ResiliencySoakTestSupport.java b/tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/mqtt/resiliency/QoS2ResiliencySoakTestSupport.java new file mode 100644 index 000000000000..a3e39805d6e9 --- /dev/null +++ b/tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/mqtt/resiliency/QoS2ResiliencySoakTestSupport.java @@ -0,0 +1,274 @@ +/* + * 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.artemis.tests.soak.mqtt.resiliency; + +import java.lang.invoke.MethodHandles; +import java.nio.charset.StandardCharsets; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import com.hivemq.client.mqtt.datatypes.MqttQos; +import com.hivemq.client.mqtt.mqtt5.Mqtt5BlockingClient; +import com.hivemq.client.mqtt.mqtt5.Mqtt5Client; +import com.hivemq.client.mqtt.mqtt5.message.publish.Mqtt5PublishResult; +import io.reactivex.schedulers.Schedulers; +import org.apache.activemq.artemis.api.core.SimpleString; +import org.apache.activemq.artemis.core.persistence.impl.journal.JournalRecordIds; +import org.apache.activemq.artemis.core.postoffice.DuplicateIDCache; +import org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolManager; +import org.apache.activemq.artemis.core.protocol.mqtt.MQTTUtil; +import org.apache.activemq.artemis.core.protocol.mqtt.PacketIdCache; +import org.apache.activemq.artemis.core.remoting.impl.AbstractAcceptor; +import org.apache.activemq.artemis.core.server.ActiveMQServer; +import org.apache.activemq.artemis.core.server.Queue; +import org.apache.activemq.artemis.spi.core.protocol.ProtocolManager; +import org.apache.activemq.artemis.spi.core.remoting.Acceptor; +import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; +import org.apache.activemq.artemis.utils.Wait; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.config.LoggerConfig; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.apache.activemq.artemis.cli.commands.tools.journal.CompactJournal.compactJournal; +import static org.apache.activemq.artemis.core.persistence.impl.journal.JournalStorageManager.ACTIVEMQ_DATA; +import static org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME; + +/** + * All tests which extend this use the HiveMQ MQTT client because it is the most robust with regard to QoS2 message + * flows and error handling. The Paho client had issues which made it unacceptable for these tests. + */ +public class QoS2ResiliencySoakTestSupport extends ActiveMQTestBase { + + private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + protected static final String TOPIC = "qos2/resiliency"; + protected static final int MQTT_PORT = 1883; + + protected ActiveMQServer server; + + @BeforeEach + @Override + public void setUp() throws Exception { + super.setUp(); + Schedulers.start(); + server = createServer(true, createDefaultConfig(true)); + server.getConfiguration().setJournalMinFiles(10).setJournalFileSize(25 * 1024 * 1024); + server.getConfiguration().addAcceptorConfiguration(MQTT_PROTOCOL_NAME, "tcp://localhost:" + MQTT_PORT + "?protocols=MQTT"); + server.getConfiguration().setMqttSessionScanInterval(200); + + server.start(); + server.waitForActivation(10, TimeUnit.SECONDS); + } + + @AfterEach + @Override + public void tearDown() throws Exception { + if (server != null && server.isStarted()) { + server.stop(); + } + Schedulers.shutdown(); + super.tearDown(); + } + + private static void enableProtocolLogging() { + LoggerContext ctx = (LoggerContext) LogManager.getContext(false); + org.apache.logging.log4j.core.config.Configuration config = ctx.getConfiguration(); + LoggerConfig loggerConfig = new LoggerConfig(MQTTUtil.class.getName(), Level.TRACE, true); + config.addLogger(MQTTUtil.class.getName(), loggerConfig); + ctx.updateLoggers(); + } + + protected static void disableProtocolLogging() { + LoggerContext ctx = (LoggerContext) LogManager.getContext(false); + org.apache.logging.log4j.core.config.Configuration config = ctx.getConfiguration(); + config.removeLogger(MQTTUtil.class.getName()); + ctx.updateLoggers(); + } + + protected Set getMissingMessages(Set expected, Set received) { + Set missing = new HashSet<>(expected); + missing.removeAll(received); + return missing; + } + + protected static String getClientId(Mqtt5BlockingClient subscriber) { + return subscriber.getConfig().getClientIdentifier().get().toString(); + } + + protected Mqtt5BlockingClient createHiveMQClient(String clientId, boolean autoReconnect) { + var builder = Mqtt5Client.builder() + .identifier(clientId) + .serverHost("localhost") + .serverPort(MQTT_PORT); + if (autoReconnect) { + builder.automaticReconnect() + .initialDelay(500, TimeUnit.MILLISECONDS) + .maxDelay(500, TimeUnit.MILLISECONDS) + .applyAutomaticReconnect(); + } + return builder.buildBlocking(); + } + + protected static void waitForClientConnected(Mqtt5BlockingClient client) { + Wait.waitFor(() -> client.getConfig().getState().isConnected(), 30_000, 500); + } + + protected static void cleanDisconnect(Mqtt5BlockingClient client) { + logger.info("cleanDisconnect for {}", getClientId(client)); + try { + if (client.getConfig().getState().isConnected()) { + client.disconnect(); + } + client.connectWith().cleanStart(true).sessionExpiryInterval(0).send(); + client.disconnect(); + } catch (Exception e) { + logger.debug("Error disconnecting: {}", e.getMessage()); + } + } + + protected MQTTProtocolManager getProtocolManager() { + Acceptor acceptor = server.getRemotingService().getAcceptor(MQTT_PROTOCOL_NAME); + if (acceptor instanceof AbstractAcceptor abstractAcceptor) { + ProtocolManager protocolManager = abstractAcceptor.getProtocolMap().get(MQTT_PROTOCOL_NAME); + if (protocolManager instanceof MQTTProtocolManager mqttProtocolManager) { + return mqttProtocolManager; + } + } + return null; + } + + protected DuplicateIDCache getPubCache(String clientId) { + return getCache(clientId, PacketIdCache.TYPE.PUBLISH); + } + + protected int getPubCacheSize(String clientId) { + DuplicateIDCache cache = getPubCache(clientId); + return cache == null ? 0 : cache.getMap().size(); + } + + protected DuplicateIDCache getSubCache(String clientId) { + return getCache(clientId, PacketIdCache.TYPE.PUBREC); + } + + protected int getSubCacheSize(String clientId) { + DuplicateIDCache cache = getSubCache(clientId); + return cache == null ? 0 : cache.getMap().size(); + } + + private DuplicateIDCache getCache(String clientId, PacketIdCache.TYPE type) { + SimpleString cacheName = PacketIdCache.getCacheName(server.getInternalNamingPrefix(), clientId, type); + if (server.getPostOffice().duplicateIDCacheExists(cacheName)) { + return server.getPostOffice().getDuplicateIDCache(cacheName); + } + return null; + } + + protected Queue getSubscriptionQueue(String mqttTopicFilter, String clientId) { + return server.locateQueue(MQTTUtil.getCoreQueueFromMqttTopic(mqttTopicFilter, clientId, server.getConfiguration().getWildcardConfiguration())); + } + + protected record BrokerRestartTask(ScheduledExecutorService scheduler, ScheduledFuture future) {} + + protected BrokerRestartTask startBrokerRestartTask(long delayMillis, Runnable restartLogger) { + ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); + ScheduledFuture future = scheduler.scheduleWithFixedDelay(() -> { + try { + if (restartLogger != null) { + restartLogger.run(); + } + logger.info("Stopping broker"); + server.stop(); + waitForServerToStop(server); + + logger.info("Compacting journal..."); + compactJournal(server.getConfiguration().getJournalLocation(), server.getConfiguration().getJournalRetentionLocation(), ACTIVEMQ_DATA, "amq", server.getConfiguration().getJournalMinFiles(), + server.getConfiguration().getJournalPoolFiles(), server.getConfiguration().getJournalFileSize(), null, JournalRecordIds.UPDATE_DELIVERY_COUNT, + JournalRecordIds.SET_SCHEDULED_DELIVERY_TIME); + logger.info("Compacted journal."); + + server.start(); + waitForServerToStart(server); + } catch (Exception e) { + logger.warn("Error during broker restart", e); + } + }, delayMillis, delayMillis, TimeUnit.MILLISECONDS); + return new BrokerRestartTask(scheduler, future); + } + + protected void stopBrokerRestartTask(BrokerRestartTask restartTask) throws Exception { + restartTask.future().cancel(true); + restartTask.scheduler().shutdownNow(); + restartTask.scheduler().awaitTermination(10, TimeUnit.SECONDS); + if (!server.isStarted()) { + server.start(); + waitForServerToStart(server); + } + } + + protected record PublishResult(ExecutorService executor, Set sentMessages, AtomicInteger publishErrors) {} + + protected PublishResult startPublishing(List publishers, int numMessages) { + final ExecutorService publisherExecutor = Executors.newFixedThreadPool(publishers.size()); + runAfter(() -> { + publisherExecutor.shutdownNow(); + try { + publisherExecutor.awaitTermination(10, TimeUnit.SECONDS); + } catch (InterruptedException ignored) { + } + }); + final Set sentMessages = ConcurrentHashMap.newKeySet(publishers.size() * numMessages); + final AtomicInteger publishErrors = new AtomicInteger(0); + for (int i = 0; i < publishers.size(); i++) { + final int pubId = i; + final Mqtt5BlockingClient publisher = publishers.get(i); + publisherExecutor.execute(() -> { + for (int seq = 0; seq < numMessages; seq++) { + String payload = pubId + "-" + seq; + try { + waitForClientConnected(publisher); + Mqtt5PublishResult result = publisher.publishWith() + .topic(TOPIC) + .qos(MqttQos.EXACTLY_ONCE) + .payload(payload.getBytes(StandardCharsets.UTF_8)) + .send(); + if (result.getError().isPresent()) { + throw result.getError().get(); + } + } catch (Throwable e) { + publishErrors.incrementAndGet(); + logger.info("Pub failed: {}; in-flight QoS 2 state will be resumed on reconnect", payload, e); + } + sentMessages.add(payload); + } + logger.info("Publisher {} finished sending all {} messages", pubId, numMessages); + }); + } + return new PublishResult(publisherExecutor, sentMessages, publishErrors); + } +} diff --git a/tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/mqtt/resiliency/QoS2SubscriberResiliencySoakTest.java b/tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/mqtt/resiliency/QoS2SubscriberResiliencySoakTest.java new file mode 100644 index 000000000000..f456d2a21f41 --- /dev/null +++ b/tests/soak-tests/src/test/java/org/apache/activemq/artemis/tests/soak/mqtt/resiliency/QoS2SubscriberResiliencySoakTest.java @@ -0,0 +1,184 @@ +/* + * 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.artemis.tests.soak.mqtt.resiliency; + +import java.lang.invoke.MethodHandles; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; + +import com.hivemq.client.mqtt.MqttGlobalPublishFilter; +import com.hivemq.client.mqtt.datatypes.MqttQos; +import com.hivemq.client.mqtt.mqtt5.Mqtt5BlockingClient; +import org.apache.activemq.artemis.utils.TestParameters; +import org.apache.activemq.artemis.utils.Wait; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +public class QoS2SubscriberResiliencySoakTest extends QoS2ResiliencySoakTestSupport { + + private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + private static final String TEST_NAME = "QOS2_SUBSCRIBER_RESILIENCY_SOAK"; + + private static final int NUM_SUBSCRIBERS = TestParameters.testProperty(TEST_NAME, "NUM_SUBSCRIBERS", 5); + private static final int NUM_MESSAGES = TestParameters.testProperty(TEST_NAME, "NUM_MESSAGES", 10_000); + private static final int RESTART_PAUSE = TestParameters.testProperty(TEST_NAME, "RESTART_PAUSE", 2_000); + private static final int TIMEOUT_SECONDS = TestParameters.testProperty(TEST_NAME, "TIMEOUT_SECONDS", 180); + + @Test + @Timeout(value = 5, unit = TimeUnit.MINUTES) + public void testQoS2SubscriberResiliency() throws Exception { + disableProtocolLogging(); + logger.info("{} subscribers, {} messages/subscriber", NUM_SUBSCRIBERS, NUM_MESSAGES); + + // create and subscribe then disconnect to leave idle subscriptions on the broker + final List subscribers = new ArrayList<>(NUM_SUBSCRIBERS); + runAfter(() -> subscribers.forEach(c -> { + try { + c.disconnect(); + } catch (Exception ignored) { + } + })); + for (int i = 0; i < NUM_SUBSCRIBERS; i++) { + String clientId = "sub-" + i; + Mqtt5BlockingClient subscriber = createHiveMQClient(clientId, true); + subscriber.connectWith() + .cleanStart(false) + .sessionExpiryInterval(300) + .send(); + subscriber.subscribeWith() + .topicFilter(TOPIC) + .qos(MqttQos.EXACTLY_ONCE) + .send(); + subscriber.disconnect(); + subscribers.add(subscriber); + assertNotNull(getSubscriptionQueue(TOPIC, getClientId(subscriber))); + } + + // send messages using QoS 2 + final Set sentMessages = new HashSet<>(NUM_MESSAGES); + Mqtt5BlockingClient publisher = createHiveMQClient("pub", false); + publisher.connectWith().cleanStart(true).send(); + logger.info("Publishing {} messages...", NUM_MESSAGES); + for (int seq = 0; seq < NUM_MESSAGES; seq++) { + String payload = String.valueOf(seq); + sentMessages.add(payload); + publisher.publishWith() + .topic(TOPIC) + .qos(MqttQos.EXACTLY_ONCE) + .payload(payload.getBytes(StandardCharsets.UTF_8)) + .send(); + } + logger.info("Published {} messages.", NUM_MESSAGES); + cleanDisconnect(publisher); + + for (Mqtt5BlockingClient subscriber : subscribers) { + assertEquals(NUM_MESSAGES, getSubscriptionQueue(TOPIC, getClientId(subscriber)).getMessageCount()); + } + + // enableProtocolLogging(); + + final Map> receivedPerSubscriber = new HashMap<>(); + final Map> duplicatesPerSubscriber = new HashMap<>(); + final AtomicLong lastReceiveTime = new AtomicLong(System.currentTimeMillis()); + + for (Mqtt5BlockingClient subscriber : subscribers) { + final String clientId = getClientId(subscriber); + final Set received = ConcurrentHashMap.newKeySet(NUM_MESSAGES); + receivedPerSubscriber.put(clientId, received); + final Set duplicates = ConcurrentHashMap.newKeySet(); + duplicatesPerSubscriber.put(clientId, duplicates); + subscriber.toAsync().publishes(MqttGlobalPublishFilter.ALL, publish -> { + String payload = new String(publish.getPayloadAsBytes(), StandardCharsets.UTF_8); + if (!received.add(payload)) { + logger.warn("Subscriber {} received duplicate: {}", clientId, payload); + duplicates.add(payload); + } + lastReceiveTime.set(System.currentTimeMillis()); + }); + subscriber.connectWith() + .cleanStart(false) + .sessionExpiryInterval(300) + .send(); + logger.info("Subscriber {} reconnected", clientId); + } + + // start broker restart task + BrokerRestartTask restartTask = startBrokerRestartTask(RESTART_PAUSE, () -> { + logger.info("==========="); + for (Map.Entry> entry : receivedPerSubscriber.entrySet()) { + logger.info("Subscriber {} received {}/{} messages", entry.getKey(), entry.getValue().size(), NUM_MESSAGES); + } + logger.info("Last message received {}ms ago.", System.currentTimeMillis() - lastReceiveTime.get()); + logger.info("==========="); + }); + + final long SUBSCRIBER_TIMEOUT = 20_000; + Wait.assertTrue(() -> { + // quit early if subscribers are dead/stalled for some reason + if (System.currentTimeMillis() - lastReceiveTime.get() > SUBSCRIBER_TIMEOUT) { + for (Map.Entry> entry : receivedPerSubscriber.entrySet()) { + logger.warn("Subscriber {} received {}/{} messages", entry.getKey(), entry.getValue().size(), NUM_MESSAGES); + } + throw new AssertionError("No subscriber has received a message in " + SUBSCRIBER_TIMEOUT / 1000 + " seconds"); + } + // any duplicate is a failure, no need to wait until the end + for (Map.Entry> duplicates : duplicatesPerSubscriber.entrySet()) { + assertEquals(0, duplicates.getValue().size(), "Subscriber " + duplicates.getKey() + " received duplicates: " + duplicates.getValue()); + } + for (Set received : receivedPerSubscriber.values()) { + if (received.size() < NUM_MESSAGES) { + return false; + } + } + return true; + }, TIMEOUT_SECONDS * 1000L, 100); + + disableProtocolLogging(); + + // stop reconnection task, ensure broker is running + stopBrokerRestartTask(restartTask); + + // enableProtocolLogging(); + + // verify all expected messages received with no duplicates + for (Mqtt5BlockingClient subscriber : subscribers) { + String clientId = getClientId(subscriber); + assertEquals(0, duplicatesPerSubscriber.get(clientId).size(), "Subscriber " + getClientId(subscriber) + " received duplicates: " + duplicatesPerSubscriber.get(clientId)); + assertEquals(NUM_MESSAGES, receivedPerSubscriber.get(clientId).size(), "Subscriber " + getClientId(subscriber) + " didn't receive: " + getMissingMessages(sentMessages, receivedPerSubscriber.get(clientId))); + assertEquals(0L, getSubscriptionQueue(TOPIC, getClientId(subscriber)).getMessageCount(), "Subscription queue for " + getClientId(subscriber) + " has incorrect message count"); + assertEquals(0, getProtocolManager().getStateManager().getPacketIdCorrelationSize(getClientId(subscriber))); + assertEquals(0, getSubCacheSize(getClientId(subscriber))); + cleanDisconnect(subscriber); + assertNull(getSubCache(getClientId(subscriber)), "Sub cache should be null after clean start for " + getClientId(subscriber)); + } + } +} \ No newline at end of file