[GitHub] [rocketmq] dongeforever commented on a diff in pull request #4383: [ISSUE #4382]Namesrv nearby route

[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