jasperjiaguo commented on a change in pull request #8441:
URL: https://github.com/apache/pinot/pull/8441#discussion_r840924957
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/instance/InstanceAssignmentDriver.java
##########
@@ -77,7 +95,31 @@ public InstancePartitions
assignInstances(InstancePartitionsType instancePartiti
new
InstanceReplicaGroupPartitionSelector(assignmentConfig.getReplicaGroupPartitionConfig(),
tableNameWithType);
InstancePartitions instancePartitions = new InstancePartitions(
instancePartitionsType.getInstancePartitionsName(TableNameBuilder.extractRawTableName(tableNameWithType)));
+ if (shouldRetainInstanceSequence) {
+ // Keep the pool to instances map if instance sequence should be
retained.
+ instancePartitions
+
.setPoolToInstancesMap(extractInstanceNamesFromPoolToInstanceConfigsMap(poolToInstanceConfigsMap));
+ }
Review comment:
I'm wondering would it be better if we can have one config instead of
two to avoid some confusion. I mean as a user I might expect the
`retainInstanceSequence` in rebalance to retain current sequence, whereas it's
actually a "best effort" depending on if the corresponding flag is set in the
initial assignment.
Similarly for current large use-cases without this map already available we
might need some migration plan?
--
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
