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 2021/09/22 06:05:00 UTC

[GitHub] [rocketmq] ni-ze opened a new pull request #3365: Allocate MessageQueue to client first by same Machine IP.

ni-ze opened a new pull request #3365:
URL: https://github.com/apache/rocketmq/pull/3365


   add a new allocate MessageQueue strategy: Allocated by ip, if messageQueue localed in broker has same ip with client, Allocate messageQueue to this client firstly.
   


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

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

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

[GitHub] [rocketmq] ni-ze commented on pull request #3365: [ISSUE #3366] Allocate MessageQueue to client first by same Machine IP.

Posted by GitBox <gi...@apache.org>.
ni-ze commented on pull request #3365:
URL: https://github.com/apache/rocketmq/pull/3365#issuecomment-924655988


   > Why not add an interface, but directly modify the original interface?
   
   when allocate MessageQueue, I think it is a natural thing to tall allocator which broker messageQueues local in.


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

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

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

[GitHub] [rocketmq] ShannonDing commented on pull request #3365: [ISSUE #3366] Allocate MessageQueue to client first by same Machine IP.

Posted by GitBox <gi...@apache.org>.
ShannonDing commented on pull request #3365:
URL: https://github.com/apache/rocketmq/pull/3365#issuecomment-924626713


   Why not add an interface, but directly modify the original interface?


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

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

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

