jackjlli commented on a change in pull request #8441:
URL: https://github.com/apache/pinot/pull/8441#discussion_r840888705
##########
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:
IMO we should only persist the instance sequence if it's required.
The actual value of `shouldRetainInstanceSequence` is determined by
`(existingPoolToInstancesMap != null)`. There are two cases need to
differentiate:
* If it's false, that means `retainInstanceSequence` is set to false in the
rebalance config. There is no need to retain instance sequence.
* If it's true but the map is empty, that means `retainInstanceSequence` is
set to true but there is no such map in the ZK yet.
Inside the driver, if `retainInstanceSequence` from rebalance config is
true, the existingPoolToInstancesMap should be fetched from ZK. The first fetch
would return empty map since there is no such map persisted in the ZK yet. But
since `shouldRetainInstanceSequence` is true because
`(existingPoolToInstancesMap != null)`, this new map will be persisted into the
ZK for the next calculation. So next time, when the same request comes, this
map can be reused.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
