siddharthteotia commented on a change in pull request #8112:
URL: https://github.com/apache/pinot/pull/8112#discussion_r798248291
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/assignment/InstanceAssignmentConfigUtils.java
##########
@@ -105,9 +106,11 @@ public static InstanceAssignmentConfig
getInstanceAssignmentConfig(TableConfig t
int numReplicaGroups = segmentConfig.getReplicationNumber();
ReplicaGroupStrategyConfig replicaGroupStrategyConfig =
segmentConfig.getReplicaGroupStrategyConfig();
Preconditions.checkState(replicaGroupStrategyConfig != null, "Failed to
find the replica-group strategy config");
+ SegmentPartitionConfig segmentPartitionConfig =
tableConfig.getIndexingConfig().getSegmentPartitionConfig();
Review comment:
Thanks. I don't think anything needs to be changed here. There are 3
scenarios:
- If the table is not partitioned - from legacy config,
`numInstancesPerPartition` becomes `numInstancesPerReplicaGroup` in the new way
of doing things.
- If the table is partitioned but not partitioned at the replica group level
- partitionColumn in legacy config is null. This is same as previous scenario
because replica group config does not get impacted
- If the table is partitioned and also partitioned at the replica group
level - partitionColumn in legacy config is non-null.
`numInstancesPerPartition` from legacy config is retained in the new config.
`numPartitions` comes from `segmentPartitionConfig`.
numInstancesPerReplicaGroup does not matter.
These are already handled.
--
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]
