You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2020/02/27 10:49:44 UTC

[GitHub] [rocketmq] xujianhai666 opened a new pull request #1799: feat(admin): remove group of deleted sub info when queryTopicConsumeB…

xujianhai666 opened a new pull request #1799: feat(admin): remove group of deleted sub info when queryTopicConsumeB…
URL: https://github.com/apache/rocketmq/pull/1799
 
 
   when use `mqadmin deleteSubGroup` to delte subInfo, broker will
   remove subInfo from SubscriptionGroupManager. but when admin use
   `queryTopicConsumeByWho` to get consumer info, the delted group
   of sub will back, which is not expected.
   
   - use SubscriptionGroupManager to filter groups
   
   Closes #1798
    

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [rocketmq] xujianhai666 commented on a change in pull request #1799: [ISSUE #1798]feat(admin): remove group of deleted sub info when queryTopicConsumeB…

Posted by GitBox <gi...@apache.org>.
xujianhai666 commented on a change in pull request #1799: [ISSUE #1798]feat(admin): remove group of deleted sub info when queryTopicConsumeB…
URL: https://github.com/apache/rocketmq/pull/1799#discussion_r385493569
 
 

 ##########
 File path: broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
 ##########
 @@ -1025,8 +1026,15 @@ private RemotingCommand queryTopicConsumeByWho(ChannelHandlerContext ctx,
             groups.addAll(groupInOffset);
         }
 
+        HashSet<String> filteredGroups = new HashSet<String>();
 
 Review comment:
   @duhenglucky so maybe I should add request flag for client, then broker will check flag bool indicator to dicide to filter ? 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [rocketmq] coveralls edited a comment on issue #1799: [ISSUE #1798]feat(admin): remove group of deleted sub info when queryTopicConsumeB…

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #1799: [ISSUE #1798]feat(admin): remove group of deleted sub info when queryTopicConsumeB…
URL: https://github.com/apache/rocketmq/pull/1799#issuecomment-592283877
 
 
   
   [![Coverage Status](https://coveralls.io/builds/29074151/badge)](https://coveralls.io/builds/29074151)
   
   Coverage decreased (-0.2%) to 50.883% when pulling **1a7cc036a73d2a562c9ce4db4c55c71ab683753a on xujianhai666:fix-group-noexits** into **fc3fb2fdbe1d3c34299eb209e71d8f8aae54962e on apache:develop**.
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [rocketmq] coveralls commented on issue #1799: [ISSUE #1798]feat(admin): remove group of deleted sub info when queryTopicConsumeB…

Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #1799: [ISSUE #1798]feat(admin): remove group of deleted sub info when queryTopicConsumeB…
URL: https://github.com/apache/rocketmq/pull/1799#issuecomment-592283877
 
 
   
   [![Coverage Status](https://coveralls.io/builds/29014873/badge)](https://coveralls.io/builds/29014873)
   
   Coverage decreased (-0.2%) to 50.872% when pulling **093c703bb0ffc51d107596ea658d231e3ba285b8 on xujianhai666:fix-group-noexits** into **fc3fb2fdbe1d3c34299eb209e71d8f8aae54962e on apache:develop**.
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [rocketmq] duhenglucky commented on a change in pull request #1799: [ISSUE #1798]feat(admin): remove group of deleted sub info when queryTopicConsumeB…

Posted by GitBox <gi...@apache.org>.
duhenglucky commented on a change in pull request #1799: [ISSUE #1798]feat(admin): remove group of deleted sub info when queryTopicConsumeB…
URL: https://github.com/apache/rocketmq/pull/1799#discussion_r385491861
 
 

 ##########
 File path: broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
 ##########
 @@ -1025,8 +1026,15 @@ private RemotingCommand queryTopicConsumeByWho(ChannelHandlerContext ctx,
             groups.addAll(groupInOffset);
         }
 
+        HashSet<String> filteredGroups = new HashSet<String>();
 
 Review comment:
   It would be better to make an optional configuration here, because some users also want to show the consumption progress of topics that they have subscribed to, and it seems that users in the community are already used to this usage, so forward compatibility is necessary

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [rocketmq] duhenglucky commented on a change in pull request #1799: [ISSUE #1798]feat(admin): remove group of deleted sub info when queryTopicConsumeB…

Posted by GitBox <gi...@apache.org>.
duhenglucky commented on a change in pull request #1799: [ISSUE #1798]feat(admin): remove group of deleted sub info when queryTopicConsumeB…
URL: https://github.com/apache/rocketmq/pull/1799#discussion_r385494020
 
 

 ##########
 File path: broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
 ##########
 @@ -1025,8 +1026,15 @@ private RemotingCommand queryTopicConsumeByWho(ChannelHandlerContext ctx,
             groups.addAll(groupInOffset);
         }
 
+        HashSet<String> filteredGroups = new HashSet<String>();
 
 Review comment:
   @ IMHO, it seems that add a flag in the client will be a better choice

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services