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 2022/07/01 05:53:10 UTC

[GitHub] [rocketmq] lwclover opened a new pull request, #4383: [ISSUE #4382]Namesrv nearby route

lwclover opened a new pull request, #4383:
URL: https://github.com/apache/rocketmq/pull/4383

   ### RocketMQ同机房就近生产和消费机制。
   
   
   RocketMQ注册、发现机制
   - Broker
     - 间隔30秒发送broker上的Topic全量信息到所有Nameserv。broker启动的时候也是延迟注册到Namesrv上的。当有topic发生变更时TopicConfigManager.dataVersion发生变更。
     - 当有topic创建和更新时，broker立刻发送Topic增量信息到所有Namesrv。同时触发Namesrv的BrokerLiveInfo.dataVersion发生变更。
   - Namesrv
     - 保存集群和broker关系、broker信息（brokerName和地址）、broker真实连接和topic版本号，topic信息。
     - 调度线程每10秒钟检查一次，如果发现一个broker 120秒都没有更新注册信息，则删除和这个broker相关的所有信息。
   - Producer&Consumer
     - Producer第一次发送Message时，会同步向namesrv请求topic路由信息
     - Consumer启动过程中会向namesrv请求topic路由信息
     - 每隔30秒向namesrv拉取一次topic路由信息，根据路由信息内容（不是版本号）判断是否变化，发生变化更新
   - 总结：
   Producer第一次发送消息时会同步向namesrv请求topic路由信息，然后默认轮询往每个Queue发送消息。
   Consumer启动时获取topic路由信息，然后请求broker获取consumer实例列表，最后根据consumer的数量和负载均衡算法给consumer分配queue。
   
   ### 实现rocketmq同机房就近生产和消费：
   1.broker增加一个zoneName标识配置，通过heartbeat注册到namesrv上。
   2.Producer和Consumer可以通过API或者环境变量、系统变量设置zoneName和zoneMode
   3.Producer和Consumer获取topic路由信息时，在namesrv上返回对应标识的broker信息
   
   ![image](https://user-images.githubusercontent.com/4252409/170472929-85f56a48-ab10-4bbc-b241-72029256b5ec.png)
   
   
   ![image](https://user-images.githubusercontent.com/4252409/170473144-8728e41d-4d69-4248-b690-0fbec426ccbe.png)
   


-- 
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] lwclover commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
lwclover commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1171823290

   > @lwclover This solution is enough for an inner network inside an organization. It may not be enough for common use cases.
   > 
   > And this issue is so important that it is worth adding a config in the client/broker.
   > 
   > It will nice if you could move forward with a more general solution and I'm glad to help you.
   > 
   > IMO, a RPCHook will be a nice way to add the extField to the API Headers.
   
   The optimized version has been submitted


-- 
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] dongeforever commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
dongeforever commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1154883293

   @lwclover I have read all the code. This solution is simple and useful in some cases.
   
   And the same time, there are two potential problems.
   
   Firstly,  in the public cloud or hybrid cloud,  with SLB/PrivateLink, the network may be complicated, and we could identify the consumer by their remote address.  So we may need a more common way to identify the client and broker. For example, we may abstract a config named "site", and the client/broker inject the "site" info to the extFields of RemotingCommand.
   
   Secondly, we may need a way to solve the conflict between consumers of the same group in different networks. For example, some consumers on site A, some consumers on site global.


-- 
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] lwclover commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
lwclover commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1161633919

   > @lwclover I have read all the code. This solution is simple and useful in some cases.
   > 
   > And the same time, there are two potential problems.
   > 
   > Firstly, in the public cloud or hybrid cloud, with SLB/PrivateLink, the network may be complicated, and we may not able to identify the client/broker by their remote address. So we may need a more common way to identify the client/broker. For example, we could abstract a config named "site", and the client/broker inject the "site" info to the extFields of RemotingCommand.
   > 
   > Secondly, we may need a way to solve the conflict between consumers of the same group in different networks. For example, some consumers on site A, some consumers on site global.
   
   @dongeforever Thank you for reviewing my code.
   For your first suggestion, My goal is to be transparent to the client，if I  abstract a config named "site", client/broker need config it.
   
   For your sencod suggestion,I agree with you.
   


-- 
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] dongeforever merged pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
dongeforever merged PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383


-- 
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] lwclover commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
lwclover commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1178732762

   > LGTM， but would you like to rebase your codebase first? it seems this PR contained so many other's commits
   
   Next time. haha


