[GitHub] [helix] dasahcc commented on a change in pull request #1619: Allow cutomizing default rebalance configuration for the supercluster resources.

[GitBox <gi...@apache.org>]

dasahcc commented on a change in pull request #1619:
URL: https://github.com/apache/helix/pull/1619#discussion_r568169224



##########
File path: helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java
##########
@@ -134,7 +137,10 @@
     // offline for more than this specified time period, and users call purge participant API,
     // then the node will be removed.
     // The unit is milliseconds.
-    OFFLINE_DURATION_FOR_PURGE_MS
+    OFFLINE_DURATION_FOR_PURGE_MS,
+
+    // Resource rebalance configuration for the super cluster which manages Helix controller resources.
+    SUPER_CLUSTER_REBALANCE_CONFIG

Review comment:
       This could be confusing. Because other sub clusters can have this entry. What does it mean to the sub cluster? If this is only designed for super cluster. Would this be meaningful in ClusterConfig?
   
   I have concerns to add it as specific for super cluster IS rebalance strategy. We should make it generic. Either we add it as default rebalance strategy for existing cluster. Or let's make the the configurable passing down from the admin API every time.

##########
File path: helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java
##########
@@ -1018,6 +1032,22 @@ public void setAbnormalStateResolverMap(Map<String, String> resolverMap) {
     return idealStateRuleMap;
   }
 
+  public void setSuperClusterRebalanceConfig(RebalanceConfig rebalanceConfig) {
+    _record.setMapField(ClusterConfigProperty.SUPER_CLUSTER_REBALANCE_CONFIG.name(),
+        rebalanceConfig.getConfigsMap());
+  }
+
+  public RebalanceConfig getSuperClusterRebalanceConfig() {
+    Map<String, String> rebalanceConfigMap =
+        _record.getMapField(ClusterConfigProperty.SUPER_CLUSTER_REBALANCE_CONFIG.name());
+    if (rebalanceConfigMap != null) {
+      return new RebalanceConfig(rebalanceConfigMap);
+    } else {

Review comment:
       +1 to @NealSun96 . I would suggest to follow the coding convention.




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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org