siddharthteotia commented on code in PR #8434:
URL: https://github.com/apache/pinot/pull/8434#discussion_r887197719
##########
pinot-common/src/main/java/org/apache/pinot/common/assignment/InstanceAssignmentConfigUtils.java:
##########
@@ -120,6 +120,6 @@ public static InstanceAssignmentConfig
getInstanceAssignmentConfig(TableConfig t
replicaGroupStrategyConfig.getNumInstancesPerPartition(), 0, 0,
minimizeDataMovement);
}
- return new InstanceAssignmentConfig(tagPoolConfig, null,
replicaGroupPartitionConfig);
+ return new InstanceAssignmentConfig(tagPoolConfig, null,
replicaGroupPartitionConfig, null);
Review Comment:
So one thing to think about is that within Li we have not yet migrated to
the new replica group assignment config. So none of our production tables
actually use `instanceAssignmentConfigMap`.
We still configure replica group assignment config through
ReplicaGroupStrategyConfig and the InstanceAssignmentDriver code takes care of
generating the `InstanceAssignmentConfig` and
`InstanceReplicaGroupPartitionConfig` in the above code.
So, when we roll out the FD aware instance assignment and start configuring
it, does that require us to first migrate to new config (which ideally we
should but let's discuss and let's start that soon) ? We will need code changes
in our internal tool etc that applies / updates table config changes.
Alternately, for the time-being until we migrate to new config we can piggy
back on the old replica group config to configure FD aware assignment.
I personally prefer the first option since old replica group config is like
debt at this point and adding new stuff to it is not desirable imo. So, we
rather use this as a forcing function to migrate to `InstanceAssignmentConfig`
and `InstanceReplicaGroupPartitionConfig` which is cleaner.
--
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]
