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