-- 
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] lwclover commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
lwclover commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1173651381

   > 
   
   OK，I will do 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.

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

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

[GitHub] [rocketmq] lwclover commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
lwclover commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1171963337

   > Yes，That's how I implemented it, I just didn't change the description of the 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] coveralls commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

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

   
   [![Coverage Status](https://coveralls.io/builds/49482759/badge)](https://coveralls.io/builds/49482759)
   
   Coverage decreased (-0.5%) to 51.785% when pulling **ad13689f548b34335b37ac8ba05a418980258c66 on lwclover:develop** into **af011b1e2d4395a5619ba9ffd27769d4d5ecdd19 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.

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

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

[GitHub] [rocketmq] haqiaolong commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
haqiaolong commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1139231155

   I finally found this function. What a surprise


-- 
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] duhenglucky commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
duhenglucky commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1178625912

   LGTM， but would you like to rebase your codebase first? it seems this PR contained so many other's commits


-- 
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] dongeforever commented on a diff in pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
dongeforever commented on code in PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#discussion_r916507881


##########
common/src/main/java/org/apache/rocketmq/common/protocol/RequestCode.java:
##########
@@ -194,4 +194,5 @@ public class RequestCode {
     public static final int GET_ALL_PRODUCER_INFO = 328;
 
     public static final int DELETE_EXPIRED_COMMITLOG = 329;
+    
 }

Review Comment:
   remove these trivial changes.



##########
broker/src/main/java/org/apache/rocketmq/broker/plugin/MessageStoreFactory.java:
##########
@@ -1,45 +1,45 @@
-/*

Review Comment:
   Why this file is labeled as changed?
   
   Maybe some newline delimiter conflicts.



##########
common/src/main/java/org/apache/rocketmq/common/protocol/route/BrokerData.java:
##########
@@ -29,17 +29,18 @@ public class BrokerData implements Comparable<BrokerData> {
     private String cluster;
     private String brokerName;
     private HashMap<Long/* brokerId */, String/* broker address */> brokerAddrs;
-
+    private String zoneName;
     private final Random random = new Random();
 
     public BrokerData() {
 
     }
 
