J-HowHuang commented on code in PR #15110:
URL: https://github.com/apache/pinot/pull/15110#discussion_r1984248686


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/instance/InstanceAssignmentDriver.java:
##########
@@ -55,40 +55,64 @@ public InstanceAssignmentDriver(TableConfig tableConfig) {
 
   public InstancePartitions assignInstances(InstancePartitionsType 
instancePartitionsType,
       List<InstanceConfig> instanceConfigs, @Nullable InstancePartitions 
existingInstancePartitions) {
+    return assignInstances(instancePartitionsType, instanceConfigs, 
existingInstancePartitions, false);
+  }
+
+  public InstancePartitions assignInstances(InstancePartitionsType 
instancePartitionsType,
+      List<InstanceConfig> instanceConfigs, @Nullable InstancePartitions 
existingInstancePartitions,
+      boolean forceMinimizeDataMovement) {

Review Comment:
   @Jackie-Jiang Consider that enabling `minimizeDataMovement` is always 
encouraged, based on the philosophy I think for this API it's better to be kept 
like this (only to override false with true but not the other way round). 
   If anyone wants to override a table config that enables 
`minimizeDataMovement` with false by all means, he can modify the table config 
per se.



-- 
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: commits-unsubscr...@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to