[grepjava]

JournalHashMap.clear() passed each MapRecord to remove(Object key). No key equals a record, so every lookup missed, no delete record was written, and map.clear() emptied the map in memory only. MQTT calls clear() from MQTTStateManager.clearPacketIdCorrelation when a client starts a clean session, so the client's packet ID correlation records stayed in the journal and came back after a restart, holding their packet IDs as in use.

clear() now calls removed(record) for each record, which writes the delete. Removing by key while iterating the values would throw a ConcurrentModificationException.

JournalHashMapTest.testClearRemovesRecords puts 100 entries, clears the map and loads the journal again, and checks no record comes back. It fails without the fix: all 100 records are reloaded.

QoS1SubscriberResiliencyTest.testQoS1CleanStartBeforePubAckSentThenBrokerRestart does the same through MQTT: a subscriber with a persistent session gets a QoS 1 PUBLISH whose PUBACK never reaches the broker, reconnects with a clean start, and the broker restarts. It fails without the fix: the client's packet ID correlation is reloaded.

https://issues.apache.org/jira/browse/ARTEMIS-6336

[jbertram]

Any chance you could work up an MQTT-specific test for this? I could have sworn this was already covered somewhere in one of these:

• org.apache.activemq.artemis.tests.integration.mqtt5.QoS1SubscriberResiliencyTest
• org.apache.activemq.artemis.tests.integration.mqtt5.QoS2PublisherResiliencyTest
• org.apache.activemq.artemis.tests.integration.mqtt5.QoS2SubscriberResiliencyTest

If not, I'll take a closer look tomorrow when I'm back online. Thanks!

[grepjava]

There wasn't one. Those tests clear the session only after the correlations are already gone, and none restarts after a clean start. I added QoS1SubscriberResiliencyTest.testQoS1CleanStartBeforePubAckSentThenBrokerRestart: a QoS 1 PUBLISH whose PUBACK never arrives, then a clean start and a broker restart. Without the fix the client's correlation comes back after the restart.

[jbertram]

Great work, @grepjava. Many thanks!