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


##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/instance/InstanceAssignmentDriver.java:
##########
@@ -55,40 +55,65 @@ public InstanceAssignmentDriver(TableConfig tableConfig) {
 
   public InstancePartitions assignInstances(InstancePartitionsType 
instancePartitionsType,
       List<InstanceConfig> instanceConfigs, @Nullable InstancePartitions 
existingInstancePartitions) {
+    return assignInstances(instancePartitionsType, instanceConfigs, 
existingInstancePartitions, (Boolean) null);
+  }
+
+  public InstancePartitions assignInstances(InstancePartitionsType 
instancePartitionsType,
+      List<InstanceConfig> instanceConfigs, @Nullable InstancePartitions 
existingInstancePartitions,
+      @Nullable Boolean minimizeDataMovement) {
     String tableNameWithType = _tableConfig.getTableName();
     InstanceAssignmentConfig assignmentConfig =
         
InstanceAssignmentConfigUtils.getInstanceAssignmentConfig(_tableConfig, 
instancePartitionsType);
     return getInstancePartitions(
         
instancePartitionsType.getInstancePartitionsName(TableNameBuilder.extractRawTableName(tableNameWithType)),
-        assignmentConfig, instanceConfigs, existingInstancePartitions, null);
+        assignmentConfig, instanceConfigs, existingInstancePartitions, null, 
minimizeDataMovement);
   }
 
   public InstancePartitions assignInstances(InstancePartitionsType 
instancePartitionsType,
       List<InstanceConfig> instanceConfigs, @Nullable InstancePartitions 
existingInstancePartitions,
       @Nullable InstancePartitions preConfiguredInstancePartitions) {
+    return assignInstances(instancePartitionsType, instanceConfigs, 
existingInstancePartitions,
+        preConfiguredInstancePartitions, null);
+  }
+
+  public InstancePartitions assignInstances(InstancePartitionsType 
instancePartitionsType,
+      List<InstanceConfig> instanceConfigs, @Nullable InstancePartitions 
existingInstancePartitions,
+      @Nullable InstancePartitions preConfiguredInstancePartitions, @Nullable 
Boolean minimizeDataMovement) {
     String tableNameWithType = _tableConfig.getTableName();
     InstanceAssignmentConfig assignmentConfig =
         
InstanceAssignmentConfigUtils.getInstanceAssignmentConfig(_tableConfig, 
instancePartitionsType);
     return getInstancePartitions(
         
instancePartitionsType.getInstancePartitionsName(TableNameBuilder.extractRawTableName(tableNameWithType)),
-        assignmentConfig, instanceConfigs, existingInstancePartitions, 
preConfiguredInstancePartitions);
+        assignmentConfig, instanceConfigs, existingInstancePartitions, 
preConfiguredInstancePartitions,
+        minimizeDataMovement);
   }
 
   public InstancePartitions assignInstances(String tierName, 
List<InstanceConfig> instanceConfigs,
       @Nullable InstancePartitions existingInstancePartitions, 
InstanceAssignmentConfig instanceAssignmentConfig) {
+    return assignInstances(tierName, instanceConfigs, 
existingInstancePartitions, instanceAssignmentConfig, null);
+  }
+
+  public InstancePartitions assignInstances(String tierName, 
List<InstanceConfig> instanceConfigs,
+      @Nullable InstancePartitions existingInstancePartitions, 
InstanceAssignmentConfig instanceAssignmentConfig,
+      @Nullable Boolean minimizeDataMovement) {
     return getInstancePartitions(
         
InstancePartitionsUtils.getInstancePartitionsNameForTier(_tableConfig.getTableName(),
 tierName),
-        instanceAssignmentConfig, instanceConfigs, existingInstancePartitions, 
null);
+        instanceAssignmentConfig, instanceConfigs, existingInstancePartitions, 
null, minimizeDataMovement);
   }
 
   private InstancePartitions getInstancePartitions(String 
instancePartitionsName,
       InstanceAssignmentConfig instanceAssignmentConfig, List<InstanceConfig> 
instanceConfigs,
       @Nullable InstancePartitions existingInstancePartitions,
-      @Nullable InstancePartitions preConfiguredInstancePartitions) {
+      @Nullable InstancePartitions preConfiguredInstancePartitions, @Nullable 
Boolean minimizeDataMovementFlag) {
     String tableNameWithType = _tableConfig.getTableName();
-    LOGGER.info("Starting {} instance assignment for table {}", 
instancePartitionsName, tableNameWithType);
 
-    boolean minimizeDataMovement = 
instanceAssignmentConfig.isMinimizeDataMovement();
+    // minimizeDataMovement might be set back to false within 
InstanceTagPoolSelector and InstancePartitionSelector

Review Comment:
   no, this is still relevant. minimizeDataMovement might not be carried out if 
there's no existing instance partition or when `bootstrap=true`. See 
https://github.com/apache/pinot/blob/8dab53ae4efd4475cbd23b7b808fb684018a4cb0/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/instance/InstanceTagPoolSelector.java#L55



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