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 09:29:52 UTC
[GitHub] [rocketmq] xujianhai666 opened a new pull request #1797:
feat(admin): remove extra code for getConsumeStats
xujianhai666 opened a new pull request #1797: feat(admin): remove extra code for getConsumeStats
URL: https://github.com/apache/rocketmq/pull/1797
two lines are logic conflicted. when findSubscriptionData is null, then
findSubscriptionDataCount is always zero, so it never run into
`continue` code
Closes #1796
----------------------------------------------------------------
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
#1797: [ISSUE #1796]feat(admin): filter deleted group for getConsumeStats
Posted by GitBox <gi...@apache.org>.
xujianhai666 commented on a change in pull request #1797: [ISSUE #1796]feat(admin): filter deleted group for getConsumeStats
URL: https://github.com/apache/rocketmq/pull/1797#discussion_r386090495
##########
File path: broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
##########
@@ -864,17 +864,6 @@ private RemotingCommand getConsumeStats(ChannelHandlerContext ctx,
continue;
}
- {
- SubscriptionData findSubscriptionData =
- this.brokerController.getConsumerManager().findSubscriptionData(requestHeader.getConsumerGroup(), topic);
-
- if (null == findSubscriptionData
Review comment:
@duhenglucky According to discuss above, I update code to distinguish group deleted and sub many topic.
----------------------------------------------------------------
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 #1797:
[ISSUE #1796]feat(admin): remove extra code for getConsumeStats
Posted by GitBox <gi...@apache.org>.
duhenglucky commented on a change in pull request #1797: [ISSUE #1796]feat(admin): remove extra code for getConsumeStats
URL: https://github.com/apache/rocketmq/pull/1797#discussion_r385532740
##########
File path: broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
##########
@@ -864,17 +864,6 @@ private RemotingCommand getConsumeStats(ChannelHandlerContext ctx,
continue;
}
- {
- SubscriptionData findSubscriptionData =
- this.brokerController.getConsumerManager().findSubscriptionData(requestHeader.getConsumerGroup(), topic);
-
- if (null == findSubscriptionData
Review comment:
yep, if the topic was set in the request, these codes look a bit messy, and we need to deal with this in an elegant way
----------------------------------------------------------------
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 opened a new pull request #1797: [ISSUE
#1796]feat(admin): remove extra code for getConsumeStats
Posted by GitBox <gi...@apache.org>.
xujianhai666 opened a new pull request #1797: [ISSUE #1796]feat(admin): remove extra code for getConsumeStats
URL: https://github.com/apache/rocketmq/pull/1797
two lines are logic conflicted. when findSubscriptionData is null, then
findSubscriptionDataCount is always zero, so it never run into
`continue` code
Closes #1796
----------------------------------------------------------------
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 #1797: feat(admin): remove
extra code for getConsumeStats
Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #1797: feat(admin): remove extra code for getConsumeStats
URL: https://github.com/apache/rocketmq/pull/1797#issuecomment-591895957
[](https://coveralls.io/builds/28994389)
Coverage increased (+0.02%) to 50.955% when pulling **14286dba8db4da2208240ff1ceb60c228e71f2ce on xujianhai666:feat-extra** into **73bb4b402a5da02faf9feded8f4d79dd1ba88348 on apache:master**.
----------------------------------------------------------------
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 #1797: [ISSUE
#1796]feat(admin): remove extra code for getConsumeStats
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #1797: [ISSUE #1796]feat(admin): remove extra code for getConsumeStats
URL: https://github.com/apache/rocketmq/pull/1797#issuecomment-591895957
[](https://coveralls.io/builds/29018388)
Coverage decreased (-0.1%) to 50.909% when pulling **14286dba8db4da2208240ff1ceb60c228e71f2ce on xujianhai666:feat-extra** 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] xujianhai666 commented on a change in pull request
#1797: [ISSUE #1796]feat(admin): remove extra code for getConsumeStats
Posted by GitBox <gi...@apache.org>.
xujianhai666 commented on a change in pull request #1797: [ISSUE #1796]feat(admin): remove extra code for getConsumeStats
URL: https://github.com/apache/rocketmq/pull/1797#discussion_r385493155
##########
File path: broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
##########
@@ -864,17 +864,6 @@ private RemotingCommand getConsumeStats(ChannelHandlerContext ctx,
continue;
}
- {
- SubscriptionData findSubscriptionData =
- this.brokerController.getConsumerManager().findSubscriptionData(requestHeader.getConsumerGroup(), topic);
-
- if (null == findSubscriptionData
Review comment:
got 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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [rocketmq] xujianhai666 commented on a change in pull request
#1797: [ISSUE #1796]feat(admin): remove extra code for getConsumeStats
Posted by GitBox <gi...@apache.org>.
xujianhai666 commented on a change in pull request #1797: [ISSUE #1796]feat(admin): remove extra code for getConsumeStats
URL: https://github.com/apache/rocketmq/pull/1797#discussion_r385522772
##########
File path: broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
##########
@@ -864,17 +864,6 @@ private RemotingCommand getConsumeStats(ChannelHandlerContext ctx,
continue;
}
- {
- SubscriptionData findSubscriptionData =
- this.brokerController.getConsumerManager().findSubscriptionData(requestHeader.getConsumerGroup(), topic);
-
- if (null == findSubscriptionData
Review comment:
@duhenglucky I know ur idea, but this code should check the case: group is not used, then the findSubscriptionData return null and findSubscriptionDataCount return 0. As the code run, the not used group will return back to user. so maybe we should distinguish two case, one as u say above, and the second as I refer. Do u agree with that?
----------------------------------------------------------------
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 #1797:
[ISSUE #1796]feat(admin): remove extra code for getConsumeStats
Posted by GitBox <gi...@apache.org>.
duhenglucky commented on a change in pull request #1797: [ISSUE #1796]feat(admin): remove extra code for getConsumeStats
URL: https://github.com/apache/rocketmq/pull/1797#discussion_r385490415
##########
File path: broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
##########
@@ -864,17 +864,6 @@ private RemotingCommand getConsumeStats(ChannelHandlerContext ctx,
continue;
}
- {
- SubscriptionData findSubscriptionData =
- this.brokerController.getConsumerManager().findSubscriptionData(requestHeader.getConsumerGroup(), topic);
-
- if (null == findSubscriptionData
Review comment:
findSubscriptionData = null not means the subscription count is 0 because one consumer group can subscribe more than one topic.
----------------------------------------------------------------
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
#1797: [ISSUE #1796]feat(admin): remove extra code for getConsumeStats
Posted by GitBox <gi...@apache.org>.
xujianhai666 commented on a change in pull request #1797: [ISSUE #1796]feat(admin): remove extra code for getConsumeStats
URL: https://github.com/apache/rocketmq/pull/1797#discussion_r385521817
##########
File path: broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
##########
@@ -864,17 +864,6 @@ private RemotingCommand getConsumeStats(ChannelHandlerContext ctx,
continue;
}
- {
- SubscriptionData findSubscriptionData =
- this.brokerController.getConsumerManager().findSubscriptionData(requestHeader.getConsumerGroup(), topic);
-
- if (null == findSubscriptionData
Review comment:
@duhenglucky no as ur expected, as code above, when consumerGroupInfo==null, then findSubscriptionDataCount will return 0.
----------------------------------------------------------------
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 #1797: [ISSUE
#1796]feat(admin): filter deleted group for getConsumeStats
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #1797: [ISSUE #1796]feat(admin): filter deleted group for getConsumeStats
URL: https://github.com/apache/rocketmq/pull/1797#issuecomment-591895957
[](https://coveralls.io/builds/29053896)
Coverage decreased (-0.02%) to 50.856% when pulling **28875be00a5737b11dbcde768d8e52b8fcf20702 on xujianhai666:feat-extra** into **997164b1cdc122bfd767d23b476a789461507332 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] xujianhai666 closed pull request #1797: [ISSUE
#1796]feat(admin): remove extra code for getConsumeStats
Posted by GitBox <gi...@apache.org>.
xujianhai666 closed pull request #1797: [ISSUE #1796]feat(admin): remove extra code for getConsumeStats
URL: https://github.com/apache/rocketmq/pull/1797
----------------------------------------------------------------
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