somandal commented on code in PR #15838:
URL: https://github.com/apache/pinot/pull/15838#discussion_r2096567187
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/TableRebalancer.java:
##########
@@ -1581,84 +1581,61 @@ private static Map<String, Map<String, String>>
getNextStrictReplicaGroupAssignm
Logger tableRebalanceLogger) {
Map<String, Map<String, String>> nextAssignment = new TreeMap<>();
Map<String, Integer> numSegmentsToOffloadMap =
getNumSegmentsToOffloadMap(currentAssignment, targetAssignment);
- Map<Integer, Map<Set<String>, Map<String, Map<String, String>>>>
- partitionIdToAssignedInstancesToCurrentAssignmentMap;
- if (batchSizePerServer == RebalanceConfig.DISABLE_BATCH_SIZE_PER_SERVER) {
- // Don't calculate the partition id to assigned instances to current
assignment mapping if batching is disabled
- // since we want to update the next assignment based on all partitions
in this case. Use partitionId as 0
- // and a dummy set for the assigned instances.
- partitionIdToAssignedInstancesToCurrentAssignmentMap = new TreeMap<>();
- partitionIdToAssignedInstancesToCurrentAssignmentMap.put(0, new
HashMap<>());
-
partitionIdToAssignedInstancesToCurrentAssignmentMap.get(0).put(Set.of(""),
currentAssignment);
- } else {
- partitionIdToAssignedInstancesToCurrentAssignmentMap =
-
getPartitionIdToAssignedInstancesToCurrentAssignmentMap(currentAssignment,
segmentPartitionIdMap,
- partitionIdFetcher);
- }
Map<Pair<Set<String>, Set<String>>, Set<String>> assignmentMap = new
HashMap<>();
Map<Set<String>, Set<String>> availableInstancesMap = new HashMap<>();
-
Map<String, Integer> serverToNumSegmentsAddedSoFar = new HashMap<>();
- for (Map<Set<String>, Map<String, Map<String, String>>>
assignedInstancesToCurrentAssignment
- : partitionIdToAssignedInstancesToCurrentAssignmentMap.values()) {
- boolean anyServerExhaustedBatchSize = false;
- if (batchSizePerServer != RebalanceConfig.DISABLE_BATCH_SIZE_PER_SERVER)
{
- // The number of segments for a given partition, accumulates as we
iterate over the assigned instances
- Map<String, Integer> serverToNumSegmentsToBeAddedForPartitionMap = new
HashMap<>();
-
- // Check if the servers of the first assignment for each unique set of
assigned instances has any space left
- // to move this partition. If so, let's mark the partitions as to be
moved, otherwise we mark the partition
- // as a whole as not moveable.
- for (Map<String, Map<String, String>> curAssignment :
assignedInstancesToCurrentAssignment.values()) {
- Map.Entry<String, Map<String, String>> firstEntry =
curAssignment.entrySet().iterator().next();
- // It is enough to check for whether any server for one segment is
above the limit or not since all segments
- // in curAssignment will have the same assigned instances list
- Map<String, String> firstEntryInstanceStateMap =
firstEntry.getValue();
- SingleSegmentAssignment firstAssignment =
- getNextSingleSegmentAssignment(firstEntryInstanceStateMap,
targetAssignment.get(firstEntry.getKey()),
- minAvailableReplicas, lowDiskMode, numSegmentsToOffloadMap,
assignmentMap);
- Set<String> serversAdded =
getServersAddedInSingleSegmentAssignment(firstEntryInstanceStateMap,
- firstAssignment._instanceStateMap);
+
+ Map<Pair<Set<String>, Set<String>>, Map<Integer, Map<String, Map<String,
String>>>>
+ currentAndTargetInstancesToPartitionIdToCurrentAssignmentMap;
+ if (batchSizePerServer == RebalanceConfig.DISABLE_BATCH_SIZE_PER_SERVER) {
Review Comment:
good point, makes the code much neater. done
--
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
