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