jackjlli commented on a change in pull request #8441:
URL: https://github.com/apache/pinot/pull/8441#discussion_r839303449
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/instance/InstanceAssignmentDriver.java
##########
@@ -51,16 +52,34 @@ public InstanceAssignmentDriver(TableConfig tableConfig) {
_tableConfig = tableConfig;
}
+ /**
+ * Assign instances to InstancePartitions object.
+ * @param instancePartitionsType type of instance partitions
+ * @param instanceConfigs list of instance configs
+ * @param partitionToInstancesMap existing instance with sequence that
should be respected. An empty list
+ * means no preceding sequence to
respect and the instances would be sorted.
+ */
public InstancePartitions assignInstances(InstancePartitionsType
instancePartitionsType,
- List<InstanceConfig> instanceConfigs) {
+ List<InstanceConfig> instanceConfigs, Map<Integer, List<String>>
partitionToInstancesMap) {
+ boolean shouldRetainInstanceSequence = !partitionToInstancesMap.isEmpty();
String tableNameWithType = _tableConfig.getTableName();
- LOGGER.info("Starting {} instance assignment for table: {}",
instancePartitionsType, tableNameWithType);
+ LOGGER.info("Starting {} instance assignment for table: {}. Should retain
instance sequence: {}",
+ instancePartitionsType, tableNameWithType,
shouldRetainInstanceSequence);
InstanceAssignmentConfig assignmentConfig =
InstanceAssignmentConfigUtils.getInstanceAssignmentConfig(_tableConfig,
instancePartitionsType);
InstanceTagPoolSelector tagPoolSelector =
new InstanceTagPoolSelector(assignmentConfig.getTagPoolConfig(),
tableNameWithType);
- Map<Integer, List<InstanceConfig>> poolToInstanceConfigsMap =
tagPoolSelector.selectInstances(instanceConfigs);
+ Map<Integer, List<InstanceConfig>> poolToInstanceConfigsMap =
+ tagPoolSelector.selectInstances(instanceConfigs,
partitionToInstancesMap);
+
+ InstancePartitions instancePartitions = new InstancePartitions(
+
instancePartitionsType.getInstancePartitionsName(TableNameBuilder.extractRawTableName(tableNameWithType)));
+ if (shouldRetainInstanceSequence) {
+ // Keep the partition to instances map if instance sequence should be
retained.
+ instancePartitions
+
.setPartitionToInstancesMap(extractInstanceNamesFromPoolToInstanceConfigsMap(poolToInstanceConfigsMap));
+ }
InstanceConstraintConfig constraintConfig =
assignmentConfig.getConstraintConfig();
Review comment:
Yes I did think of extending the `InstanceConstraintApplier`, but I do
want to keep the current logic if the `existingPoolToInstancesMap` is not
given. Plus, it'd be better to store the instance with the sequence before the
rotation, as it will always return the deterministic list of instance if the
input instance sequence is fixed. And I don't like the idea of overriding the
existing method as it'd be hard to maintain two copies of code if some changes
is needed.
--
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
