[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=212007=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-212007 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 12/Mar/19 20:23 Start Date: 12/Mar/19 20:23 Worklog Time Spent: 10m Work Description: clebertsuconic commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-472166231 @michaelandrepearce the commit message should have been edited before merged. no big deal.. just mentioning it. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 212007) Time Spent: 22h 40m (was: 22.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 22h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=211994=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-211994 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 12/Mar/19 19:57 Start Date: 12/Mar/19 19:57 Worklog Time Spent: 10m Work Description: asfgit commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 211994) Time Spent: 22.5h (was: 22h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 22.5h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=202203=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-202203 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 21/Feb/19 20:12 Start Date: 21/Feb/19 20:12 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-466148548 @jbertram @clebertsuconic and anyone else. As no one else is commenting /reviewing, and to me this looks ok, Ill look to merge this in a few days if no further review comment from anyone. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 202203) Time Spent: 22h 20m (was: 22h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 22h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=199081=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-199081 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 15/Feb/19 02:34 Start Date: 15/Feb/19 02:34 Worklog Time Spent: 10m Work Description: onlyMIT commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-463883656 @michaelandrepearce Ok, thank you very much for your help. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 199081) Time Spent: 22h 10m (was: 22h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 22h 10m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=199068=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-199068 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 15/Feb/19 01:32 Start Date: 15/Feb/19 01:32 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-463871181 @onlyMIT i would like some of others in the group to review this as well, as there changes to clustering and also to the MQTT protocol behaviour. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 199068) Time Spent: 21h 40m (was: 21.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 21h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=199070=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-199070 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 15/Feb/19 01:34 Start Date: 15/Feb/19 01:34 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-463871181 @onlyMIT i would like some of others in the group to review this as well, as there changes to clustering and also to the MQTT protocol behaviour here, that are not opt in or out on for end users by config. E.g. maybe there is some use cases where the current behaviour is being relied on else where im not aware of. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 199070) Time Spent: 22h (was: 21h 50m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 22h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=199069=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-199069 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 15/Feb/19 01:33 Start Date: 15/Feb/19 01:33 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-463871181 @onlyMIT i would like some of others in the group to review this as well, as there changes to clustering and also to the MQTT protocol behaviour here, that are not opt in or out on. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 199069) Time Spent: 21h 50m (was: 21h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 21h 50m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=199055=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-199055 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 15/Feb/19 01:29 Start Date: 15/Feb/19 01:29 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-463871181 @onlyMIT i would like some of others in the group to review this as well. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 199055) Time Spent: 21.5h (was: 21h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 21.5h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=198022=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-198022 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 13/Feb/19 11:07 Start Date: 13/Feb/19 11:07 Worklog Time Spent: 10m Work Description: onlyMIT commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-463157879 Add java doc This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 198022) Time Spent: 21h 20m (was: 21h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 21h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=197625=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-197625 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 12/Feb/19 14:47 Start Date: 12/Feb/19 14:47 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r255985412 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionBridge.java ## @@ -351,10 +355,15 @@ private String appendIgnoresToFilter(String filterString) { } filterString += "!" + storeAndForwardPrefix; filterString += ",!" + managementAddress; - filterString += ",!" + managementNotificationAddress; return filterString; } + private String createPermissiveManagementNotificationToFilter() { + StringBuilder filterBuilder = new StringBuilder(ManagementHelper.HDR_NOTIFICATION_TYPE).append(" = '") Review comment: Although it is possible to add a dedicated address to the SESSION_CREATED notification when the cluster-connection is created, it is not necessary to do so. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 197625) Time Spent: 21h 10m (was: 21h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 21h 10m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=197622=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-197622 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 12/Feb/19 14:43 Start Date: 12/Feb/19 14:43 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r255983228 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionBridge.java ## @@ -351,10 +355,15 @@ private String appendIgnoresToFilter(String filterString) { } filterString += "!" + storeAndForwardPrefix; filterString += ",!" + managementAddress; - filterString += ",!" + managementNotificationAddress; return filterString; } + private String createPermissiveManagementNotificationToFilter() { + StringBuilder filterBuilder = new StringBuilder(ManagementHelper.HDR_NOTIFICATION_TYPE).append(" = '") Review comment: The inter-cluster notification message needs to specify the routing address to route the message. Different from the notifications of CONSUMER_CREATE and BINDING_ADD, the routing address (the address created or consumed by itself) has been determined before the notification is sent. When sending a SESSION_CREATED notification, We can't know which address to use, so I chose to use the `ManagementManagementAddress`, which is consumed by each node in the cluster-connection. But the notification message using the `ManagementNotificationAddress` address is filtered by the filter, so I modified the filter and allow SESSION_CREATED notification to be routed to other nodes in the cluster. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 197622) Time Spent: 21h (was: 20h 50m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 21h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=197567=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-197567 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 12/Feb/19 13:52 Start Date: 12/Feb/19 13:52 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r255960926 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java ## @@ -2954,7 +2955,13 @@ public void onNotification(org.apache.activemq.artemis.core.server.management.No if (!(notification.getType() instanceof CoreNotificationType)) return; CoreNotificationType type = (CoreNotificationType) notification.getType(); - TypedProperties prop = notification.getProperties(); + if (type == CoreNotificationType.SESSION_CREATED) { + TypedProperties props = notification.getProperties(); + //SESSION_CREATED notification should not be broadcast twice + if (props.getIntProperty(ManagementHelper.HDR_DISTANCE) > 0) { Review comment: To avoid calling the broadcast twice.The original logical SESSION_CREATED notification will not be transmitted between cluster nodes, and it is now required to transfer between cluster nodes. In order to maintain consistency with the original logic, this code to prevent the node receiving the SESSION_CREATED notification from making a broadcast call again. Keep the original logic for other notification messages not judged This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 197567) Time Spent: 20h 50m (was: 20h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 20h 50m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=197512=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-197512 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 12/Feb/19 11:56 Start Date: 12/Feb/19 11:56 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r255920956 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java ## @@ -2954,7 +2955,13 @@ public void onNotification(org.apache.activemq.artemis.core.server.management.No if (!(notification.getType() instanceof CoreNotificationType)) return; CoreNotificationType type = (CoreNotificationType) notification.getType(); - TypedProperties prop = notification.getProperties(); + if (type == CoreNotificationType.SESSION_CREATED) { + TypedProperties props = notification.getProperties(); + //SESSION_CREATED notification should not be broadcast twice + if (props.getIntProperty(ManagementHelper.HDR_DISTANCE) > 0) { Review comment: Also isnt cyclic between nodes in cluster already avoided, because of filter in the cluster connection bridge where it does a HDR_DISTANCE < flow.maxHops. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 197512) Time Spent: 20h 40m (was: 20.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 20h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=197510=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-197510 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 12/Feb/19 11:54 Start Date: 12/Feb/19 11:54 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-462731246 @onlyMIT looking better, i have some questions, which im sure all makes sense if explained, but probably a sign a little java doc might be beneficial, and would help future understanding when this PR is closed. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 197510) Time Spent: 20.5h (was: 20h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 20.5h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=197509=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-197509 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 12/Feb/19 11:47 Start Date: 12/Feb/19 11:47 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r255918235 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionBridge.java ## @@ -351,10 +355,15 @@ private String appendIgnoresToFilter(String filterString) { } filterString += "!" + storeAndForwardPrefix; filterString += ",!" + managementAddress; - filterString += ",!" + managementNotificationAddress; return filterString; } + private String createPermissiveManagementNotificationToFilter() { + StringBuilder filterBuilder = new StringBuilder(ManagementHelper.HDR_NOTIFICATION_TYPE).append(" = '") Review comment: Whats this trying to achieve? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 197509) Time Spent: 20h 20m (was: 20h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 20h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=197504=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-197504 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 12/Feb/19 11:43 Start Date: 12/Feb/19 11:43 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r255916966 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java ## @@ -2954,7 +2955,13 @@ public void onNotification(org.apache.activemq.artemis.core.server.management.No if (!(notification.getType() instanceof CoreNotificationType)) return; CoreNotificationType type = (CoreNotificationType) notification.getType(); - TypedProperties prop = notification.getProperties(); + if (type == CoreNotificationType.SESSION_CREATED) { + TypedProperties props = notification.getProperties(); + //SESSION_CREATED notification should not be broadcast twice + if (props.getIntProperty(ManagementHelper.HDR_DISTANCE) > 0) { Review comment: Why would this logic specific to just SESSION_CREATED, would this not be generic to all? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 197504) Time Spent: 20h 10m (was: 20h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 20h 10m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=197434=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-197434 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 12/Feb/19 08:50 Start Date: 12/Feb/19 08:50 Worklog Time Spent: 10m Work Description: onlyMIT commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-462671664 @michaelandrepearce move logic to SESSION_CREATE notification This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 197434) Time Spent: 20h (was: 19h 50m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 20h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=197330=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-197330 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 12/Feb/19 02:32 Start Date: 12/Feb/19 02:32 Worklog Time Spent: 10m Work Description: onlyMIT commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-462589647 @michaelandrepearce I was on vacation last week, started this week. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 197330) Time Spent: 19h 50m (was: 19h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 19h 50m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=197258=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-197258 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 11/Feb/19 23:39 Start Date: 11/Feb/19 23:39 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-462540225 As noted i sent an unpolished pr to your branch last week, to enhance and use SESSION_CREATED As noted connected == create session in core, so im against constructing some new terminology within core notifications, here. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 197258) Time Spent: 19h 40m (was: 19.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 19h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=195928=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-195928 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 07/Feb/19 20:59 Start Date: 07/Feb/19 20:59 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-460700605 First of all mqtt connect = create session in core. I dont want to introduce anything thats protocol specific. Nor introduce something that is already represented. Im happy with current create session being a top level supported notification (eg move it from plugin and have it in session like consumer notification is) as this is agnostic and will cause little extra traffic compared with create consumer which already is. And im happy with extra data being added to this notification. i sent a pr to your branch as a sort of example (needs polish) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 195928) Time Spent: 19.5h (was: 19h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 19.5h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194645=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194645 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 05/Feb/19 16:24 Start Date: 05/Feb/19 16:24 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-460702843 I am off now for 48 hours (pto). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194645) Time Spent: 19h 10m (was: 19h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 19h 10m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194644=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194644 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 05/Feb/19 16:22 Start Date: 05/Feb/19 16:22 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-460700605 First of all mqtt connect = create session in core. I dont want to introduce anything thats protocol specific. Nor introduce something that is already represented. Im happy with current create session being a top level supported notification as this is agnostic and will cause little extra traffic compared with create consumer which already is. And im happy with extra data being added to this notification. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194644) Time Spent: 19h (was: 18h 50m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 19h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194647=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194647 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 05/Feb/19 16:27 Start Date: 05/Feb/19 16:27 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-460700605 First of all mqtt connect = create session in core. I dont want to introduce anything thats protocol specific. Nor introduce something that is already represented. Im happy with current create session being a top level supported notification (eg move it from plugin and have it in session like consumer notification is) as this is agnostic and will cause little extra traffic compared with create consumer which already is. And im happy with extra data being added to this notification. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194647) Time Spent: 19h 20m (was: 19h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 19h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194643=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194643 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 05/Feb/19 16:19 Start Date: 05/Feb/19 16:19 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-460700605 First of all mqtt connect = create session in core. I dont want to introduce anything thats protocol specific. Im happy with current create session being a top level supported notification as this is agnostic and will cause little extra traffic compared with create consumer which already is. And im happy with extra data being added to this notification This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194643) Time Spent: 18h 50m (was: 18h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 18h 50m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194642=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194642 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 05/Feb/19 16:18 Start Date: 05/Feb/19 16:18 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-460700605 First of all mqtt connect = create session in core. I dont want to introduce anything thats protocol specific. Im happy with current create session being a top level supported notification as this is agnostic and will cause little extra traffic compared with create consumer which already is. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194642) Time Spent: 18h 40m (was: 18.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 18h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194632=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194632 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 05/Feb/19 16:01 Start Date: 05/Feb/19 16:01 Worklog Time Spent: 10m Work Description: onlyMIT commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-460693569 Now let's solve it step by step. First, we need to determine whether to use the existing notification or add a new notification to handle this logic. as I comment, it seems that the existing notification is not suitable. In the second step, if we use the new notification, we need to determine how to make it more reasonable. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194632) Time Spent: 18.5h (was: 18h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 18.5h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194631=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194631 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 05/Feb/19 16:00 Start Date: 05/Feb/19 16:00 Worklog Time Spent: 10m Work Description: onlyMIT commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-460693569 Now let's solve it step by step. First, we need to determine whether to use the existing notification or add a new notification to handle this logic. If I comment, it seems that the existing notification is not suitable. In the second step, if we use the new notification, we need to determine how to make it more reasonable. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194631) Time Spent: 18h 20m (was: 18h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 18h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194622=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194622 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 05/Feb/19 15:54 Start Date: 05/Feb/19 15:54 Worklog Time Spent: 10m Work Description: onlyMIT commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-460691058 @michaelandrepearce As described in my comment.Even if use a plugin, we can't use notifications in the plugin like SESSION_CREATED,it can't achieve our goal, modify the SESSION_CREATED filter? I think we need to think carefully, and if we modify their filtering rules, we can't affect the existing logic. We need to conduct comprehensive testing and evaluation.I think the plugin should be where the optional code is placed, and the logic that is must required is not suitable for the plugin. Like the first version I submitted, the logic was placed in CONSUMER_CREATE, the readability was too poor and not exactly matched with the MQTT specification; This version the logic was added by adding CONNECTED_CREATED. It seems that not all protocols are used, and it is not the most Good plan. If you want to use SESSION_CREATED, CONNECTION_CREATED, must force the MQTT protocol to open all plugins? and modify the filter so that the SESSION_CREATED notification can communicate between the clusters. This is the risk of making the original notification rules undefined,and need to modify their filtering rules, I am not sure if modifying the filtering rules will introduce other issues. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194622) Time Spent: 18h 10m (was: 18h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 18h 10m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194584=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194584 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 05/Feb/19 15:27 Start Date: 05/Feb/19 15:27 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-460672007 As ive already said you have two options either make mqtt to ensure plugin is enabled or simply make this notification more perm as like consumer one is. Also in that would be adding that session created is passed through This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194584) Time Spent: 18h (was: 17h 50m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 18h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194576=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194576 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 05/Feb/19 15:08 Start Date: 05/Feb/19 15:08 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-460672007 As ive already said you have two options either make mqtt to ensure plugin is enabled or simply make this notification more perm as like consumer one is. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194576) Time Spent: 17h 50m (was: 17h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 17h 50m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194575=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194575 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 05/Feb/19 15:07 Start Date: 05/Feb/19 15:07 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-460672007 As ive already said you have two options either make mqtt to ensure plugin js enabled or simply make this notification more perm as like consumer one is. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194575) Time Spent: 17h 40m (was: 17.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 17h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194527=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194527 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 05/Feb/19 14:16 Start Date: 05/Feb/19 14:16 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253880048 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: ``` if (hasBrokerSessionPlugins()) { callBrokerSessionPlugins(plugin -> plugin.afterCreateSession(session)); } ``` SESSION_CREATED will only be sent if BrokerSessionPlugins is turned on in the configuration file. Also the notifications of CONNECTION_CREATE and SESSIONTION_CREATE will be filtered and will not be sent to other nodes in the cluster.You can see `ClusterConnectionBridge.setupNotificationConsumer()` method, where the cluster-connection filter is set . This is why I want to modify the filtering rules, CONNECTION_CREATE, SESSIONTION_CREATE and other connection-related notifications, only can use the `ManagementNotificationAddress` address to route messages. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194527) Time Spent: 17h 20m (was: 17h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 17h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194532=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194532 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 05/Feb/19 14:22 Start Date: 05/Feb/19 14:22 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253882381 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: ``` private void handleNotificationMessage(ClientMessage message, CoreNotificationType ntype) throws Exception { // TODO - optimised this by just passing int in header - but filter needs to be extended to support IN with switch (ntype) { case BINDING_ADDED: { doBindingAdded(message); break; } case BINDING_REMOVED: { doBindingRemoved(message); break; } case CONSUMER_CREATED: { doConsumerCreated(message); break; } case CONSUMER_CLOSED: { doConsumerClosed(message); break; } case PROPOSAL: { doProposalReceived(message); break; } case PROPOSAL_RESPONSE: { doProposalResponseReceived(message); break; } case UNPROPOSAL: { doUnProposalReceived(message); break; } default: { throw ActiveMQMessageBundle.BUNDLE.invalidType(ntype); } } } ``` Only these notifications will pass between cluster nodes This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194532) Time Spent: 17.5h (was: 17h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 17.5h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194106=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194106 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 04/Feb/19 16:38 Start Date: 04/Feb/19 16:38 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253543767 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: re > The clientId is read from the client's connection message. The logic for reading the clientId cannot be placed in the CORE code, when create a connection or create a session. during handling connect packet in MQTTConnectionManager, the clientid is set on the connection BEFORE createServerSession as such it will be available when the SESSION_CREATED notification is created, MQTTConnectionManager lines 78, 79 ``` session.getConnection().setClientID(clientId); ServerSessionImpl serverSession = createServerSession(username, password); ``` just need to populate the extra field which would be available at the time via ``` session.getRemotingConnection().getClientID() . ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194106) Time Spent: 17h (was: 16h 50m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 17h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194102=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194102 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 04/Feb/19 16:36 Start Date: 04/Feb/19 16:36 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253543767 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: re "The clientId is read from the client's connection message. The logic for reading the clientId cannot be placed in the CORE code, when create a connection or create a session." the clientid is set on the connection BEFORE createServerSession as such it will be available when the SESSION_CREATED notification is created, MQTTConnectionManager lines 78, 79 ``` session.getConnection().setClientID(clientId); ServerSessionImpl serverSession = createServerSession(username, password); ``` just need to populate the extra field which would be available at the time via ``` session.getRemotingConnection().getClientID() . ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194102) Time Spent: 16h 40m (was: 16.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 16h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194098=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194098 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 04/Feb/19 16:35 Start Date: 04/Feb/19 16:35 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253543767 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: re "The clientId is read from the client's connection message. The logic for reading the clientId cannot be placed in the CORE code, when create a connection or create a session." the clientid is set on the connection BEFORE createServerSession as such it will be available when the SESSION_CREATED notification is created, MQTTConnectionManager lines 78, 79 ``` session.getConnection().setClientID(clientId); ServerSessionImpl serverSession = createServerSession(username, password); ``` just need to populate the extra field which would be available at the time via session.getRemotingConnection().getClientID() . This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194098) Time Spent: 16.5h (was: 16h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 16.5h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194107=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194107 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 04/Feb/19 16:38 Start Date: 04/Feb/19 16:38 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253543767 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: re > The clientId is read from the client's connection message. The logic for reading the clientId cannot be placed in the CORE code, when create a connection or create a session. during handling connect packet in MQTTConnectionManager, the clientid is set on the connection BEFORE createServerSession as such it will be available when the SESSION_CREATED notification is created, MQTTConnectionManager lines 78, 79 ``` session.getConnection().setClientID(clientId); ServerSessionImpl serverSession = createServerSession(username, password); ``` just need to populate the extra field in the SESSION_CREATED which would be available at the time via ``` session.getRemotingConnection().getClientID() . ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194107) Time Spent: 17h 10m (was: 17h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 17h 10m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194104=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194104 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 04/Feb/19 16:36 Start Date: 04/Feb/19 16:36 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253543767 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: re > The clientId is read from the client's connection message. The logic for reading the clientId cannot be placed in the CORE code, when create a connection or create a session. the clientid is set on the connection BEFORE createServerSession as such it will be available when the SESSION_CREATED notification is created, MQTTConnectionManager lines 78, 79 ``` session.getConnection().setClientID(clientId); ServerSessionImpl serverSession = createServerSession(username, password); ``` just need to populate the extra field which would be available at the time via ``` session.getRemotingConnection().getClientID() . ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194104) Time Spent: 16h 50m (was: 16h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 16h 50m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194027=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194027 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 04/Feb/19 14:36 Start Date: 04/Feb/19 14:36 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253492310 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: I am here to make a unified reply to your comments. In fact, this is only the mqtt protocol need, other protocols do not.Either way, it doesn't look too good in CORE, it just tries to make it look reasonable and more readable. ``` /** * Called when all connection related operations are completed. */ default void connected() throws Exception { ``` I use `connected` as default not a interface method,Because we need to send cluster-notifation when the mqtt protocol connection is initialized, But other protocols don't need this method to do anything, use a empty default methods I think is appropriate.And all protocol connections have this connection completion status, and other protocols have the possibility to use this method. As for the logic that is only needed for MQTT to add a listener or callback, I think it is over-designed. I also find SESSION_CREATED and CONNECTION_CREATED, but i can not use them, because we need clientId, and clientId read from client message. The clientId is read from the client's connection message. The logic for reading the clientId cannot be placed in the CORE code, when create a connection or create a session. For using callbacks or listeners, I still prefer to use the default method `connected`. Unless you have a better design to let me change my mind. the clientId is read from the client message, it seems that the message can only be sent after the protocol connection has been processed. ( MQTT protocol neet to process the CONNECT request packet, some protocols do not have a CONNECT request packet) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194027) Time Spent: 16h 20m (was: 16h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 16h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=194026=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-194026 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 04/Feb/19 14:34 Start Date: 04/Feb/19 14:34 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253492310 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: I am here to make a unified reply to your comments. In fact, this is only the mqtt protocol need, other protocols do not.Either way, it doesn't look too good in CORE, it just tries to make it look reasonable and more readable. ``` /** * Called when all connection related operations are completed. */ default void connected() throws Exception { ``` I use `connected` as default not a interface method,Because we need to send cluster-notifation when the mqtt protocol connection is initialized, But other protocols don't need this method to do anything, use a empty default methods I think is appropriate.And all protocol connections have this connection completion status, and other protocols have the possibility to use this method. As for the logic that is only needed for MQTT to add a listener or callback, I think it is over-designed. I also find SESSION_CREATED and CONNECTION_CREATED, but i can not use them, because we need clientId, and clientId read from client message. The clientId is read from the client's connection message. The logic for reading the clientId cannot be placed in the CORE code, when create a connection or create a session. For using callbacks or listeners, I still prefer to use the default method connected. Unless you have a better design to let me change my mind. the clientId is read from the client message, it seems that the message can only be sent after the protocol connection has been processed. ( MQTT protocol neet to process the CONNECT request packet, some protocols do not have a CONNECT request packet) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 194026) Time Spent: 16h 10m (was: 16h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 16h 10m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193833=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193833 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 21:35 Start Date: 03/Feb/19 21:35 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253319407 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: Going through and running this, and running with notification plugin, i notice every time your new CONNECTION_CONNECTED occurs, a SESSION_CREATED is also sent. digging deeper and looking going through the logic of the connect in MQTTConnectionManager it seems this actually connected in the MQTTConnectionManager maps over to CORE as a server session (e.g. connect in mqtt protocol parlance == in core create a serversession), which means we already get a SESSION_CREATED notification with that plugin, so is it actually we want to ensure SESSION_CREATED notifications are not reliant on plugin enabled, e.g. bring that from the plugin (either moving code into ActiveMQServer and invoke always this notification like with consumers, or another approach could be, MQTT ensuring that the plugin configured/ enabled and the notification needed in the plugin is enabled). populating and then just enhance that notification with e.g. session.getRemotingConnection().getClientID() This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193833) Time Spent: 16h (was: 15h 50m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 16h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193832=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193832 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 21:34 Start Date: 03/Feb/19 21:34 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253319407 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: Going through and running this, and running with notification plugin, i notice every time your new CONNECTION_CONNECTED occurs, a SESSION_CREATED is also sent. digging deeper and looking going through the logic of the connect in MQTTConnectionManager it seems this actually connected in the MQTTConnectionManager maps over to CORE as a server session (e.g. connect in mqtt protocol parlance == in core create a serversession), which means we already get a SESSION_CREATED notification with that plugin, so is it actually we want to ensure SESSION_CREATED notificaions are not reliant on plugin enabled, e.g. bring that from the plugin (either moving code into core, or MQTT ensuring that the plugin is enabled and the notification needed is enabled). populating and then just enhance that notification with e.g. session.getRemotingConnection().getClientID() This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193832) Time Spent: 15h 50m (was: 15h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 15h 50m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193822=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193822 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 21:05 Start Date: 03/Feb/19 21:05 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253318691 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnection.java ## @@ -216,8 +220,10 @@ public void flush() { public void bufferReceived(Object connectionID, ActiveMQBuffer buffer) { } - public void setConnected(boolean connected) { - this.connected = connected; + @Override + public void connected() throws Exception { + remotingService.sendConnectedNotification(this); Review comment: This should be responsibility of core system to sending notification after all connection behaviours are completed, so connected notification is generic. Eg. should be as like the current logic in plugin almost notifications at this level shouldn't be protocol specific, and like other notifications, they're not specific for a single protocol. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193822) Time Spent: 14h 50m (was: 14h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 14h 50m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193827=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193827 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 21:12 Start Date: 03/Feb/19 21:12 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253319407 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: Going through and running this, and running with notification plugin, i notice every time your new CONNECTION_CONNECTED occurs, a SESSION_CREATED is also sent. digging deeper and looking going through the logic of the connect in MQTTConnectionManager it seems this actually connected in the MQTTConnectionManager maps over to CORE as a server session (e.g. connect in mqtt protocol parlance == in core create a serversession), which means we already get a SESSION_CREATED notification with that plugin, so is it actually we want to ensure SESSION_CREATED notificaions are not reliant on plugin enabled, e.g. bring that from the plugin. populating. session.getRemotingConnection().getClientID() This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193827) Time Spent: 15h 40m (was: 15.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 15h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193826=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193826 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 21:09 Start Date: 03/Feb/19 21:09 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253319407 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: Going through and running this, and running with notification plugin, i notice every time your new CONNECTION_CONNECTED occurs, a SESSION_CREATED is also sent. digging deeper and looking going through the logic of the connect in MQTTConnectionManager it seems this actually connected in the MQTTConnectionManager maps over to CORE as a server session (e.g. connect in mqtt protocol parlance == in core create a serversession), which means we already get a SESSION_CREATED notification with that plugin, so is it actually we want to ensure SESSION_CREATED notificaions are not reliant on plugin, e.g. bring that in... populating. session.getRemotingConnection().getClientID() This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193826) Time Spent: 15.5h (was: 15h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 15.5h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193825=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193825 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 21:08 Start Date: 03/Feb/19 21:08 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253319564 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: So this ties in with the fact you want to ensure a single connected session (its not actually at connection level, its actually at the session level), and need to kill off any other sessions (and their associated connection). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193825) Time Spent: 15h 20m (was: 15h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 15h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193824=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193824 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 21:07 Start Date: 03/Feb/19 21:07 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253319564 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: So this ties in with the fact you want to ensure a single connected session (its not actually at connection level, its actually at the session level). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193824) Time Spent: 15h 10m (was: 15h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 15h 10m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193823=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193823 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 21:06 Start Date: 03/Feb/19 21:06 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253319511 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnection.java ## @@ -216,8 +220,10 @@ public void flush() { public void bufferReceived(Object connectionID, ActiveMQBuffer buffer) { } - public void setConnected(boolean connected) { - this.connected = connected; + @Override + public void connected() throws Exception { + remotingService.sendConnectedNotification(this); Review comment: see other comments, ive dug deeper on this and seems actually the act of mqtt connected == core server session created. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193823) Time Spent: 15h (was: 14h 50m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 15h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193821=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193821 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 21:05 Start Date: 03/Feb/19 21:05 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253319407 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: Going through and running this, and running with notification plugin, i notice every time this occurs, a SESSION_CREATED is also sent. digging deeper and looking going through the logic of the connect in MQTTConnectionManager it seems this actually connected in the MQTTConnectionManager maps over to CORE as a server session (e.g. connect in mqtt protocol parlance == in core create a serversession), which means we already get a SESSION_CREATED notification with that plugin, so is it actually we want to ensure SESSION_CREATED notificaions are not reliant on plugin, e.g. bring that in... populating. session.getRemotingConnection().getClientID() This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193821) Time Spent: 14h 40m (was: 14.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 14h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193820=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193820 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 21:04 Start Date: 03/Feb/19 21:04 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253319407 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: Going through and running this, and running with notification plugin, i notice every time this occurs, a SESSION_CREATED is also sent. digging deeper and looking going through the logic of the connect in MQTTConnectionManager it seems this actually connected in the MQTTConnectionManager maps over to CORE as a server session (e.g. connect == create a serversession), which means we already get a SESSION_CREATED notification with that plugin, so is it actually we want to ensure SESSION_CREATED notificaions are not reliant on plugin, e.g. bring that in... populating. session.getRemotingConnection().getClientID() This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193820) Time Spent: 14.5h (was: 14h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 14.5h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193819=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193819 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 20:54 Start Date: 03/Feb/19 20:54 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253318957 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: because this is in core, this must be implemented in a generic way to trigger for ALL protocols. It can't just be for MQTT This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193819) Time Spent: 14h 20m (was: 14h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 14h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193817=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193817 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 20:53 Start Date: 03/Feb/19 20:53 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253318957 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: because this is in core, this must be implemented in a generic way for ALL protocols. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193817) Time Spent: 14h (was: 13h 50m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 14h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193818=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193818 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 20:53 Start Date: 03/Feb/19 20:53 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253318957 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/CoreNotificationType.java ## @@ -47,7 +47,8 @@ SESSION_CREATED(26), SESSION_CLOSED(27), MESSAGE_DELIVERED(28), - MESSAGE_EXPIRED(29); + MESSAGE_EXPIRED(29), + CONNECTION_CONNECTED(30); Review comment: because this is in core, this must be implemented in a generic way to trigger for ALL protocols. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193818) Time Spent: 14h 10m (was: 14h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 14h 10m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193816=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193816 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 20:50 Start Date: 03/Feb/19 20:50 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253318691 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnection.java ## @@ -216,8 +220,10 @@ public void flush() { public void bufferReceived(Object connectionID, ActiveMQBuffer buffer) { } - public void setConnected(boolean connected) { - this.connected = connected; + @Override + public void connected() throws Exception { + remotingService.sendConnectedNotification(this); Review comment: This should be responsibility of core system to sending notification after all connection behaviours are completed, so connected notification is generic. Eg. should be as like the current logic in plugin almost notifications at this level shouldn't be protocol specific, and like other notifications, they're not specific for a single protocol. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193816) Time Spent: 13h 50m (was: 13h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 13h 50m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193815=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193815 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 20:49 Start Date: 03/Feb/19 20:49 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253318691 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnection.java ## @@ -216,8 +220,10 @@ public void flush() { public void bufferReceived(Object connectionID, ActiveMQBuffer buffer) { } - public void setConnected(boolean connected) { - this.connected = connected; + @Override + public void connected() throws Exception { + remotingService.sendConnectedNotification(this); Review comment: This should be responsibility of core system to sending notification so connected notification is generic. Eg. should be as like the current logic in plugin almost notifications at this level shouldn't be protocol specific. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193815) Time Spent: 13h 40m (was: 13.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 13h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193814=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193814 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 20:48 Start Date: 03/Feb/19 20:48 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253318691 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnection.java ## @@ -216,8 +220,10 @@ public void flush() { public void bufferReceived(Object connectionID, ActiveMQBuffer buffer) { } - public void setConnected(boolean connected) { - this.connected = connected; + @Override + public void connected() throws Exception { + remotingService.sendConnectedNotification(this); Review comment: This should be responsibility of core system to sending notification so connected notification is generic. Eg. should be as like the current logic in plugin almost This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193814) Time Spent: 13.5h (was: 13h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 13.5h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193813=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193813 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 20:47 Start Date: 03/Feb/19 20:47 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253318691 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnection.java ## @@ -216,8 +220,10 @@ public void flush() { public void bufferReceived(Object connectionID, ActiveMQBuffer buffer) { } - public void setConnected(boolean connected) { - this.connected = connected; + @Override + public void connected() throws Exception { + remotingService.sendConnectedNotification(this); Review comment: This should be responsibility of core system to sending notification so connected notification is generic. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193813) Time Spent: 13h 20m (was: 13h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 13h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193812=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193812 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 20:46 Start Date: 03/Feb/19 20:46 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253318615 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/spi/core/protocol/RemotingConnection.java ## @@ -245,6 +245,13 @@ */ String getTransportLocalAddress(); + /** +* Called when all connection related operations are completed. +*/ + default void connected() throws Exception { Review comment: As this introduces a notification about connections created, should ensure that this is always called, and not dependent on a protocol logic, e.g. present a callback to a protocol which could choose to do something, and then after it produces a connection connected notification This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193812) Time Spent: 13h 10m (was: 13h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 13h 10m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193811=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193811 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 03/Feb/19 20:45 Start Date: 03/Feb/19 20:45 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253318615 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/spi/core/protocol/RemotingConnection.java ## @@ -245,6 +245,13 @@ */ String getTransportLocalAddress(); + /** +* Called when all connection related operations are completed. +*/ + default void connected() throws Exception { Review comment: As this introduces a notification about connections created, should ensure that this is always called, and not dependent on a protocol logic. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193811) Time Spent: 13h (was: 12h 50m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 13h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193682=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193682 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 02/Feb/19 15:17 Start Date: 02/Feb/19 15:17 Worklog Time Spent: 10m Work Description: onlyMIT commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-459972986 @michaelandrepearce I have addressed the comments your sent a pr to my branch. Change the clusterConsumer queue’s filter. Because it will filter CONNECTION_CONNECTED notification. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193682) Time Spent: 12h 50m (was: 12h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 12h 50m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193451=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193451 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 16:40 Start Date: 01/Feb/19 16:40 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253117361 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTConnectionManager.java ## @@ -163,13 +163,7 @@ synchronized void disconnect(boolean failure) { private MQTTSessionState getSessionState(String clientId) { /* [MQTT-3.1.2-4] Attach an existing session if one exists otherwise create a new one. */ - MQTTSessionState state = MQTTSession.SESSIONS.get(clientId); - if (state == null) { - state = new MQTTSessionState(clientId); - MQTTSession.SESSIONS.put(clientId, state); - } - - return state; + return MQTTSession.getSessionState(clientId, session.getServer().getIdentity()); Review comment: -1 Ive raised this already, simply shouldnt be having static map holding state. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193451) Time Spent: 12h 40m (was: 12.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 12h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193435=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193435 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 16:09 Start Date: 01/Feb/19 16:09 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253105287 ## File path: artemis-core-client/src/main/java/org/apache/activemq/artemis/core/server/management/NotificationService.java ## @@ -37,4 +37,8 @@ void removeNotificationListener(NotificationListener listener); + default void callNotificationListener(Notification notification) { Review comment: This is purpose of sendNotification, any new thing should integrate with the existing way things work. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193435) Time Spent: 12h 20m (was: 12h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 12h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193437=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193437 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 16:11 Start Date: 01/Feb/19 16:11 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253105898 ## File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/management/impl/ManagementServiceImpl.java ## @@ -633,6 +633,20 @@ public boolean isStarted() { return started; } + @Override + public void callNotificationListener(Notification notification) { Review comment: This is repeating logic of sendNotification. Please just use that. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193437) Time Spent: 12.5h (was: 12h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 12.5h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193342=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193342 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 12:36 Start Date: 01/Feb/19 12:36 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-459709207 There is the factory if thats the case where a map could be passed into each handler during favtory crreate. What we cant have though is statics for this tho This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193342) Time Spent: 12h 10m (was: 12h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 12h 10m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193332=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193332 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 12:07 Start Date: 01/Feb/19 12:07 Worklog Time Spent: 10m Work Description: onlyMIT commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-459702347 @michaelandrepearce This PR has updated. `SESSIONS` is still in MQTTSession, do not move to `MQTTProtocolHandler ` . Because SESSIONS should be shared by multiple Acceptors. ``` public class MQTTSession { static Map SESSIONS = new ConcurrentHashMap<>(); private static Map SESSIONS = new ConcurrentHashMap<>(); ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193332) Time Spent: 12h (was: 11h 50m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 12h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193320=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193320 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 11:14 Start Date: 01/Feb/19 11:14 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253014812 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: And clientId is null is this place: ``` @Override public ConnectionEntry createConnectionEntry(Acceptor acceptorUsed, Connection connection) { try { //New code, to kill of existingConnection if same client id. String clientID = connection.getProtocolConnection().getClientID(); MQTTConnection existingMQTTConnection = connectedClients.get(clientID); ``` clientId is set after the `createConnectionEntry `method is called. In `MQTTConnectionManager.connect method` ``` synchronized void connect(String cId, String username, byte[] passwordInBytes, boolean will, byte[] willMessage, String willTopic, boolean willRetain, int willQosLevel, boolean cleanSession) throws Exception { String clientId = validateClientId(cId, cleanSession); if (clientId == null) { session.getProtocolHandler().sendConnack(MqttConnectReturnCode.CONNECTION_REFUSED_IDENTIFIER_REJECTED); session.getProtocolHandler().disconnect(true); return; } String password = passwordInBytes == null ? null : new String(passwordInBytes, CharsetUtil.UTF_8); session.getConnection().setClientID(clientId); ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193320) Time Spent: 11h 50m (was: 11h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL:
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193319=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193319 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 11:05 Start Date: 01/Feb/19 11:05 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r253012354 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: My goal is to close connections that have the same clientId in different nodes.The following code already done the logic which your mentioned . ``` private String validateClientId(String clientId, boolean cleanSession) { if (clientId == null || clientId.isEmpty()) { // [MQTT-3.1.3-7] [MQTT-3.1.3-6] If client does not specify a client ID and clean session is set to 1 create it. if (cleanSession) { clientId = UUID.randomUUID().toString(); } else { // [MQTT-3.1.3-8] Return ID rejected and disconnect if clean session = false and client id is null return null; } } else { MQTTConnection connection = session.getProtocolManager().addConnectedClient(clientId, session.getConnection()); if (connection != null) { // [MQTT-3.1.4-2] If the client ID represents a client already connected to the server then the server MUST disconnect the existing client connection.disconnect(false); } } return clientId; } ``` I think I am almost revised, I will update after the test is completed. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193319) Time Spent: 11h 40m (was: 11.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 11h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193261=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193261 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 08:22 Start Date: 01/Feb/19 08:22 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252965032 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: e.g. from ``` @Override public ConnectionEntry createConnectionEntry(Acceptor acceptorUsed, Connection connection) { try { MQTTConnection mqttConnection = new MQTTConnection(connection); ConnectionEntry entry = new ConnectionEntry(mqttConnection, null, System.currentTimeMillis(), MQTTUtil.DEFAULT_KEEP_ALIVE_FREQUENCY); NettyServerConnection nettyConnection = ((NettyServerConnection) connection); MQTTProtocolHandler protocolHandler = nettyConnection.getChannel().pipeline().get(MQTTProtocolHandler.class); protocolHandler.setConnection(mqttConnection, entry); return entry; } catch (Exception e) { log.error(e); return null; } } ``` to something like: (replacing the code added with the logic you think is needed) ``` @Override public ConnectionEntry createConnectionEntry(Acceptor acceptorUsed, Connection connection) { try { MQTTConnection mqttConnection = new MQTTConnection(connection); ConnectionEntry entry = new ConnectionEntry(mqttConnection, null, System.currentTimeMillis(), MQTTUtil.DEFAULT_KEEP_ALIVE_FREQUENCY); NettyServerConnection nettyConnection = ((NettyServerConnection) connection); MQTTProtocolHandler protocolHandler = nettyConnection.getChannel().pipeline().get(MQTTProtocolHandler.class); protocolHandler.setConnection(mqttConnection, entry); String clientID = connection.getProtocolConnection().getClientID(); MQTTConnection existingMQTTConnection = connectedClients.get(clientID); if (existingMQTTConnection != null) { existingMQTTConnection.getTransportConnection().getProtocolConnection().destroy(); } return entry; } catch (Exception e) { log.error(e); return null; } } ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193263=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193263 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 08:24 Start Date: 01/Feb/19 08:24 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252965032 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: e.g. from ``` @Override public ConnectionEntry createConnectionEntry(Acceptor acceptorUsed, Connection connection) { try { MQTTConnection mqttConnection = new MQTTConnection(connection); ConnectionEntry entry = new ConnectionEntry(mqttConnection, null, System.currentTimeMillis(), MQTTUtil.DEFAULT_KEEP_ALIVE_FREQUENCY); NettyServerConnection nettyConnection = ((NettyServerConnection) connection); MQTTProtocolHandler protocolHandler = nettyConnection.getChannel().pipeline().get(MQTTProtocolHandler.class); protocolHandler.setConnection(mqttConnection, entry); return entry; } catch (Exception e) { log.error(e); return null; } } ``` to something like: (replacing the code added with the logic you think is needed) ``` @Override public ConnectionEntry createConnectionEntry(Acceptor acceptorUsed, Connection connection) { try { //New code, to kill of existingConnection if same client id. String clientID = connection.getProtocolConnection().getClientID(); MQTTConnection existingMQTTConnection = connectedClients.get(clientID); if (existingMQTTConnection != null) { existingMQTTConnection.getTransportConnection().getProtocolConnection().destroy(); } //End new code - dont commit with comments for illustration only MQTTConnection mqttConnection = new MQTTConnection(connection); ConnectionEntry entry = new ConnectionEntry(mqttConnection, null, System.currentTimeMillis(), MQTTUtil.DEFAULT_KEEP_ALIVE_FREQUENCY); NettyServerConnection nettyConnection = ((NettyServerConnection) connection); MQTTProtocolHandler protocolHandler = nettyConnection.getChannel().pipeline().get(MQTTProtocolHandler.class); protocolHandler.setConnection(mqttConnection, entry); return entry; } catch (Exception e) { log.error(e); return null; } } ```
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193257=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193257 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 08:16 Start Date: 01/Feb/19 08:16 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252963097 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: Looking some more when an connection with MQTT is created the MQTTProtocolManer is called, so actually may not even need notification could just hook into that logic and flow. createConnectionEntry method. As such you could just hook in the logic you need there as when a new connection is made that will be called. We also have the old connection This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193257) Time Spent: 11h 10m (was: 11h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 11h 10m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193256=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193256 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 08:13 Start Date: 01/Feb/19 08:13 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252963097 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: Looking some more when an connection with MQTT is created the MQTTProtocolHandler is called, so actually may not even need notification could just hook into that logic and flow This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193256) Time Spent: 11h (was: 10h 50m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 11h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193201=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193201 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 03:10 Start Date: 01/Feb/19 03:10 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252921461 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: got it thanks This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193201) Time Spent: 10h 50m (was: 10h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 10h 50m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193200=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193200 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 03:09 Start Date: 01/Feb/19 03:09 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252920340 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: @onlyMIT but re your top level options: > Add a new cluster notification when the client’s connections connect to Artemis, close existing connections that use the same clientId when processing cluster-notification. This solution is the best way to address the MQTT agreement.But I don't think it's necessary to solve this problem by sending a cluster-notification every time client connect. There already is a cluster-notification sent every time a client connects if using NotificationActiveMQServerPlugin, so if anything simply we could just make that more built in, like consumer notifications are. Re number of notifications, it would be less than consumers, as you can have many consumers per connection so really are not adding much there. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193200) Time Spent: 10h 40m (was: 10.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 10h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193193=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193193 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:58 Start Date: 01/Feb/19 02:58 Worklog Time Spent: 10m Work Description: onlyMIT commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-459587945 @michaelandrepearce thanks to your review, I will address the comments. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193193) Time Spent: 9h 50m (was: 9h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 9h 50m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193198=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193198 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 03:04 Start Date: 01/Feb/19 03:04 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252920340 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: @onlyMIT but re your top level options: > Add a new cluster notification when the client’s connections connect to Artemis, close existing connections that use the same clientId when processing cluster-notification. This solution is the best way to address the MQTT agreement.But I don't think it's necessary to solve this problem by sending a cluster-notification every time client connect. There already is a cluster-notification sent every time a client connects if using NotificationActiveMQServerPlugin, so if anything simply we could just make that more built in, like consumer notifications are, and simply make it enable the flag sendConnectionNotifications, if MQTT protocol is used (keeping it as is today disabed by default for those who have not enabled MQTT) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193198) Time Spent: 10h 20m (was: 10h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 10h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193199=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193199 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 03:04 Start Date: 01/Feb/19 03:04 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252920340 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: @onlyMIT but re your top level options: > Add a new cluster notification when the client’s connections connect to Artemis, close existing connections that use the same clientId when processing cluster-notification. This solution is the best way to address the MQTT agreement.But I don't think it's necessary to solve this problem by sending a cluster-notification every time client connect. There already is a cluster-notification sent every time a client connects if using NotificationActiveMQServerPlugin, so if anything simply we could just make that more built in, like consumer notifications are, and simply make it enable the flag sendConnectionNotifications, if MQTT protocol is used (keeping it as is today disabled by default for those who have not enabled MQTT) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193199) Time Spent: 10.5h (was: 10h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 10.5h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193197=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193197 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 03:03 Start Date: 01/Feb/19 03:03 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252920340 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: @onlyMIT but re your top level options: > Add a new cluster notification when the client’s connections connect to Artemis, close existing connections that use the same clientId when processing cluster-notification. This solution is the best way to address the MQTT agreement.But I don't think it's necessary to solve this problem by sending a cluster-notification every time client connect. There already is a cluster-notification sent every time a client connects if using NotificationActiveMQServerPlugin, so if anything simply we could just make that more built in, like consumer notifications are, and simply make it enable the flag sendConnectionNotifications, if MQTT protocol is used (keeping it as is today disabling for those who have not enabled MQTT) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193197) Time Spent: 10h 10m (was: 10h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 10h 10m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193196=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193196 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 03:01 Start Date: 01/Feb/19 03:01 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252920340 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: @onlyMIT but re your top level options: > Add a new cluster notification when the client’s connections connect to Artemis, close existing connections that use the same clientId when processing cluster-notification. This solution is the best way to address the MQTT agreement.But I don't think it's necessary to solve this problem by sending a cluster-notification every time client connect. There already is a cluster-notification sent every time a client connects if using NotificationActiveMQServerPlugin, so if anything simply we could just make that more built in, like consumer one is, and simply make it enable the flag sendConnectionNotifications, if MQTT protocol is used (keeping it as is today disabling for those who have not enabled MQTT) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193196) Time Spent: 10h (was: 9h 50m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 10h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193190=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193190 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:49 Start Date: 01/Feb/19 02:49 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252918661 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: This code seems to be easily misunderstood, I am trying to find out if there is a better way to handle this logic. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193190) Time Spent: 9h 20m (was: 9h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 9h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193192=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193192 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:53 Start Date: 01/Feb/19 02:53 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252919168 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: Thanks for your answer,i not find the connection notification,if we had we can use it. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193192) Time Spent: 9h 40m (was: 9.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 9h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193191=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193191 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:50 Start Date: 01/Feb/19 02:50 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252918453 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: we already have a connection notification, just need to add the generic extra data as you were going to on the consumer ``` private void sendConnectionNotification(final RemotingConnection connection, final CoreNotificationType type) { final ManagementService managementService = getManagementService(); if (managementService != null && sendConnectionNotifications) { try { final TypedProperties props = new TypedProperties(); props.putSimpleStringProperty(ManagementHelper.HDR_CONNECTION_NAME, SimpleString.toSimpleString(connection.getID().toString())); props.putSimpleStringProperty(ManagementHelper.HDR_REMOTE_ADDRESS, SimpleString.toSimpleString(connection.getRemoteAddress())); props.putSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID, SimpleString.toSimpleString(connection.getClientID())); props.putSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME, SimpleString.toSimpleString(connection.getProtocolName())); managementService.sendNotification(new Notification(null, type, props)); } catch (Exception e) { logger.warn("Error sending notification: " + type, e.getMessage(), e); } } } ``` and then act just at the connection level. MQTTProtocolManager already is keeping track of MQTT connections also. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193191) Time Spent: 9.5h (was: 9h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 >
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193188=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193188 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:48 Start Date: 01/Feb/19 02:48 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252918453 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: we already have a connection notification, just need to add the generic extra data as you were going to on the consumer private void sendConnectionNotification(final RemotingConnection connection, final CoreNotificationType type) { final ManagementService managementService = getManagementService(); if (managementService != null && sendConnectionNotifications) { try { final TypedProperties props = new TypedProperties(); props.putSimpleStringProperty(ManagementHelper.HDR_CONNECTION_NAME, SimpleString.toSimpleString(connection.getID().toString())); props.putSimpleStringProperty(ManagementHelper.HDR_REMOTE_ADDRESS, SimpleString.toSimpleString(connection.getRemoteAddress())); props.putSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID, SimpleString.toSimpleString(connection.getClientID())); props.putSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME, SimpleString.toSimpleString(connection.getProtocolName())); managementService.sendNotification(new Notification(null, type, props)); } catch (Exception e) { logger.warn("Error sending notification: " + type, e.getMessage(), e); } } } and then act just at the connection level. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193188) Time Spent: 9h (was: 8h 50m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193189=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193189 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:48 Start Date: 01/Feb/19 02:48 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252918453 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: we already have a connection notification, just need to add the generic extra data as you were going to on the consumer ``` private void sendConnectionNotification(final RemotingConnection connection, final CoreNotificationType type) { final ManagementService managementService = getManagementService(); if (managementService != null && sendConnectionNotifications) { try { final TypedProperties props = new TypedProperties(); props.putSimpleStringProperty(ManagementHelper.HDR_CONNECTION_NAME, SimpleString.toSimpleString(connection.getID().toString())); props.putSimpleStringProperty(ManagementHelper.HDR_REMOTE_ADDRESS, SimpleString.toSimpleString(connection.getRemoteAddress())); props.putSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID, SimpleString.toSimpleString(connection.getClientID())); props.putSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME, SimpleString.toSimpleString(connection.getProtocolName())); managementService.sendNotification(new Notification(null, type, props)); } catch (Exception e) { logger.warn("Error sending notification: " + type, e.getMessage(), e); } } } ``` and then act just at the connection level. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193189) Time Spent: 9h 10m (was: 9h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193185=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193185 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:44 Start Date: 01/Feb/19 02:44 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252917956 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: Why is: Adding a new cluster notification when the client’s connections connect to Artemis, it is the best way to address the MQTT agreement, but the cost is too great Whats the cost? To me if you want the behaviour on connection behaviour this truely would be better, rather than working on consumer This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193185) Time Spent: 8h 50m (was: 8h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 8h 50m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193182=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193182 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:40 Start Date: 01/Feb/19 02:40 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252917296 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: But your current logic in this PR is simply find all consumers with matching clientId and kill beneath it, their connection. You may as well simply close down the consumer This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193182) Time Spent: 8h 40m (was: 8.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 8h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193181=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193181 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:39 Start Date: 01/Feb/19 02:39 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252917296 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: But your current logic is simply find all consumers with matching clientId and kill beneath it, their connection. You may as well simply close down the consumer This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193181) Time Spent: 8.5h (was: 8h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 8.5h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193167=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193167 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:28 Start Date: 01/Feb/19 02:28 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252915920 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; Review comment: I answered in another comment, I still think that closing the connection is a better choice. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193167) Time Spent: 7h 50m (was: 7h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 7h 50m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193171=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193171 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:35 Start Date: 01/Feb/19 02:35 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252916780 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSession.java ## @@ -30,10 +30,12 @@ public class MQTTSession { - static Map SESSIONS = new ConcurrentHashMap<>(); + private static Map SESSIONS = new ConcurrentHashMap<>(); Review comment: i sent a possible solution to address this, in a pr to your branch. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193171) Time Spent: 8h 10m (was: 8h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 8h 10m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193172=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193172 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:35 Start Date: 01/Feb/19 02:35 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252916808 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); Review comment: i sent this in pr to your branch This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193172) Time Spent: 8h 20m (was: 8h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 8h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193169=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193169 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:33 Start Date: 01/Feb/19 02:33 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on issue #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#issuecomment-459583628 @onlyMIT to help with addressing the comments i sent a pr to your branch with a number of proposed changes This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193169) Time Spent: 8h (was: 7h 50m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 8h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193163=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193163 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:17 Start Date: 01/Feb/19 02:17 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252914292 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: Our goal is to avoid simultaneous occurrence of multiple MQTT connections with the same clientId, not to prevent MQTT consumers with the same clientId to subscribe the same topic. described in the [two solutions](https://github.com/apache/activemq-artemis/pull/2528#issue-248449219) . Adding a new cluster notification when the client’s connections connect to Artemis, it is the best way to address the MQTT agreement, but the cost is too great. So it put the logic in CREATE_CONSUMER cluster-notification. The following code description, before each processing of the mqtt connection request in artemis, check whether there is a connection with the same clientId in the node, and if so, close it. Similarly, in the cluster, we also want to avoid the connection of the same clientId. Executing this logic only in CREATE_CONSUMER is just that I think it can reduce resource consumption and avoid problems, although this is not the best way to address the MQTT specification. ``` private String validateClientId(String clientId, boolean cleanSession) { if (clientId == null || clientId.isEmpty()) { // [MQTT-3.1.3-7] [MQTT-3.1.3-6] If client does not specify a client ID and clean session is set to 1 create it. if (cleanSession) { clientId = UUID.randomUUID().toString(); } else { // [MQTT-3.1.3-8] Return ID rejected and disconnect if clean session = false and client id is null return null; } } else { MQTTConnection connection = session.getProtocolManager().addConnectedClient(clientId, session.getConnection()); if (connection != null) { // [MQTT-3.1.4-2] If the client ID represents a client already connected to the server then the server MUST disconnect the existing client connection.disconnect(false); } } return clientId; } ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193166=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193166 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:24 Start Date: 01/Feb/19 02:24 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252915302 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSession.java ## @@ -30,10 +30,12 @@ public class MQTTSession { - static Map SESSIONS = new ConcurrentHashMap<>(); + private static Map SESSIONS = new ConcurrentHashMap<>(); Review comment: You are right, it does not seem to fit properly in the MQTTSession This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193166) Time Spent: 7h 40m (was: 7.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 7h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193165=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193165 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:24 Start Date: 01/Feb/19 02:24 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252915242 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSession.java ## @@ -204,4 +208,35 @@ public CoreMessageObjectPools getCoreMessageObjectPools() { public static Map getSessions() { return new HashMap<>(SESSIONS); } + + public MQTTSessionState getSessionStateFromSessionMap(String clientId) { + return SESSIONS.get(generateSessionStateKey(clientId)); + } + + public void putSessionStateIntoSessionMap(String clientId, MQTTSessionState state) { + SESSIONS.put(generateSessionStateKey(clientId), state); + } + + public void removeSessionStateFromSessionMap(String clientId) { + SESSIONS.remove(generateSessionStateKey(clientId)); + } + + /** +* When performing cluster testing, different nodes of the cluster are actually started in the same JVM process, Review comment: This is because we have state in a static, that is really bad, and simply needs to be addressed, see other comment. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193165) Time Spent: 7.5h (was: 7h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 7.5h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193162=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193162 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:17 Start Date: 01/Feb/19 02:17 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252914292 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; + if (clientId.equals(serverConsumer.getConnectionClientID())) + serverConsumer.getRemotingConnection().destroy(); Review comment: Our goal is to avoid simultaneous occurrence of multiple MQTT connections with the same clientId, not to prevent MQTT consumers with the same clientId to subscribe the same topic. described in the [two solutions](https://github.com/apache/activemq-artemis/pull/2528#issue-248449219) . Adding a new cluster notification when the client’s connections connect to Artemis, it is the best way to address the MQTT agreement, but the cost is too great. So it put the logic in CREATE_CONSUMER cluster-notification. The following code description, before each processing of the mqtt connection request in artemis, check whether there is a connection with the same clientId in the node, and if so, close it. Similarly, in the cluster, we also want to avoid the connection of the same clientId. Executing this logic only in CREATE_CONSUMER is just that I think it can reduce resource consumption and avoid problems, although this is not the way to match the MQTT specification. ``` private String validateClientId(String clientId, boolean cleanSession) { if (clientId == null || clientId.isEmpty()) { // [MQTT-3.1.3-7] [MQTT-3.1.3-6] If client does not specify a client ID and clean session is set to 1 create it. if (cleanSession) { clientId = UUID.randomUUID().toString(); } else { // [MQTT-3.1.3-8] Return ID rejected and disconnect if clean session = false and client id is null return null; } } else { MQTTConnection connection = session.getProtocolManager().addConnectedClient(clientId, session.getConnection()); if (connection != null) { // [MQTT-3.1.4-2] If the client ID represents a client already connected to the server then the server MUST disconnect the existing client connection.disconnect(false); } } return clientId; } ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193164=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193164 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:21 Start Date: 01/Feb/19 02:21 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252914838 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); Review comment: got it This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193164) Time Spent: 7h 20m (was: 7h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 7h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193156=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193156 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:07 Start Date: 01/Feb/19 02:07 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252912679 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; Review comment: what method? if youre talking re remoting connection that shouldnt be needed, as you simply want to close previous consumers, you therefor should be calling close method on the serverconsumer, which is in interface This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193156) Time Spent: 6h 50m (was: 6h 40m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 6h 50m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193150=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193150 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:05 Start Date: 01/Feb/19 02:05 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252912679 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; Review comment: what method? if youre talking re remoting connection you shouldnt be even have added that new method, you simply want to close previous consumers, you therefor should be calling close method on the consumer. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193150) Time Spent: 6.5h (was: 6h 20m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 6.5h > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193155=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193155 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 02:05 Start Date: 01/Feb/19 02:05 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252912679 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; Review comment: what method? if youre talking re remoting connection you shouldnt be even have added that new method, you simply want to close previous consumers, you therefor should be calling close method on the serverconsumer, which is in interface This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193155) Time Spent: 6h 40m (was: 6.5h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 6h 40m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193144=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193144 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 01:49 Start Date: 01/Feb/19 01:49 Worklog Time Spent: 10m Work Description: onlyMIT commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252910278 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTProtocolManager.java ## @@ -62,11 +73,44 @@ List outgoingInterceptors) { this.server = server; this.updateInterceptors(incomingInterceptors, outgoingInterceptors); + server.getManagementService().addNotificationListener(this); } @Override public void onNotification(Notification notification) { - // TODO handle notifications + if (!(notification.getType() instanceof CoreNotificationType)) + return; + + CoreNotificationType type = (CoreNotificationType) notification.getType(); + if (type != CONSUMER_CREATED) + return; + + TypedProperties props = notification.getProperties(); + + SimpleString protocolName = props.getSimpleStringProperty(ManagementHelper.HDR_PROTOCOL_NAME); + + if (protocolName == null || !protocolName.toString().equals(MQTTProtocolManagerFactory.MQTT_PROTOCOL_NAME)) + return; + + int distance = props.getIntProperty(ManagementHelper.HDR_DISTANCE); + + if (distance > 0) { + SimpleString queueName = props.getSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME); + + Binding binding = server.getPostOffice().getBinding(queueName); + if (binding != null) { +Queue queue = (Queue) binding.getBindable(); +String clientId = props.getSimpleStringProperty(ManagementHelper.HDR_CLIENT_ID).toString(); +//If the client ID represents a client already connected to the server then the server MUST disconnect the existing client. +//Avoid consumers with the same client ID in the cluster appearing at different nodes at the same time +Collection consumersSet = queue.getConsumers(); +for (Consumer consumer : consumersSet) { + ServerConsumerImpl serverConsumer = (ServerConsumerImpl) consumer; Review comment: I also disagree with putting this method into the `ServerConsumer` interface. Using instanceof is a good suggestion. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193144) Time Spent: 6h 20m (was: 6h 10m) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 6h 20m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (ARTEMIS-2226) (MQTT)In the cluster,the last consumer connection should close the previous consumer connection
[ https://issues.apache.org/jira/browse/ARTEMIS-2226?focusedWorklogId=193143=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-193143 ] ASF GitHub Bot logged work on ARTEMIS-2226: --- Author: ASF GitHub Bot Created on: 01/Feb/19 01:49 Start Date: 01/Feb/19 01:49 Worklog Time Spent: 10m Work Description: michaelandrepearce commented on pull request #2528: ARTEMIS-2226 last consumer connection should close the previous consu… URL: https://github.com/apache/activemq-artemis/pull/2528#discussion_r252900294 ## File path: artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSession.java ## @@ -30,10 +30,12 @@ public class MQTTSession { - static Map SESSIONS = new ConcurrentHashMap<>(); + private static Map SESSIONS = new ConcurrentHashMap<>(); Review comment: Looking at this im bit worried that this is a static in first place, e.g. its possible to have multiple embedded brokers, in that case each broker shouldnt share any state, by being static means they would. really should be held as a non static at some common level above where would just be one per acceptor such as MQTTProtocolHandler This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 193143) Time Spent: 6h 10m (was: 6h) > (MQTT)In the cluster,the last consumer connection should close the previous > consumer connection > --- > > Key: ARTEMIS-2226 > URL: https://issues.apache.org/jira/browse/ARTEMIS-2226 > Project: ActiveMQ Artemis > Issue Type: Bug >Reporter: Shiping Liang >Priority: Major > Time Spent: 6h 10m > Remaining Estimate: 0h > > Multiple consumers using the same clientId in the cluster, the last consumer > connection should close the previous consumer connection! -- This message was sent by Atlassian JIRA (v7.6.3#76005)