You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "jbertram (via GitHub)" <gi...@apache.org> on 2023/01/27 01:17:44 UTC

[GitHub] [activemq-artemis] jbertram opened a new pull request, #4346: ARTEMIS-4145 MQTT shared sub queue may be inadvertently removed

jbertram opened a new pull request, #4346:
URL: https://github.com/apache/activemq-artemis/pull/4346

   o.a.a.a.c.p.m.MQTTSubscriptionManager#removeSubscription() had a chunk of code from 971f673c602f859f342e22afe988c71687f754b6 removed. I couldn't determine why that bit of code was actually added in the first place, but the tests from that commit all still pass even with it removed now (as well as all the other MQTT tests) so I think it's safe.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] gemmellr commented on pull request #4346: ARTEMIS-4145 MQTT shared sub queue may be inadvertently removed

Posted by "gemmellr (via GitHub)" <gi...@apache.org>.
gemmellr commented on PR #4346:
URL: https://github.com/apache/activemq-artemis/pull/4346#issuecomment-1406313148

   The last point of the [original changes description](https://github.com/apache/activemq-artemis/pull/2466) seems to be about why that code was added:
   >3.  For the MQTT protocol, there is one and only one consumer connection per queue, which is a good choice for closing the old MQTT consumer before the new MQTT consumer connects.
   The original code could not effectively clean up the 'old consumer' in the queue when the 'new MQTT connection' was connected to the Artemis broker. Modify ‘MQTTSubscriptionManager.removeSubscription’ to get the queue consumer collection from the ‘QueueImpl’ instance and close them.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] clebertsuconic merged pull request #4346: ARTEMIS-4145 MQTT shared sub queue may be inadvertently removed

Posted by "clebertsuconic (via GitHub)" <gi...@apache.org>.
clebertsuconic merged PR #4346:
URL: https://github.com/apache/activemq-artemis/pull/4346


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] jbertram commented on pull request #4346: ARTEMIS-4145 MQTT shared sub queue may be inadvertently removed

Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #4346:
URL: https://github.com/apache/activemq-artemis/pull/4346#issuecomment-1406631399

   @gemmellr, thanks for the clarification. I'll update the commit message based on this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org