[GitHub] [rocketmq] coveralls commented on pull request #3365: [ISSUE #3366] Allocate MessageQueue to client first by same Machine IP.

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3365:
URL: https://github.com/apache/rocketmq/pull/3365#issuecomment-925452224


   
   [![Coverage Status](https://coveralls.io/builds/43013101/badge)](https://coveralls.io/builds/43013101)
   
   Coverage increased (+0.01%) to 53.212% when pulling **8c5035f1257297e1f633477a305efc6c7907a1ef on ni-ze:allocateByMachine** into **48d3c7eeba3d6658de10b5d8fd5890b513a16eb5 on apache:5.0.0-preview**.
   


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

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

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

[GitHub] [rocketmq] ShannonDing commented on a change in pull request #3365: [ISSUE #3366] Allocate MessageQueue to client first by same Machine IP.

Posted by GitBox <gi...@apache.org>.
ShannonDing commented on a change in pull request #3365:
URL: https://github.com/apache/rocketmq/pull/3365#discussion_r713628436



##########
File path: broker/src/main/java/org/apache/rocketmq/broker/processor/QueryAssignmentProcessor.java
##########
@@ -220,13 +220,13 @@ private RemotingCommand queryAssignment(ChannelHandlerContext ctx, RemotingComma
                         } else {
                             if (cidAll.size() <= mqAll.size()) {
                                 //consumer working in pop mode could share the MessageQueues assigned to the N (N = popWorkGroupSize) consumer following it in the cid list
-                                allocateResult = allocateMessageQueueStrategy.allocate(consumerGroup, clientId, mqAll, cidAll);
+                                allocateResult = allocateMessageQueueStrategy.allocate(null, consumerGroup, clientId, mqAll, cidAll);

Review comment:
       Is it compatible with the previous version?




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

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

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

[GitHub] [rocketmq] Jason918 commented on a change in pull request #3365: [ISSUE #3366] Allocate MessageQueue to client first by same Machine IP.

Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #3365:
URL: https://github.com/apache/rocketmq/pull/3365#discussion_r725611259



##########
File path: client/src/main/java/org/apache/rocketmq/client/consumer/AllocateMessageQueueStrategy.java
##########
@@ -27,13 +30,16 @@
     /**
      * Allocating by consumer id
      *
+     * @param topicRouteData runtime info of topic route data, use it when you need broker info to allocate,
+     *                       if don't use it, can be null.
      * @param consumerGroup current consumer group
      * @param currentCID current consumer id
      * @param mqAll message queue set in current topic
      * @param cidAll consumer set in current consumer group
      * @return The allocate result of given strategy
      */
     List<MessageQueue> allocate(
+        final TopicRouteData topicRouteData,

Review comment:
       Maybe it's better to avoid adding so many nulls in original strategy calls by retain the origin interface here with default method implementation.




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

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

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

[GitHub] [rocketmq] ni-ze commented on a change in pull request #3365: [ISSUE #3366] Allocate MessageQueue to client first by same Machine IP.

Posted by GitBox <gi...@apache.org>.
ni-ze commented on a change in pull request #3365:
URL: https://github.com/apache/rocketmq/pull/3365#discussion_r713653321



##########
File path: broker/src/main/java/org/apache/rocketmq/broker/processor/QueryAssignmentProcessor.java
##########
@@ -220,13 +220,13 @@ private RemotingCommand queryAssignment(ChannelHandlerContext ctx, RemotingComma
                         } else {
                             if (cidAll.size() <= mqAll.size()) {
                                 //consumer working in pop mode could share the MessageQueues assigned to the N (N = popWorkGroupSize) consumer following it in the cid list
-                                allocateResult = allocateMessageQueueStrategy.allocate(consumerGroup, clientId, mqAll, cidAll);
+                                allocateResult = allocateMessageQueueStrategy.allocate(null, consumerGroup, clientId, mqAll, cidAll);

Review comment:
       QueryAssignmentProcessor and AllocateMessageQueueStrategy always  has same version, new version of QueryAssignmentProcessor will not depend on old AllocateMessageQueueStrategy interface.




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

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

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

[GitHub] [rocketmq] codecov-commenter commented on pull request #3365: [ISSUE #3366] Allocate MessageQueue to client first by same Machine IP.

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #3365:
URL: https://github.com/apache/rocketmq/pull/3365#issuecomment-925452261


   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3365?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3365](https://codecov.io/gh/apache/rocketmq/pull/3365?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8c5035f) into [5.0.0-preview](https://codecov.io/gh/apache/rocketmq/commit/da5d30b4ec619a75fd854d9f1551bce4da48f12d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (da5d30b) will **increase** coverage by `0.82%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/rocketmq/pull/3365/graphs/tree.svg?width=650&height=150&src=pr&token=4w0sxP1wZv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/rocketmq/pull/3365?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@                 Coverage Diff                 @@
   ##             5.0.0-preview    #3365      +/-   ##
   ===================================================
   + Coverage            46.61%   47.44%   +0.82%     
   + Complexity            4955     3460    -1495     
   ===================================================
     Files                  610      463     -147     
     Lines                41964    29979   -11985     
     Branches              5757     4096    -1661     
   ===================================================
   - Hits                 19561    14223    -5338     
   + Misses               19751    13811    -5940     
   + Partials              2652     1945     -707     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3365?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...tmq/broker/processor/QueryAssignmentProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3365/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL1F1ZXJ5QXNzaWdubWVudFByb2Nlc3Nvci5qYXZh) | `53.03% <0.00%> (ø)` | |
   | [...rocketmq/broker/filtersrv/FilterServerManager.java](https://codecov.io/gh/apache/rocketmq/pull/3365/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvZmlsdGVyc3J2L0ZpbHRlclNlcnZlck1hbmFnZXIuamF2YQ==) | `20.00% <0.00%> (-14.29%)` | :arrow_down: |
   | [...ketmq/logappender/log4j/RocketmqLog4jAppender.java](https://codecov.io/gh/apache/rocketmq/pull/3365/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bG9nYXBwZW5kZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL2xvZ2FwcGVuZGVyL2xvZzRqL1JvY2tldG1xTG9nNGpBcHBlbmRlci5qYXZh) | `40.35% <0.00%> (-10.53%)` | :arrow_down: |
   | [...va/org/apache/rocketmq/logging/inner/Appender.java](https://codecov.io/gh/apache/rocketmq/pull/3365/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bG9nZ2luZy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbG9nZ2luZy9pbm5lci9BcHBlbmRlci5qYXZh) | `32.58% <0.00%> (-4.50%)` | :arrow_down: |
   | [...ketmq/common/protocol/body/ConsumerConnection.java](https://codecov.io/gh/apache/rocketmq/pull/3365/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvYm9keS9Db25zdW1lckNvbm5lY3Rpb24uamF2YQ==) | `95.83% <0.00%> (-4.17%)` | :arrow_down: |
   | [...a/org/apache/rocketmq/filter/util/BloomFilter.java](https://codecov.io/gh/apache/rocketmq/pull/3365/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZmlsdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9maWx0ZXIvdXRpbC9CbG9vbUZpbHRlci5qYXZh) | `59.13% <0.00%> (-2.16%)` | :arrow_down: |
   | [...a/org/apache/rocketmq/store/StoreStatsService.java](https://codecov.io/gh/apache/rocketmq/pull/3365/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL1N0b3JlU3RhdHNTZXJ2aWNlLmphdmE=) | `28.85% <0.00%> (-0.66%)` | :arrow_down: |
   | [...n/java/org/apache/rocketmq/store/ha/HAService.java](https://codecov.io/gh/apache/rocketmq/pull/3365/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2hhL0hBU2VydmljZS5qYXZh) | `71.14% <0.00%> (-0.66%)` | :arrow_down: |
   | [...rocketmq/broker/processor/PopMessageProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3365/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL1BvcE1lc3NhZ2VQcm9jZXNzb3IuamF2YQ==) | `36.95% <0.00%> (-0.55%)` | :arrow_down: |
   | [...main/java/org/apache/rocketmq/store/CommitLog.java](https://codecov.io/gh/apache/rocketmq/pull/3365/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL0NvbW1pdExvZy5qYXZh) | `66.46% <0.00%> (-0.11%)` | :arrow_down: |
   | ... and [150 more](https://codecov.io/gh/apache/rocketmq/pull/3365/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/3365?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3365?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [da5d30b...8c5035f](https://codecov.io/gh/apache/rocketmq/pull/3365?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


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

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

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