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