GSharayu commented on code in PR #9309:
URL: https://github.com/apache/pinot/pull/9309#discussion_r970324574
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/BaseSegmentAssignment.java:
##########
@@ -93,62 +96,22 @@ public void init(HelixManager helixManager, TableConfig
tableConfig) {
protected abstract int getReplication(TableConfig tableConfig);
/**
- * Helper method to check whether the number of replica-groups matches the
table replication for replica-group based
- * instance partitions. Log a warning if they do not match and use the one
inside the instance partitions. The
- * mismatch can happen when table is not configured correctly (table
replication and numReplicaGroups does not match
- * or replication changed without reassigning instances).
+ * Set Segment assignment strategy for different instance partitions and
puts into a map of
+ * Map<InstancePartitionsType, SegmentAssignmentStrategy>
*/
- protected void checkReplication(InstancePartitions instancePartitions) {
- int numReplicaGroups = instancePartitions.getNumReplicaGroups();
- if (numReplicaGroups != _replication) {
- _logger.warn(
- "Number of replica-groups in instance partitions {}: {} does not
match replication in table config: {} for "
- + "table: {}, using: {}",
instancePartitions.getInstancePartitionsName(), numReplicaGroups, _replication,
- _tableNameWithType, numReplicaGroups);
- }
- }
-
- /**
- * Helper method to assign instances based on the current assignment and
instance partitions.
- */
- protected List<String> assignSegment(String segmentName, Map<String,
Map<String, String>> currentAssignment,
- InstancePartitions instancePartitions) {
- int numReplicaGroups = instancePartitions.getNumReplicaGroups();
- int numPartitions = instancePartitions.getNumPartitions();
-
- if (numReplicaGroups == 1 && numPartitions == 1) {
- // Non-replica-group based assignment
-
- return
SegmentAssignmentUtils.assignSegmentWithoutReplicaGroup(currentAssignment,
instancePartitions,
- _replication);
- } else {
- // Replica-group based assignment
-
- checkReplication(instancePartitions);
-
- int partitionId;
- if (_partitionColumn == null || numPartitions == 1) {
- partitionId = 0;
- } else {
- // Uniformly spray the segment partitions over the instance partitions
- partitionId = getSegmentPartitionId(segmentName) % numPartitions;
- }
-
- return
SegmentAssignmentUtils.assignSegmentWithReplicaGroup(currentAssignment,
instancePartitions, partitionId);
- }
+ protected void setSegmentAssignmentStrategyMap(
+ Map<InstancePartitionsType, InstancePartitions> instancePartitionsMap) {
+ _assignmentStrategyMap = SegmentAssignmentStrategyFactory
+ .getSegmentAssignmentStrategy(_helixManager, _tableConfig,
instancePartitionsMap);
Review Comment:
Yes, Initially we were setting assignment strategy in Offline and Realtime
segment assignment and getting that from factory and the naming was misaligned.
We have refactored that part of code. So naming will now not be conflicting
--
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
