jasperjiaguo edited a comment on pull request #8441: URL: https://github.com/apache/pinot/pull/8441#issuecomment-1086320784
> > > I don't follow why we need to store the pool to instances mapping in the ZK. Essentially what we need is to pass the existing instance partitions to the `InstanceReplicaGroupPartitionSelector.selectInstances()`, and add a flag to enable `minimizeMovement` and use a greedy algorithm to retain as much instances as possible from the candidate instances from each pool. > > > > > > Yes the initial plan is to reuse the existing instance partitions, but the thing is that for the instance which no longer exists in the current list of instance configs (which is gone), there is no way to know which pool it came from; i.e. even though we know there is an empty seat there, we don't know which pool we should select the new instance from. As long as we can control the input parameters of `replicaPartitionSelector.selectInstances()` (the input instancePartitions is always empty, thus as long as we control the instance sequence of `poolToInstanceConfigsMap`), we can always get the deterministic instancePartitions as output. > > Actually we can know the pool where the original instance is coming from without storing the pool information. The pool selection is deterministic, and is based on the hash code of the table name. Following the same algorithm, for each replica id, you can get the same pool id as the original assignment. Then you may run the greedy algorithm to pick the instances from the candidate instances from the pool to achieve minimum movement. IMO there is no hard requirement for a pool id to be mapped ~~1:1~~ 1:N to a replica id, right? It's just in current strategy of `InstanceReplicaGroupPartitionSelector` we assign instances from a pool to a replica. But this should not be enforced for future use, especially right now we are implementing selector with FD awareness and it can have instances from multiple pools in 1 replica group. In other words we should not rely on the status-quo of we can "reverse engineering the pool id from replica group id". So I think the pool -> instance mapping should probably be saved. -- 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