-    public BrokerData(String cluster, String brokerName, HashMap<Long, String> brokerAddrs) {

Review Comment:
   Keep the old constructor, and let it be compatible.



##########
common/src/main/java/org/apache/rocketmq/common/rpchook/DynamicalExtFieldRPCHook.java:
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.rocketmq.common.rpchook;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.rocketmq.common.MixAll;
+import org.apache.rocketmq.remoting.RPCHook;
+import org.apache.rocketmq.remoting.protocol.RemotingCommand;
+
+public class DynamicalExtFieldRPCHook implements RPCHook {
+
+    @Override
+    public void doBeforeRequest(String remoteAddr, RemotingCommand request) {
+        String zoneName = System.getProperty(MixAll.ROCKETMQ_ZONE_PROPERTY, System.getenv(MixAll.ROCKETMQ_ZONE_ENV));
+        if (StringUtils.isNotBlank(zoneName)) {
+            request.addExtField(MixAll.ZONE_NAME, System.getProperty(MixAll.ROCKETMQ_ZONE_PROPERTY, System.getenv(MixAll.ROCKETMQ_ZONE_ENV)));

Review Comment:
   use zoneName instead of duplicated getting code.



-- 
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] dongeforever commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
dongeforever commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1162861925

   @lwclover This solution is enough for an inner network inside an organization. It may not be enough for common use cases.
   
   And this issue is so important that it is worth adding a config in the client/broker.
   
   It will nice if you could move forward with a more general solution and I'm glad to help you.
   
   IMO, a RPCHook will be a nice way to add the extField to the API Headers.
   
   


-- 
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] lwclover closed pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
lwclover closed pull request #4383: [ISSUE #4382]Namesrv nearby route
URL: https://github.com/apache/rocketmq/pull/4383


-- 
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 #4383: [ISSUE #4382]Namesrv nearby route

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

   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4383?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 [#4383](https://codecov.io/gh/apache/rocketmq/pull/4383?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ad13689) into [develop](https://codecov.io/gh/apache/rocketmq/commit/af011b1e2d4395a5619ba9ffd27769d4d5ecdd19?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (af011b1) will **decrease** coverage by `0.23%`.
   > The diff coverage is `26.31%`.
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #4383      +/-   ##
   =============================================
   - Coverage      48.20%   47.96%   -0.24%     
   + Complexity      5089     5085       -4     
   =============================================
     Files            642      648       +6     
     Lines          42780    42968     +188     
     Branches        5597     5621      +24     
   =============================================
   - Hits           20622    20611      -11     
   - Misses         19652    19855     +203     
   + Partials        2506     2502       -4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/4383?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...a/org/apache/rocketmq/broker/BrokerController.java](https://codecov.io/gh/apache/rocketmq/pull/4383/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvQnJva2VyQ29udHJvbGxlci5qYXZh) | `46.63% <ø> (ø)` | |
   | [...g/apache/rocketmq/client/impl/MQClientAPIImpl.java](https://codecov.io/gh/apache/rocketmq/pull/4383/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9NUUNsaWVudEFQSUltcGwuamF2YQ==) | `13.74% <0.00%> (-0.24%)` | :arrow_down: |
   | [...g/apache/rocketmq/common/protocol/RequestCode.java](https://codecov.io/gh/apache/rocketmq/pull/4383/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvUmVxdWVzdENvZGUuamF2YQ==) | `0.00% <ø> (ø)` | |
   | [.../org/apache/rocketmq/common/route/NearbyRoute.java](https://codecov.io/gh/apache/rocketmq/pull/4383/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcm91dGUvTmVhcmJ5Um91dGUuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...java/org/apache/rocketmq/common/route/Network.java](https://codecov.io/gh/apache/rocketmq/pull/4383/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcm91dGUvTmV0d29yay5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [.../apache/rocketmq/common/utils/ServiceProvider.java](https://codecov.io/gh/apache/rocketmq/pull/4383/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vdXRpbHMvU2VydmljZVByb3ZpZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...tmq/namesrv/processor/DefaultRequestProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/4383/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-bmFtZXNydi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbmFtZXNydi9wcm9jZXNzb3IvRGVmYXVsdFJlcXVlc3RQcm9jZXNzb3IuamF2YQ==) | `69.64% <0.00%> (-4.65%)` | :arrow_down: |
   | [...apache/rocketmq/tools/admin/DefaultMQAdminExt.java](https://codecov.io/gh/apache/rocketmq/pull/4383/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-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2FkbWluL0RlZmF1bHRNUUFkbWluRXh0LmphdmE=) | `40.45% <0.00%> (-0.95%)` | :arrow_down: |
   | [...he/rocketmq/tools/admin/DefaultMQAdminExtImpl.java](https://codecov.io/gh/apache/rocketmq/pull/4383/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-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2FkbWluL0RlZmF1bHRNUUFkbWluRXh0SW1wbC5qYXZh) | `39.59% <0.00%> (-0.25%)` | :arrow_down: |
   | [.../apache/rocketmq/tools/command/MQAdminStartup.java](https://codecov.io/gh/apache/rocketmq/pull/4383/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-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2NvbW1hbmQvTVFBZG1pblN0YXJ0dXAuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | ... and [23 more](https://codecov.io/gh/apache/rocketmq/pull/4383/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/4383?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/4383?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 [af011b1...ad13689](https://codecov.io/gh/apache/rocketmq/pull/4383?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

[GitHub] [rocketmq] lwclover closed pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
lwclover closed pull request #4383: [ISSUE #4382]Namesrv nearby route
URL: https://github.com/apache/rocketmq/pull/4383


-- 
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] lwclover commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
lwclover commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1171962487

   > IMO, we can refer to affinity/anti-affinity in k8s. each broker can be labeled some tag, such as idc, rack and so on. namesrv hold broker's tag info through heartbeat. And then, client request route with tags and tolerations through rpchook.
   
   Yes，That's how I implemented it, I just didn't change the description of the 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] dongeforever commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
dongeforever commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1178618110

   @lwclover It seems ok. 
   
   BTW, some trivial code polishments could be done to make the code clear.


-- 
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] zyl0622 commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
zyl0622 commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1139209332

   good job


-- 
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] dongeforever commented on a diff in pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
dongeforever commented on code in PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#discussion_r912596976


##########
common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java:
##########
@@ -195,6 +195,8 @@ public class BrokerConfig {
      * Whether to distinguish log paths when multiple brokers are deployed on the same machine
      */
     private boolean isolateLogEnable = false;
+    @ImportantField
+    private String zoneName;

Review Comment:
   Currently, use the rpchook instead.
   
   We need more time to prove this PR's general value.



##########
client/src/main/java/org/apache/rocketmq/client/route/hook/AddZoneRPCHook.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.rocketmq.client.route.hook;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.rocketmq.common.MixAll;
+import org.apache.rocketmq.common.protocol.RequestCode;
+import org.apache.rocketmq.remoting.RPCHook;
+import org.apache.rocketmq.remoting.protocol.RemotingCommand;
+
+public class AddZoneRPCHook implements RPCHook {
+
+    public static final AddZoneRPCHook INSTANCE = new AddZoneRPCHook();
+    /**
+     * The name of zone to which producer or consumer belong.
+     */
+    private String zoneName = System.getProperty(MixAll.ROCKETMQ_CLIENT_ZONE_PROPERTY, System.getenv(MixAll.ROCKETMQ_CLIENT_ZONE_ENV));
+    /**

Review Comment:
   How about adding more loading mechanisms for zone name configurations?
   
   In some cases, it is important to load the value dynamically. 
   
   For example, load from /etc/rocketmq/extFields.conf  dynamicaly.
   
   And this RPCHook is named to DynamicalExtFieldRPCHook.
   



##########
common/src/main/java/org/apache/rocketmq/common/MixAll.java:
##########
@@ -90,6 +90,13 @@ public class MixAll {
     public static final String LMQ_PREFIX = "%LMQ%";
     public static final String MULTI_DISPATCH_QUEUE_SPLITTER = ",";
     public static final String REQ_T = "ReqT";
+    public static final String ROCKETMQ_CLIENT_ZONE_ENV = "ROCKETMQ_CLIENT_ZONE";
+    public static final String ROCKETMQ_CLIENT_ZONE_PROPERTY = "rocketmq.client.zone";
+    public static final String ROCKETMQ_CLIENT_ZONE_MODE_ENV = "ROCKETMQ_CLIENT_ZONE_MODE";
+    public static final String ROCKETMQ_CLIENT_ZONE_MODE_PROPERTY = "rocketmq.client.zone.mode";
+    public static final String ZONE_NAME = "zoneName"; 
+    public static final String ZONE_MODE = "zoneMode";
+

Review Comment:
   To avoid the conflict between normal header fields and extent fields, how about using the prefix "__" to identify the extend fields?



-- 
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] duhenglucky commented on a diff in pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
duhenglucky commented on code in PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#discussion_r911549726


##########
client/src/main/java/org/apache/rocketmq/client/ClientConfig.java:
##########
@@ -340,6 +341,22 @@ public void setEnableStreamRequestType(boolean enableStreamRequestType) {
         this.enableStreamRequestType = enableStreamRequestType;
     }
 
+    public void setZoneName(String zoneName) {

Review Comment:
   It is very difficult to let customers to upgrade the client version. If this zone information can be obtained through ip or other environmental information, it will be more friendly



-- 
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] dongeforever commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
dongeforever commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1173300806

   @lwclover Currently, we need to make the feature simple enough and prove it along the time.
   In the client and broker, we just add a general rpcHook and do not introduce explicit fields. 
   Write all the logic inside the nameserver to control the code population.
   
   BTW, the rpcHook could be a general DynamicalExtFieldRPCHook, loading the extent fields from -D, env, etc files.
   


-- 
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] zhouxinyu commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
zhouxinyu commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1171852379

   I think we need more discussions about this feature and implementation. I have two points:
   1. Is there any chance to implement this feature through the client hooks, e.g., `QueueSelector` for the producer and `AllocateMessageQueueStrategy` for the consumer.
   2. The associated implementation seems to introduce more complexity for both client and server, if we really want to bring in this feature, can we consider only implementing it on the server-side, e.g., control the topic route info from namesrv follow the az info of clients? 


-- 
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] lizhiboo commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
lizhiboo commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1171889943

   IMO, we can refer to affinity/anti-affinity in k8s. each broker can be labeled some tag, such as idc, rack and so on. namesrv hold broker's tag info through heartbeat. And then, client request route with tags and tolerations through rpchook.


-- 
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] lwclover commented on pull request #4383: [ISSUE #4382]Namesrv nearby route

Posted by GitBox <gi...@apache.org>.
lwclover commented on PR #4383:
URL: https://github.com/apache/rocketmq/pull/4383#issuecomment-1171964067

   > I think we need more discussions about this feature and implementation. I have two points:
   > 
   > 1. Is there any chance to implement this feature through the client hooks, e.g., `QueueSelector` for the producer and `AllocateMessageQueueStrategy` for the consumer.
   > 2. The associated implementation seems to introduce more complexity for both client and server, if we really want to bring in this feature, can we consider only implementing it on the server-side, e.g., control the topic route info from namesrv follow the az info of clients?
   
   My original intention was that the user would not need to pay attention to such intricate details


-- 
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