[jira] [Commented] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring
[ https://issues.apache.org/jira/browse/ARTEMIS-4366?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17744362#comment-17744362 ] ASF subversion and git services commented on ARTEMIS-4366: -- Commit e5b18b80f79dbb7f07f6b43ef424005ea4667a46 in activemq-artemis's branch refs/heads/main from Clebert Suconic [ https://gitbox.apache.org/repos/asf?p=activemq-artemis.git;h=e5b18b80f7 ] ARTEMIS-4366 Some adjustments on MirroredSubscriptionTest > Addresses with multiple subscriptions are not working with Mirroring > > > Key: ARTEMIS-4366 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4366 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Clebert Suconic >Priority: Major > Time Spent: 1h 20m > Remaining Estimate: 0h > > The NodeStore will be reused by the two Queues on the same address. Each > queue should have its own. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4355) Update Сurator to 5.5.0; Zookeeper 3.8.1
[ https://issues.apache.org/jira/browse/ARTEMIS-4355?focusedWorklogId=871643=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871643 ] ASF GitHub Bot logged work on ARTEMIS-4355: --- Author: ASF GitHub Bot Created on: 18/Jul/23 21:23 Start Date: 18/Jul/23 21:23 Worklog Time Spent: 10m Work Description: amarkevich commented on code in PR #4545: URL: https://github.com/apache/activemq-artemis/pull/4545#discussion_r1267318459 ## tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ssl/CoreClientOverOneWaySSLTest.java: ## @@ -85,9 +85,9 @@ public static Collection getParameters() { {"SunJCE", "JCEKS", false, false}, {"SUN", "JKS", false, false}, {"SunJSSE", "PKCS12", false, false}, - {"JCEKS", null, true, false}, // for compatibility with old keyStoreProvider - {"JKS", null, true, false}, // for compatibility with old keyStoreProvider - {"PKCS12", null, true, false} // for compatibility with old keyStoreProvider + {"JCEKS", null, false, false}, // for compatibility with old keyStoreProvider + {"JKS", null, false, false}, // for compatibility with old keyStoreProvider + {"PKCS12", null, false, false} // for compatibility with old keyStoreProvider Review Comment: To pass tests for this PR - otherwise quorum tests are not running. Expected changes in first commit only Issue Time Tracking --- Worklog Id: (was: 871643) Time Spent: 1h 50m (was: 1h 40m) > Update Сurator to 5.5.0; Zookeeper 3.8.1 > > > Key: ARTEMIS-4355 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4355 > Project: ActiveMQ Artemis > Issue Type: Dependency upgrade > Components: clustering >Affects Versions: 2.29.0 >Reporter: Alexey Markevich >Priority: Minor > Time Spent: 1h 50m > Remaining Estimate: 0h > -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Created] (AMQ-9291) networkConnector does not support IPv6 address
Greg Rabil created AMQ-9291: --- Summary: networkConnector does not support IPv6 address Key: AMQ-9291 URL: https://issues.apache.org/jira/browse/AMQ-9291 Project: ActiveMQ Issue Type: Bug Components: Connector Affects Versions: 5.18.2, 5.17.5, 5.16.6 Reporter: Greg Rabil In ActiveMQ 5.15.15, the following networkConnector URI works for connecting to an ActiveMQ broker over IPv6: However, in ActiveMQ 5.16, 5.17, and 5.18, this will fail with the following error (warning): 2023-07-17 12:52:30,825 | WARN | Failed to connect to [ssl://[fd00::15]:61617?verifyHostName=false] after: 1 attempt(s) with Contains non-LDH ASCII characters, continuing to retry. | org.apache.activemq.transport.failover.FailoverTransport | ActiveMQ Failover Worker: 1448780972 The problem is caused by this new code in the org.apache.activemq.transport.tcp.SslTransport.java class: {{ // Lets try to configure the SSL SNI field. Handy in case your using}} {{ // a single proxy to route to different messaging apps.}} {{ final SSLParameters sslParams = new SSLParameters();}} {{ if (remoteLocation != null) {}} {{ sslParams.setServerNames(Collections.singletonList(new SNIHostName(remoteLocation.getHost(;}} {{ }}} The remoteLocation.getHost() will return "[fd00::15]", which causes the exception in the SNIHostName constructor. It seems that the above condition should be: {{ if ((remoteLocation != null) && verifyHostName) {}} {{ sslParams.setServerNames(Collections.singletonList(new SNIHostName(remoteLocation.getHost(;}} {{ }}} Because the SNIHostName only makes sense in the context of verifying the hostname of the server certificate. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4355) Update Сurator to 5.5.0; Zookeeper 3.8.1
[ https://issues.apache.org/jira/browse/ARTEMIS-4355?focusedWorklogId=871628=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871628 ] ASF GitHub Bot logged work on ARTEMIS-4355: --- Author: ASF GitHub Bot Created on: 18/Jul/23 20:26 Start Date: 18/Jul/23 20:26 Worklog Time Spent: 10m Work Description: jbertram commented on code in PR #4545: URL: https://github.com/apache/activemq-artemis/pull/4545#discussion_r1267273788 ## tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ssl/CoreClientOverOneWaySSLTest.java: ## @@ -85,9 +85,9 @@ public static Collection getParameters() { {"SunJCE", "JCEKS", false, false}, {"SUN", "JKS", false, false}, {"SunJSSE", "PKCS12", false, false}, - {"JCEKS", null, true, false}, // for compatibility with old keyStoreProvider - {"JKS", null, true, false}, // for compatibility with old keyStoreProvider - {"PKCS12", null, true, false} // for compatibility with old keyStoreProvider + {"JCEKS", null, false, false}, // for compatibility with old keyStoreProvider + {"JKS", null, false, false}, // for compatibility with old keyStoreProvider + {"PKCS12", null, false, false} // for compatibility with old keyStoreProvider Review Comment: I don't understand why these changes were necessary. This eliminates the tests for the warning log message. Issue Time Tracking --- Worklog Id: (was: 871628) Time Spent: 1h 40m (was: 1.5h) > Update Сurator to 5.5.0; Zookeeper 3.8.1 > > > Key: ARTEMIS-4355 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4355 > Project: ActiveMQ Artemis > Issue Type: Dependency upgrade > Components: clustering >Affects Versions: 2.29.0 >Reporter: Alexey Markevich >Priority: Minor > Time Spent: 1h 40m > Remaining Estimate: 0h > -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly
[ https://issues.apache.org/jira/browse/ARTEMIS-4365?focusedWorklogId=871616=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871616 ] ASF GitHub Bot logged work on ARTEMIS-4365: --- Author: ASF GitHub Bot Created on: 18/Jul/23 19:26 Start Date: 18/Jul/23 19:26 Worklog Time Spent: 10m Work Description: jbertram commented on code in PR #4556: URL: https://github.com/apache/activemq-artemis/pull/4556#discussion_r1267222798 ## artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTPublishManager.java: ## @@ -404,12 +406,15 @@ private boolean publishToClient(int messageId, ICoreMessage message, int deliver // [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.getBooleanProperty(MQTT_MESSAGE_RETAIN_KEY); + boolean isRetain = message.containsProperty(MQTT_MESSAGE_RETAIN_INITIAL_DISTRIBUTION_KEY); MqttProperties mqttProperties = getPublishProperties(message); if (session.getVersion() == MQTTVersion.MQTT_5) { - if (session.getState().getSubscription(message.getAddress()) != null && !session.getState().getSubscription(message.getAddress()).option().isRetainAsPublished()) { -isRetain = false; + if (message.getBooleanProperty(MQTT_MESSAGE_RETAIN_KEY) && !message.containsProperty(MQTT_MESSAGE_RETAIN_INITIAL_DISTRIBUTION_KEY)) { Review Comment: I considered doing that, but it didn't seem as clear as I wanted it to be at the time. Taking another look I think it's fine. Issue Time Tracking --- Worklog Id: (was: 871616) Time Spent: 1.5h (was: 1h 20m) > MQTT retain flag not set correctly > -- > > Key: ARTEMIS-4365 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4365 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Justin Bertram >Assignee: Justin Bertram >Priority: Major > Time Spent: 1.5h > Remaining Estimate: 0h > > The {{retain}} flag set on MQTT messages dispatched to clients is incorrect > in certain circumstances. This is demonstrated by using the {{test}} command > from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.: > {noformat} > $ mqtt test --all > MQTT 3: OK > - Maximum topic length: 65535 bytes > - QoS 0: Received 10/10 publishes in 4847.01ms > - QoS 1: Received 10/10 publishes in 27413.45ms > - QoS 2: Received 10/10 publishes in 49551.40ms > - Retain: OK > - Wildcard subscriptions: OK > - Shared subscriptions: OK > - Payload size: >= 10 bytes > - Maximum client id length: 65535 bytes > - Unsupported Ascii Chars: ALL SUPPORTED > MQTT 5: OK > - Connect restrictions: > > Retain: OK > > Wildcard subscriptions: OK > > Shared subscriptions: OK > > Subscription identifiers: OK > > Maximum QoS: 2 > > Receive maximum: 65535 > > Maximum packet size: 268435455 bytes > > Topic alias maximum: 65535 > > Session expiry interval: Client-based > > Server keep alive: Client-based > - Maximum topic length: 65535 bytes > - QoS 0: Received 10/10 publishes in 706.21ms > - QoS 1: Received 10/10 publishes in 805.38ms > - QoS 2: Received 10/10 publishes in 972.98ms > - Retain: TIME_OUT > - Wildcard subscriptions: OK > - Shared subscriptions: OK > - Payload size: >= 10 bytes > - Maximum client id length: 65535 bytes > - Unsupported Ascii Chars: ALL SUPPORTED{noformat} > Notice the result of {{TIME_OUT}} when testing retain functionality for MQTT > 5. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly
[ https://issues.apache.org/jira/browse/ARTEMIS-4365?focusedWorklogId=871602=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871602 ] ASF GitHub Bot logged work on ARTEMIS-4365: --- Author: ASF GitHub Bot Created on: 18/Jul/23 17:43 Start Date: 18/Jul/23 17:43 Worklog Time Spent: 10m Work Description: gemmellr commented on code in PR #4556: URL: https://github.com/apache/activemq-artemis/pull/4556#discussion_r1267114894 ## artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTPublishManager.java: ## @@ -404,12 +406,15 @@ private boolean publishToClient(int messageId, ICoreMessage message, int deliver // [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.getBooleanProperty(MQTT_MESSAGE_RETAIN_KEY); + boolean isRetain = message.containsProperty(MQTT_MESSAGE_RETAIN_INITIAL_DISTRIBUTION_KEY); MqttProperties mqttProperties = getPublishProperties(message); if (session.getVersion() == MQTTVersion.MQTT_5) { - if (session.getState().getSubscription(message.getAddress()) != null && !session.getState().getSubscription(message.getAddress()).option().isRetainAsPublished()) { -isRetain = false; + if (message.getBooleanProperty(MQTT_MESSAGE_RETAIN_KEY) && !message.containsProperty(MQTT_MESSAGE_RETAIN_INITIAL_DISTRIBUTION_KEY)) { Review Comment: Since we already know the presence result from the earlier check, and only need to do the rest if that was false, what about: ```suggestion if (!isRetain && message.getBooleanProperty(MQTT_MESSAGE_RETAIN_KEY)) { ``` Issue Time Tracking --- Worklog Id: (was: 871602) Time Spent: 1h 20m (was: 1h 10m) > MQTT retain flag not set correctly > -- > > Key: ARTEMIS-4365 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4365 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Justin Bertram >Assignee: Justin Bertram >Priority: Major > Time Spent: 1h 20m > Remaining Estimate: 0h > > The {{retain}} flag set on MQTT messages dispatched to clients is incorrect > in certain circumstances. This is demonstrated by using the {{test}} command > from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.: > {noformat} > $ mqtt test --all > MQTT 3: OK > - Maximum topic length: 65535 bytes > - QoS 0: Received 10/10 publishes in 4847.01ms > - QoS 1: Received 10/10 publishes in 27413.45ms > - QoS 2: Received 10/10 publishes in 49551.40ms > - Retain: OK > - Wildcard subscriptions: OK > - Shared subscriptions: OK > - Payload size: >= 10 bytes > - Maximum client id length: 65535 bytes > - Unsupported Ascii Chars: ALL SUPPORTED > MQTT 5: OK > - Connect restrictions: > > Retain: OK > > Wildcard subscriptions: OK > > Shared subscriptions: OK > > Subscription identifiers: OK > > Maximum QoS: 2 > > Receive maximum: 65535 > > Maximum packet size: 268435455 bytes > > Topic alias maximum: 65535 > > Session expiry interval: Client-based > > Server keep alive: Client-based > - Maximum topic length: 65535 bytes > - QoS 0: Received 10/10 publishes in 706.21ms > - QoS 1: Received 10/10 publishes in 805.38ms > - QoS 2: Received 10/10 publishes in 972.98ms > - Retain: TIME_OUT > - Wildcard subscriptions: OK > - Shared subscriptions: OK > - Payload size: >= 10 bytes > - Maximum client id length: 65535 bytes > - Unsupported Ascii Chars: ALL SUPPORTED{noformat} > Notice the result of {{TIME_OUT}} when testing retain functionality for MQTT > 5. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring
[ https://issues.apache.org/jira/browse/ARTEMIS-4366?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17744303#comment-17744303 ] ASF subversion and git services commented on ARTEMIS-4366: -- Commit 29deb30c738ea765fbed738aa3e36517c3c89ca4 in activemq-artemis's branch refs/heads/main from Clebert Suconic [ https://gitbox.apache.org/repos/asf?p=activemq-artemis.git;h=29deb30c73 ] ARTEMIS-4366 Some Adjustment to class names on Mirror This is in relation to comments from PR #4555 > Addresses with multiple subscriptions are not working with Mirroring > > > Key: ARTEMIS-4366 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4366 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Clebert Suconic >Priority: Major > Time Spent: 1h 20m > Remaining Estimate: 0h > > The NodeStore will be reused by the two Queues on the same address. Each > queue should have its own. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring
[ https://issues.apache.org/jira/browse/ARTEMIS-4366?focusedWorklogId=871599=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871599 ] ASF GitHub Bot logged work on ARTEMIS-4366: --- Author: ASF GitHub Bot Created on: 18/Jul/23 17:11 Start Date: 18/Jul/23 17:11 Worklog Time Spent: 10m Work Description: clebertsuconic merged PR #4557: URL: https://github.com/apache/activemq-artemis/pull/4557 Issue Time Tracking --- Worklog Id: (was: 871599) Time Spent: 1h 20m (was: 1h 10m) > Addresses with multiple subscriptions are not working with Mirroring > > > Key: ARTEMIS-4366 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4366 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Clebert Suconic >Priority: Major > Time Spent: 1h 20m > Remaining Estimate: 0h > > The NodeStore will be reused by the two Queues on the same address. Each > queue should have its own. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly
[ https://issues.apache.org/jira/browse/ARTEMIS-4365?focusedWorklogId=871598=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871598 ] ASF GitHub Bot logged work on ARTEMIS-4365: --- Author: ASF GitHub Bot Created on: 18/Jul/23 17:11 Start Date: 18/Jul/23 17:11 Worklog Time Spent: 10m Work Description: jbertram commented on PR #4556: URL: https://github.com/apache/activemq-artemis/pull/4556#issuecomment-1640635646 I fixed the failure coming from `MQTTInterceptorPropertiesTest.checkMessageProperties`. The failure was due to the new (correct) semantics for the `retain` flag. Issue Time Tracking --- Worklog Id: (was: 871598) Time Spent: 1h 10m (was: 1h) > MQTT retain flag not set correctly > -- > > Key: ARTEMIS-4365 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4365 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Justin Bertram >Assignee: Justin Bertram >Priority: Major > Time Spent: 1h 10m > Remaining Estimate: 0h > > The {{retain}} flag set on MQTT messages dispatched to clients is incorrect > in certain circumstances. This is demonstrated by using the {{test}} command > from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.: > {noformat} > $ mqtt test --all > MQTT 3: OK > - Maximum topic length: 65535 bytes > - QoS 0: Received 10/10 publishes in 4847.01ms > - QoS 1: Received 10/10 publishes in 27413.45ms > - QoS 2: Received 10/10 publishes in 49551.40ms > - Retain: OK > - Wildcard subscriptions: OK > - Shared subscriptions: OK > - Payload size: >= 10 bytes > - Maximum client id length: 65535 bytes > - Unsupported Ascii Chars: ALL SUPPORTED > MQTT 5: OK > - Connect restrictions: > > Retain: OK > > Wildcard subscriptions: OK > > Shared subscriptions: OK > > Subscription identifiers: OK > > Maximum QoS: 2 > > Receive maximum: 65535 > > Maximum packet size: 268435455 bytes > > Topic alias maximum: 65535 > > Session expiry interval: Client-based > > Server keep alive: Client-based > - Maximum topic length: 65535 bytes > - QoS 0: Received 10/10 publishes in 706.21ms > - QoS 1: Received 10/10 publishes in 805.38ms > - QoS 2: Received 10/10 publishes in 972.98ms > - Retain: TIME_OUT > - Wildcard subscriptions: OK > - Shared subscriptions: OK > - Payload size: >= 10 bytes > - Maximum client id length: 65535 bytes > - Unsupported Ascii Chars: ALL SUPPORTED{noformat} > Notice the result of {{TIME_OUT}} when testing retain functionality for MQTT > 5. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly
[ https://issues.apache.org/jira/browse/ARTEMIS-4365?focusedWorklogId=871597=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871597 ] ASF GitHub Bot logged work on ARTEMIS-4365: --- Author: ASF GitHub Bot Created on: 18/Jul/23 17:10 Start Date: 18/Jul/23 17:10 Worklog Time Spent: 10m Work Description: jbertram commented on code in PR #4556: URL: https://github.com/apache/activemq-artemis/pull/4556#discussion_r1267082367 ## tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MQTTTest.java: ## @@ -2219,4 +2223,71 @@ public void testAutoDeleteRetainedQueue() throws Exception { Wait.assertTrue(() -> server.locateQueue(RETAINED_QUEUE).getMessageCount() == 0, 3000, 50); Wait.assertTrue(() -> server.locateQueue(RETAINED_QUEUE) == null, 3000, 50); } + + /* +* [MQTT-3.3.1-9] When sending a PUBLISH Packet to a Client the Server...MUST set the RETAIN flag to 0 when a PUBLISH +* Packet is sent to a Client because it matches an *established* subscription regardless of how the flag was set in +* the message it received. +*/ + @Test(timeout = 60 * 1000) + public void testRetainFlagOnEstablishedSubscription() throws Exception { + CountDownLatch latch = new CountDownLatch(1); + final String topic = RandomUtil.randomString(); + + MqttClient subscriber = createPaho3_1_1Client("subscriber"); + subscriber.connect(); + subscriber.subscribe(topic, 1); + subscriber.setCallback(new DefaultMqtt3Callback() { + @Override + public void messageArrived(String topic, MqttMessage message) throws Exception { +if (!message.isRetained()) { + latch.countDown(); +} + } + }); + + MqttClient publisher = createPaho3_1_1Client("publisher"); + publisher.connect(); + publisher.publish(topic, "retained".getBytes(StandardCharsets.UTF_8), 1, true); + publisher.disconnect(); + publisher.close(); + + assertTrue(latch.await(1000, TimeUnit.MILLISECONDS)); + + subscriber.disconnect(); + subscriber.close(); + } + + /* +* [MQTT-3.3.1-8] When sending a PUBLISH Packet to a Client the Server MUST set the RETAIN flag to 1 if a message is +* sent as a result of a new subscription being made by a Client. +*/ + @Test(timeout = 60 * 1000) + public void testRetainFlagOnNewSubscription() throws Exception { + CountDownLatch latch = new CountDownLatch(1); + final String topic = RandomUtil.randomString(); + + MqttClient publisher = createPaho3_1_1Client("publisher"); + publisher.connect(); + publisher.publish(topic, "retained".getBytes(StandardCharsets.UTF_8), 1, true); + publisher.disconnect(); + publisher.close(); + + MqttClient subscriber = createPaho3_1_1Client("subscriber"); + subscriber.connect(); + subscriber.subscribe(topic, 1); + subscriber.setCallback(new DefaultMqtt3Callback() { + @Override + public void messageArrived(String topic, MqttMessage message) throws Exception { +if (message.isRetained()) { + latch.countDown(); +} + } + }); + + assertTrue(latch.await(1000, TimeUnit.MILLISECONDS)); Review Comment: I also added messages to the relevant assertions. Issue Time Tracking --- Worklog Id: (was: 871597) Time Spent: 1h (was: 50m) > MQTT retain flag not set correctly > -- > > Key: ARTEMIS-4365 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4365 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Justin Bertram >Assignee: Justin Bertram >Priority: Major > Time Spent: 1h > Remaining Estimate: 0h > > The {{retain}} flag set on MQTT messages dispatched to clients is incorrect > in certain circumstances. This is demonstrated by using the {{test}} command > from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.: > {noformat} > $ mqtt test --all > MQTT 3: OK > - Maximum topic length: 65535 bytes > - QoS 0: Received 10/10 publishes in 4847.01ms > - QoS 1: Received 10/10 publishes in 27413.45ms > - QoS 2: Received 10/10 publishes in 49551.40ms > - Retain: OK > - Wildcard subscriptions: OK > - Shared subscriptions: OK > - Payload size: >= 10 bytes > - Maximum client id length: 65535 bytes > - Unsupported Ascii Chars: ALL SUPPORTED > MQTT 5: OK > - Connect restrictions: > > Retain: OK > > Wildcard subscriptions: OK > > Shared subscriptions: OK > > Subscription identifiers: OK > > Maximum QoS: 2 > > Receive maximum: 65535 > > Maximum
[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly
[ https://issues.apache.org/jira/browse/ARTEMIS-4365?focusedWorklogId=871596=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871596 ] ASF GitHub Bot logged work on ARTEMIS-4365: --- Author: ASF GitHub Bot Created on: 18/Jul/23 17:07 Start Date: 18/Jul/23 17:07 Worklog Time Spent: 10m Work Description: jbertram commented on code in PR #4556: URL: https://github.com/apache/activemq-artemis/pull/4556#discussion_r1267078926 ## tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MQTTTest.java: ## @@ -2219,4 +2223,71 @@ public void testAutoDeleteRetainedQueue() throws Exception { Wait.assertTrue(() -> server.locateQueue(RETAINED_QUEUE).getMessageCount() == 0, 3000, 50); Wait.assertTrue(() -> server.locateQueue(RETAINED_QUEUE) == null, 3000, 50); } + + /* +* [MQTT-3.3.1-9] When sending a PUBLISH Packet to a Client the Server...MUST set the RETAIN flag to 0 when a PUBLISH +* Packet is sent to a Client because it matches an *established* subscription regardless of how the flag was set in +* the message it received. +*/ + @Test(timeout = 60 * 1000) + public void testRetainFlagOnEstablishedSubscription() throws Exception { + CountDownLatch latch = new CountDownLatch(1); + final String topic = RandomUtil.randomString(); + + MqttClient subscriber = createPaho3_1_1Client("subscriber"); + subscriber.connect(); + subscriber.subscribe(topic, 1); + subscriber.setCallback(new DefaultMqtt3Callback() { + @Override + public void messageArrived(String topic, MqttMessage message) throws Exception { +if (!message.isRetained()) { + latch.countDown(); +} + } + }); + + MqttClient publisher = createPaho3_1_1Client("publisher"); + publisher.connect(); + publisher.publish(topic, "retained".getBytes(StandardCharsets.UTF_8), 1, true); + publisher.disconnect(); + publisher.close(); + + assertTrue(latch.await(1000, TimeUnit.MILLISECONDS)); + + subscriber.disconnect(); + subscriber.close(); + } + + /* +* [MQTT-3.3.1-8] When sending a PUBLISH Packet to a Client the Server MUST set the RETAIN flag to 1 if a message is +* sent as a result of a new subscription being made by a Client. +*/ + @Test(timeout = 60 * 1000) + public void testRetainFlagOnNewSubscription() throws Exception { + CountDownLatch latch = new CountDownLatch(1); + final String topic = RandomUtil.randomString(); + + MqttClient publisher = createPaho3_1_1Client("publisher"); + publisher.connect(); + publisher.publish(topic, "retained".getBytes(StandardCharsets.UTF_8), 1, true); + publisher.disconnect(); + publisher.close(); + + MqttClient subscriber = createPaho3_1_1Client("subscriber"); + subscriber.connect(); + subscriber.subscribe(topic, 1); + subscriber.setCallback(new DefaultMqtt3Callback() { + @Override + public void messageArrived(String topic, MqttMessage message) throws Exception { +if (message.isRetained()) { + latch.countDown(); +} + } + }); + + assertTrue(latch.await(1000, TimeUnit.MILLISECONDS)); Review Comment: I fixed a race condition between the `subsribe` and `setCallback`. I can't get it to fail anymore. Issue Time Tracking --- Worklog Id: (was: 871596) Time Spent: 50m (was: 40m) > MQTT retain flag not set correctly > -- > > Key: ARTEMIS-4365 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4365 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Justin Bertram >Assignee: Justin Bertram >Priority: Major > Time Spent: 50m > Remaining Estimate: 0h > > The {{retain}} flag set on MQTT messages dispatched to clients is incorrect > in certain circumstances. This is demonstrated by using the {{test}} command > from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.: > {noformat} > $ mqtt test --all > MQTT 3: OK > - Maximum topic length: 65535 bytes > - QoS 0: Received 10/10 publishes in 4847.01ms > - QoS 1: Received 10/10 publishes in 27413.45ms > - QoS 2: Received 10/10 publishes in 49551.40ms > - Retain: OK > - Wildcard subscriptions: OK > - Shared subscriptions: OK > - Payload size: >= 10 bytes > - Maximum client id length: 65535 bytes > - Unsupported Ascii Chars: ALL SUPPORTED > MQTT 5: OK > - Connect restrictions: > > Retain: OK > > Wildcard subscriptions: OK > > Shared subscriptions: OK > > Subscription identifiers: OK > > Maximum QoS: 2 >
[jira] [Commented] (ARTEMIS-4367) Split server logger codes into advice and info category and by logger
[ https://issues.apache.org/jira/browse/ARTEMIS-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17744275#comment-17744275 ] Robbie Gemmell commented on ARTEMIS-4367: - An approach I've seen used elsewhere is having such 'operational messages' given individual loggers in a hierarchy. That way they can all be configured 'as a whole' typically (often not configured specifically at all, just using the parent or root level logger config)...but can also still be configured very granularly if needed (i.e per message) as they also had their own log category. In this case each existing 'IDd message' has a unique code, and currently they all use a specific logger category, e.g 'the client logger' or 'the server logger' or 'journal logger'. Since all the logging stuff for these is generated, they could easily use a sub-category generated based on the code. I guess one consideration would be, how many of them are there, and would that be an issue. On the other side of that though, consider that outside of these specific 'IDd logger classes', every single implementation class with any logging has its own logger. > Split server logger codes into advice and info category and by logger > - > > Key: ARTEMIS-4367 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4367 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.29.0 >Reporter: Gary Tully >Priority: Major > > We have server logger that uses AMQXXX codes for log messages. all is good. > We have a class of log message that is advice... > * address X does not a DLQ registered > * address X does not an expiry address registered > These advice are info or warn trying to help users but often times the config > is by design and this advice is just noise. Users want to disable, but not > all of the server logger infor or warn messages. > log4j allows to filter, but that is expensive. > If we split out these advice messages into a separate class and use a nested > logger per code, it would be simple to suppress advice that is noise for a > given scenario > There was an new advice in ARTEMIS-4362 that suggests we do have a class of > advices/warns that would benefit from a simple way to suppress. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Updated] (ARTEMIS-4367) Split server logger codes into advice and info category and by logger
[ https://issues.apache.org/jira/browse/ARTEMIS-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Gary Tully updated ARTEMIS-4367: Description: We have server logger that uses AMQXXX codes for log messages. all is good. We have a class of log message that is advice... * address X does not a DLQ registered * address X does not an expiry address registered These advice are info or warn trying to help users but often times the config is by design and this advice is just noise. Users want to disable, but not all of the server logger infor or warn messages. log4j allows to filter, but that is expensive. If we split out these advice messages into a separate class and use a nested logger per code, it would be simple to suppress advice that is noise for a given scenario There was an new advice in ARTEMIS-4362 that suggests we do have a class of advices/warns that would benefit from a simple way to suppress. was: We have server logger that uses AMQXXX codes for log messages. all is good. We have a class of log message that is advice... * address X does not a DLQ registered * address X does not an expiry address registered These advice are info or warn trying to help users but often times the config is by design and this advice is just noise. Users want to disable, but not all of the server logger infor or warn messages. log4j allows to filter, but that is expensive. If we split out these advice messages into a separate class and use a nested logger per code, it would be simple to suppress advice that is noise for a given scenario There was an new advice in AMQ-6854 that suggests we do have a class of advices/warns that would benefit from a simple way to suppress. > Split server logger codes into advice and info category and by logger > - > > Key: ARTEMIS-4367 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4367 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.29.0 >Reporter: Gary Tully >Priority: Major > > We have server logger that uses AMQXXX codes for log messages. all is good. > We have a class of log message that is advice... > * address X does not a DLQ registered > * address X does not an expiry address registered > These advice are info or warn trying to help users but often times the config > is by design and this advice is just noise. Users want to disable, but not > all of the server logger infor or warn messages. > log4j allows to filter, but that is expensive. > If we split out these advice messages into a separate class and use a nested > logger per code, it would be simple to suppress advice that is noise for a > given scenario > There was an new advice in ARTEMIS-4362 that suggests we do have a class of > advices/warns that would benefit from a simple way to suppress. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Commented] (ARTEMIS-4367) Split server logger codes into advice and info category and by logger
[ https://issues.apache.org/jira/browse/ARTEMIS-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17744266#comment-17744266 ] Gary Tully commented on ARTEMIS-4367: - an example of filtering from [~jbertram] # Console appender appender.console.type=Console appender.console.name=console appender.console.layout.type=PatternLayout appender.console.layout.pattern=%d %-5level [%logger] %msg%n appender.console.filter.no_dla_or_expiry_warns.type=RegexFilter appender.console.filter.no_dla_or_expiry_warns.regex=.*AMQ222165.*|.*AMQ222166.* appender.console.filter.no_dla_or_expiry_warns.onMatch=DENY appender.console.filter.no_dla_or_expiry_warns.onMismatch=ACCEPT > Split server logger codes into advice and info category and by logger > - > > Key: ARTEMIS-4367 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4367 > Project: ActiveMQ Artemis > Issue Type: Bug > Components: Broker >Affects Versions: 2.29.0 >Reporter: Gary Tully >Priority: Major > > We have server logger that uses AMQXXX codes for log messages. all is good. > We have a class of log message that is advice... > * address X does not a DLQ registered > * address X does not an expiry address registered > These advice are info or warn trying to help users but often times the config > is by design and this advice is just noise. Users want to disable, but not > all of the server logger infor or warn messages. > log4j allows to filter, but that is expensive. > If we split out these advice messages into a separate class and use a nested > logger per code, it would be simple to suppress advice that is noise for a > given scenario > There was an new advice in AMQ-6854 that suggests we do have a class of > advices/warns that would benefit from a simple way to suppress. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Updated] (ARTEMIS-4367) Split server logger codes into advice and info category and by logger
[ https://issues.apache.org/jira/browse/ARTEMIS-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Gary Tully updated ARTEMIS-4367: Issue Type: Improvement (was: Bug) > Split server logger codes into advice and info category and by logger > - > > Key: ARTEMIS-4367 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4367 > Project: ActiveMQ Artemis > Issue Type: Improvement > Components: Broker >Affects Versions: 2.29.0 >Reporter: Gary Tully >Priority: Major > > We have server logger that uses AMQXXX codes for log messages. all is good. > We have a class of log message that is advice... > * address X does not a DLQ registered > * address X does not an expiry address registered > These advice are info or warn trying to help users but often times the config > is by design and this advice is just noise. Users want to disable, but not > all of the server logger infor or warn messages. > log4j allows to filter, but that is expensive. > If we split out these advice messages into a separate class and use a nested > logger per code, it would be simple to suppress advice that is noise for a > given scenario > There was an new advice in AMQ-6854 that suggests we do have a class of > advices/warns that would benefit from a simple way to suppress. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Updated] (ARTEMIS-4367) Split server logger codes into advice and info category and by logger
[ https://issues.apache.org/jira/browse/ARTEMIS-4367?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Gary Tully updated ARTEMIS-4367: Summary: Split server logger codes into advice and info category and by logger (was: Split server logger codes into advice and info and by logger) > Split server logger codes into advice and info category and by logger > - > > Key: ARTEMIS-4367 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4367 > Project: ActiveMQ Artemis > Issue Type: Bug > Components: Broker >Affects Versions: 2.29.0 >Reporter: Gary Tully >Priority: Major > > We have server logger that uses AMQXXX codes for log messages. all is good. > We have a class of log message that is advice... > * address X does not a DLQ registered > * address X does not an expiry address registered > These advice are info or warn trying to help users but often times the config > is by design and this advice is just noise. Users want to disable, but not > all of the server logger infor or warn messages. > log4j allows to filter, but that is expensive. > If we split out these advice messages into a separate class and use a nested > logger per code, it would be simple to suppress advice that is noise for a > given scenario > There was an new advice in AMQ-6854 that suggests we do have a class of > advices/warns that would benefit from a simple way to suppress. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Created] (ARTEMIS-4367) Split server logger codes into advice and info and by logger
Gary Tully created ARTEMIS-4367: --- Summary: Split server logger codes into advice and info and by logger Key: ARTEMIS-4367 URL: https://issues.apache.org/jira/browse/ARTEMIS-4367 Project: ActiveMQ Artemis Issue Type: Bug Components: Broker Affects Versions: 2.29.0 Reporter: Gary Tully We have server logger that uses AMQXXX codes for log messages. all is good. We have a class of log message that is advice... * address X does not a DLQ registered * address X does not an expiry address registered These advice are info or warn trying to help users but often times the config is by design and this advice is just noise. Users want to disable, but not all of the server logger infor or warn messages. log4j allows to filter, but that is expensive. If we split out these advice messages into a separate class and use a nested logger per code, it would be simple to suppress advice that is noise for a given scenario There was an new advice in AMQ-6854 that suggests we do have a class of advices/warns that would benefit from a simple way to suppress. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring
[ https://issues.apache.org/jira/browse/ARTEMIS-4366?focusedWorklogId=871555=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871555 ] ASF GitHub Bot logged work on ARTEMIS-4366: --- Author: ASF GitHub Bot Created on: 18/Jul/23 13:51 Start Date: 18/Jul/23 13:51 Worklog Time Spent: 10m Work Description: gemmellr commented on code in PR #4555: URL: https://github.com/apache/activemq-artemis/pull/4555#discussion_r1266809419 ## artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/ProtonProtocolManager.java: ## @@ -125,11 +125,11 @@ public ProtonProtocolManager(ProtonProtocolManagerFactory factory, ActiveMQServe routingHandler = new AMQPRoutingHandler(server); } - public synchronized ReferenceNodeStore getReferenceIDSupplier() { + public synchronized ReferenceNodeStoreFactory getReferenceIDSupplier() { Review Comment: Think the only one that wouldnt go static is getDefaultNodeID() , which didnt originally exist as the caller already had the value (serverId). Renaming would be good...the current naming just doesnt fit, makes it confusing to read. Issue Time Tracking --- Worklog Id: (was: 871555) Time Spent: 1h 10m (was: 1h) > Addresses with multiple subscriptions are not working with Mirroring > > > Key: ARTEMIS-4366 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4366 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Clebert Suconic >Priority: Major > Time Spent: 1h 10m > Remaining Estimate: 0h > > The NodeStore will be reused by the two Queues on the same address. Each > queue should have its own. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring
[ https://issues.apache.org/jira/browse/ARTEMIS-4366?focusedWorklogId=871551=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871551 ] ASF GitHub Bot logged work on ARTEMIS-4366: --- Author: ASF GitHub Bot Created on: 18/Jul/23 13:31 Start Date: 18/Jul/23 13:31 Worklog Time Spent: 10m Work Description: clebertsuconic opened a new pull request, #4557: URL: https://github.com/apache/activemq-artemis/pull/4557 This is in relation to comments from PR #4555 Issue Time Tracking --- Worklog Id: (was: 871551) Time Spent: 1h (was: 50m) > Addresses with multiple subscriptions are not working with Mirroring > > > Key: ARTEMIS-4366 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4366 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Clebert Suconic >Priority: Major > Time Spent: 1h > Remaining Estimate: 0h > > The NodeStore will be reused by the two Queues on the same address. Each > queue should have its own. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring
[ https://issues.apache.org/jira/browse/ARTEMIS-4366?focusedWorklogId=871547=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871547 ] ASF GitHub Bot logged work on ARTEMIS-4366: --- Author: ASF GitHub Bot Created on: 18/Jul/23 13:02 Start Date: 18/Jul/23 13:02 Worklog Time Spent: 10m Work Description: clebertsuconic commented on code in PR #4555: URL: https://github.com/apache/activemq-artemis/pull/4555#discussion_r1266740736 ## artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/ProtonProtocolManager.java: ## @@ -125,11 +125,11 @@ public ProtonProtocolManager(ProtonProtocolManagerFactory factory, ActiveMQServe routingHandler = new AMQPRoutingHandler(server); } - public synchronized ReferenceNodeStore getReferenceIDSupplier() { + public synchronized ReferenceNodeStoreFactory getReferenceIDSupplier() { Review Comment: It still the ID supplier. That's a reason why I actually didn't make those methods static. although one of them can't be made static. I can rename this as IDSupplier. it would implement the NodeStoreFactory and return a new instance of NodeStore. Issue Time Tracking --- Worklog Id: (was: 871547) Time Spent: 50m (was: 40m) > Addresses with multiple subscriptions are not working with Mirroring > > > Key: ARTEMIS-4366 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4366 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Clebert Suconic >Priority: Major > Time Spent: 50m > Remaining Estimate: 0h > > The NodeStore will be reused by the two Queues on the same address. Each > queue should have its own. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly
[ https://issues.apache.org/jira/browse/ARTEMIS-4365?focusedWorklogId=871539=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871539 ] ASF GitHub Bot logged work on ARTEMIS-4365: --- Author: ASF GitHub Bot Created on: 18/Jul/23 12:33 Start Date: 18/Jul/23 12:33 Worklog Time Spent: 10m Work Description: gemmellr commented on PR #4556: URL: https://github.com/apache/activemq-artemis/pull/4556#issuecomment-1640128430 Also saw another test failure in a CI run, in addition to the one added in the code. > java.lang.AssertionError: expected:\ but was:\ at org.apache.activemq.artemis.tests.integration.mqtt.MQTTInterceptorPropertiesTest.checkMessageProperties(MQTTInterceptorPropertiesTest.java:46) at org.apache.activemq.artemis.tests.integration.mqtt.MQTTInterceptorPropertiesTest.lambda$testCheckInterceptedMQTTMessageProperties$1(MQTTInterceptorPropertiesTest.java:84) Issue Time Tracking --- Worklog Id: (was: 871539) Time Spent: 40m (was: 0.5h) > MQTT retain flag not set correctly > -- > > Key: ARTEMIS-4365 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4365 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Justin Bertram >Assignee: Justin Bertram >Priority: Major > Time Spent: 40m > Remaining Estimate: 0h > > The {{retain}} flag set on MQTT messages dispatched to clients is incorrect > in certain circumstances. This is demonstrated by using the {{test}} command > from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.: > {noformat} > $ mqtt test --all > MQTT 3: OK > - Maximum topic length: 65535 bytes > - QoS 0: Received 10/10 publishes in 4847.01ms > - QoS 1: Received 10/10 publishes in 27413.45ms > - QoS 2: Received 10/10 publishes in 49551.40ms > - Retain: OK > - Wildcard subscriptions: OK > - Shared subscriptions: OK > - Payload size: >= 10 bytes > - Maximum client id length: 65535 bytes > - Unsupported Ascii Chars: ALL SUPPORTED > MQTT 5: OK > - Connect restrictions: > > Retain: OK > > Wildcard subscriptions: OK > > Shared subscriptions: OK > > Subscription identifiers: OK > > Maximum QoS: 2 > > Receive maximum: 65535 > > Maximum packet size: 268435455 bytes > > Topic alias maximum: 65535 > > Session expiry interval: Client-based > > Server keep alive: Client-based > - Maximum topic length: 65535 bytes > - QoS 0: Received 10/10 publishes in 706.21ms > - QoS 1: Received 10/10 publishes in 805.38ms > - QoS 2: Received 10/10 publishes in 972.98ms > - Retain: TIME_OUT > - Wildcard subscriptions: OK > - Shared subscriptions: OK > - Payload size: >= 10 bytes > - Maximum client id length: 65535 bytes > - Unsupported Ascii Chars: ALL SUPPORTED{noformat} > Notice the result of {{TIME_OUT}} when testing retain functionality for MQTT > 5. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly
[ https://issues.apache.org/jira/browse/ARTEMIS-4365?focusedWorklogId=871538=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871538 ] ASF GitHub Bot logged work on ARTEMIS-4365: --- Author: ASF GitHub Bot Created on: 18/Jul/23 12:31 Start Date: 18/Jul/23 12:31 Worklog Time Spent: 10m Work Description: gemmellr commented on code in PR #4556: URL: https://github.com/apache/activemq-artemis/pull/4556#discussion_r1266701120 ## tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MQTTTest.java: ## @@ -2219,4 +2223,71 @@ public void testAutoDeleteRetainedQueue() throws Exception { Wait.assertTrue(() -> server.locateQueue(RETAINED_QUEUE).getMessageCount() == 0, 3000, 50); Wait.assertTrue(() -> server.locateQueue(RETAINED_QUEUE) == null, 3000, 50); } + + /* +* [MQTT-3.3.1-9] When sending a PUBLISH Packet to a Client the Server...MUST set the RETAIN flag to 0 when a PUBLISH +* Packet is sent to a Client because it matches an *established* subscription regardless of how the flag was set in +* the message it received. +*/ + @Test(timeout = 60 * 1000) + public void testRetainFlagOnEstablishedSubscription() throws Exception { + CountDownLatch latch = new CountDownLatch(1); + final String topic = RandomUtil.randomString(); + + MqttClient subscriber = createPaho3_1_1Client("subscriber"); + subscriber.connect(); + subscriber.subscribe(topic, 1); + subscriber.setCallback(new DefaultMqtt3Callback() { + @Override + public void messageArrived(String topic, MqttMessage message) throws Exception { +if (!message.isRetained()) { + latch.countDown(); +} + } + }); + + MqttClient publisher = createPaho3_1_1Client("publisher"); + publisher.connect(); + publisher.publish(topic, "retained".getBytes(StandardCharsets.UTF_8), 1, true); + publisher.disconnect(); + publisher.close(); + + assertTrue(latch.await(1000, TimeUnit.MILLISECONDS)); + + subscriber.disconnect(); + subscriber.close(); + } + + /* +* [MQTT-3.3.1-8] When sending a PUBLISH Packet to a Client the Server MUST set the RETAIN flag to 1 if a message is +* sent as a result of a new subscription being made by a Client. +*/ + @Test(timeout = 60 * 1000) + public void testRetainFlagOnNewSubscription() throws Exception { + CountDownLatch latch = new CountDownLatch(1); + final String topic = RandomUtil.randomString(); + + MqttClient publisher = createPaho3_1_1Client("publisher"); + publisher.connect(); + publisher.publish(topic, "retained".getBytes(StandardCharsets.UTF_8), 1, true); + publisher.disconnect(); + publisher.close(); + + MqttClient subscriber = createPaho3_1_1Client("subscriber"); + subscriber.connect(); + subscriber.subscribe(topic, 1); + subscriber.setCallback(new DefaultMqtt3Callback() { + @Override + public void messageArrived(String topic, MqttMessage message) throws Exception { +if (message.isRetained()) { + latch.countDown(); +} + } + }); + + assertTrue(latch.await(1000, TimeUnit.MILLISECONDS)); Review Comment: I saw this test fail here on a CI run. Adding a failure message would be good. Its annoying having to go and match up line numbers in remote repositories to see what the failure actually meant. Issue Time Tracking --- Worklog Id: (was: 871538) Time Spent: 0.5h (was: 20m) > MQTT retain flag not set correctly > -- > > Key: ARTEMIS-4365 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4365 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Justin Bertram >Assignee: Justin Bertram >Priority: Major > Time Spent: 0.5h > Remaining Estimate: 0h > > The {{retain}} flag set on MQTT messages dispatched to clients is incorrect > in certain circumstances. This is demonstrated by using the {{test}} command > from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.: > {noformat} > $ mqtt test --all > MQTT 3: OK > - Maximum topic length: 65535 bytes > - QoS 0: Received 10/10 publishes in 4847.01ms > - QoS 1: Received 10/10 publishes in 27413.45ms > - QoS 2: Received 10/10 publishes in 49551.40ms > - Retain: OK > - Wildcard subscriptions: OK > - Shared subscriptions: OK > - Payload size: >= 10 bytes > - Maximum client id length: 65535 bytes > - Unsupported Ascii Chars: ALL SUPPORTED > MQTT 5: OK > - Connect restrictions: > > Retain: OK > > Wildcard subscriptions: OK >
[jira] [Work logged] (ARTEMIS-4365) MQTT retain flag not set correctly
[ https://issues.apache.org/jira/browse/ARTEMIS-4365?focusedWorklogId=871530=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871530 ] ASF GitHub Bot logged work on ARTEMIS-4365: --- Author: ASF GitHub Bot Created on: 18/Jul/23 11:25 Start Date: 18/Jul/23 11:25 Worklog Time Spent: 10m Work Description: gemmellr commented on code in PR #4556: URL: https://github.com/apache/activemq-artemis/pull/4556#discussion_r1266568647 ## artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTPublishManager.java: ## @@ -404,12 +406,17 @@ private boolean publishToClient(int messageId, ICoreMessage message, int deliver // [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.getBooleanProperty(MQTT_MESSAGE_RETAIN_KEY); + boolean isRetain = false; Review Comment: If it is primarily going to gate on MQTT_MESSAGE_RETAIN_INITIAL_DISTRIBUTION_KEY presence for both protocols as this does now, then could you not check for it once here and update isRetain based on it? Seems like it would be simpler than doing it 3 times below. Would allow dropping a couple else/else-if segments, leaving jsut the MQTT5-specific check on the MQTT_MESSAGE_RETAIN_KEY if it wasnt already true. ## tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/PublishTests.java: ## @@ -512,7 +540,8 @@ public void testRetainHandlingTwo() throws Exception { MqttClient consumer = createPahoClient(CONSUMER_ID); consumer.setCallback(new DefaultMqttCallback() { @Override - public void messageArrived(String topic, MqttMessage message) throws Exception { + public void messageArrived(String topic, MqttMessage message) { +assertTrue(message.isRetained()); Review Comment: Seems superfluous when the test is already burning 2 seconds (ouch) to verify this method isn't called. Though if it wasnt, also seems odd the assertion is the opposite of the previous test when it expected not to get the message. ## tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/PublishTests.java: ## @@ -481,7 +503,8 @@ public void messageArrived(String topic, MqttMessage message) throws Exception { final CountDownLatch latch2 = new CountDownLatch(1); consumer.setCallback(new DefaultMqttCallback() { @Override - public void messageArrived(String topic, MqttMessage message) throws Exception { + public void messageArrived(String topic, MqttMessage message) { +assertFalse(message.isRetained()); Review Comment: Seems superfluous when the test is already burning 2 seconds (ouch) to verify this method isn't called. ## tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/MQTTTest.java: ## @@ -2219,4 +2223,71 @@ public void testAutoDeleteRetainedQueue() throws Exception { Wait.assertTrue(() -> server.locateQueue(RETAINED_QUEUE).getMessageCount() == 0, 3000, 50); Wait.assertTrue(() -> server.locateQueue(RETAINED_QUEUE) == null, 3000, 50); } + + /* +* [MQTT-3.3.1-9] When sending a PUBLISH Packet to a Client the Server...MUST set the RETAIN flag to 0 when a PUBLISH +* Packet is sent to a Client because it matches an *established* subscription regardless of how the flag was set in +* the message it received. +*/ + @Test(timeout = 60 * 1000) + public void testRetainFlagOnEstablishedSubscription() throws Exception { + CountDownLatch latch = new CountDownLatch(1); + final String topic = RandomUtil.randomString(); Review Comment: Can be nice to name things based on the test to help easily see later it was for etc, useful for debugging. E.g use, or add a prefix via, getTopicName() / getQueueName() / getName() helpers. Same for next test. Issue Time Tracking --- Worklog Id: (was: 871530) Time Spent: 20m (was: 10m) > MQTT retain flag not set correctly > -- > > Key: ARTEMIS-4365 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4365 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Justin Bertram >Assignee: Justin Bertram >Priority: Major > Time Spent: 20m > Remaining Estimate: 0h > > The {{retain}} flag set on MQTT messages dispatched to clients is incorrect > in certain circumstances. This is demonstrated by using the {{test}} command > from the [MQTT CLI tool|https://hivemq.github.io/mqtt-cli/docs/test/], e.g.: >
[jira] [Work logged] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring
[ https://issues.apache.org/jira/browse/ARTEMIS-4366?focusedWorklogId=871515=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871515 ] ASF GitHub Bot logged work on ARTEMIS-4366: --- Author: ASF GitHub Bot Created on: 18/Jul/23 09:56 Start Date: 18/Jul/23 09:56 Worklog Time Spent: 10m Work Description: gemmellr commented on PR #4555: URL: https://github.com/apache/activemq-artemis/pull/4555#issuecomment-1639911264 Its odd/confusing that the ReferenceNodeStoreFactory is still named to in most parameters and fields as 'referenceIDSupplier' or 'nodeStore', especially the latter with there being an actual seperate ReferenceNodeStore. Its mixing roles and field/param names up in a confusing manner. Feels like the behaviours should be split into separate concerns, as they largely were originally. Alternatively many of the ID methods now on ReferenceNodeStoreFactory could perhaps just be made static helpers rather than being 'passed around on the side of a factory', since they only pull details from the message[reference] rather than the factory itself. Issue Time Tracking --- Worklog Id: (was: 871515) Time Spent: 40m (was: 0.5h) > Addresses with multiple subscriptions are not working with Mirroring > > > Key: ARTEMIS-4366 > URL: https://issues.apache.org/jira/browse/ARTEMIS-4366 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Clebert Suconic >Priority: Major > Time Spent: 40m > Remaining Estimate: 0h > > The NodeStore will be reused by the two Queues on the same address. Each > queue should have its own. -- This message was sent by Atlassian Jira (v8.20.10#820010)
[jira] [Work logged] (ARTEMIS-4366) Addresses with multiple subscriptions are not working with Mirroring
[ https://issues.apache.org/jira/browse/ARTEMIS-4366?focusedWorklogId=871512=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-871512 ] ASF GitHub Bot logged work on ARTEMIS-4366: --- Author: ASF GitHub Bot Created on: 18/Jul/23 09:48 Start Date: 18/Jul/23 09:48 Worklog Time Spent: 10m Work Description: gemmellr commented on code in PR #4555: URL: https://github.com/apache/activemq-artemis/pull/4555#discussion_r1266421706 ## artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/ProtonProtocolManager.java: ## @@ -75,7 +75,7 @@ public static String getMirrorAddress(String connectionName) { // We must use one referenceIDSupplier per server. // protocol manager is the perfect aggregation for that. - private ReferenceNodeStore referenceIDSupplier; + private ReferenceNodeStoreFactory referenceIDSupplier; Review Comment: Change makes the comment above seem inaccurate / contradictory (somewhat similar with the field name). There is now a factory being returned, and so the overall change here seems like it was to make sure there **is not** a single reference ID supplier per server as the comment says, but rather now one per-queue. ## artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/mirror/ReferenceNodeStoreFactory.java: ## @@ -0,0 +1,82 @@ +/* + * 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.protocol.amqp.connect.mirror; + +import org.apache.activemq.artemis.api.core.Message; +import org.apache.activemq.artemis.core.server.ActiveMQServer; +import org.apache.activemq.artemis.core.server.MessageReference; +import org.apache.activemq.artemis.utils.collections.NodeStore; +import org.apache.activemq.artemis.utils.collections.NodeStoreFactory; + +import static org.apache.activemq.artemis.protocol.amqp.connect.mirror.AMQPMirrorControllerSource.INTERNAL_BROKER_ID_EXTRA_PROPERTY; +import static org.apache.activemq.artemis.protocol.amqp.connect.mirror.AMQPMirrorControllerSource.INTERNAL_ID_EXTRA_PROPERTY; + +public class ReferenceNodeStoreFactory implements NodeStoreFactory { + + final ActiveMQServer server; + + private final String serverID; + + public ReferenceNodeStoreFactory(ActiveMQServer server) { + this.server = server; + this.serverID = server.getNodeID().toString(); + Review Comment: Superfluous newline ## artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/ProtonProtocolManager.java: ## @@ -125,11 +125,11 @@ public ProtonProtocolManager(ProtonProtocolManagerFactory factory, ActiveMQServe routingHandler = new AMQPRoutingHandler(server); } - public synchronized ReferenceNodeStore getReferenceIDSupplier() { + public synchronized ReferenceNodeStoreFactory getReferenceIDSupplier() { Review Comment: The method name also seems off now. ## artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/mirror/ReferenceNodeStoreFactory.java: ## @@ -0,0 +1,82 @@ +/* + * 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.protocol.amqp.connect.mirror; + +import org.apache.activemq.artemis.api.core.Message;