[GitHub] [helix] jiajunwang commented on a change in pull request #1607: Fix the IdealStateBuilder to set the correct instance group tag.

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

jiajunwang commented on a change in pull request #1607:
URL: https://github.com/apache/helix/pull/1607#discussion_r555320912



##########
File path: helix-core/src/main/java/org/apache/helix/model/builder/IdealStateBuilder.java
##########
@@ -85,12 +87,13 @@
    * A constraint that limits the maximum number of partitions per Node.
    */
   private int maxPartitionsPerNode;
+
   /**
    * Allocate the resource to nodes that are tagged with a specific "nodeGroup"
    * name. By default a resource will be allocated to all nodes registered to
    * the cluster.
    */
-  private String nodeGroup = "*";
+  private String nodeGroup = null;

Review comment:
       The original code is not fitting the comment. The reason why it is not failing is that the value is not passed.
   
   Please also help to double-check the code. I think "*" will cause the resource to be assigned onto the node with "*" tag. I don't see any wildcard fitting logic in the rebalancer. null is the safest option.




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