xyuanlu commented on code in PR #2986:
URL: https://github.com/apache/helix/pull/2986#discussion_r1927802152
##########
helix-core/src/main/java/org/apache/helix/cloud/topology/FaultZoneBasedVirtualGroupAssignmentAlgorithm.java:
##########
@@ -59,137 +59,127 @@ public static
FaultZoneBasedVirtualGroupAssignmentAlgorithm getInstance() {
@Override
public Map<String, Set<String>> computeAssignment(int numGroups, String
virtualGroupName,
- Map<String, Set<String>> zoneMapping, Map<String, Set<String>>
virtualZoneMapping) {
- // Build instance-to-zone mapping for quick zone lookups.
- Map<String, String> instanceToZoneMapping = new HashMap<>();
- for (Map.Entry<String, Set<String>> entry : zoneMapping.entrySet()) {
- for (String instance : entry.getValue()) {
- instanceToZoneMapping.put(instance, entry.getKey());
- }
- }
- // Collect all instances from zoneMapping
- Set<String> allInstances = zoneMapping.values()
- .stream()
- .flatMap(Set::stream)
- .collect(Collectors.toSet());
-
+ Map<String, Set<String>> zoneMapping, Map<String, Set<String>>
virtualGroupToInstancesMap) {
// 1. If the number of requested virtual groups differs from the current
assignment size,
// we must do a fresh assignment (the existing distribution is invalid).
- if (numGroups != virtualZoneMapping.size()) {
+ if (numGroups != virtualGroupToInstancesMap.size()) {
Map<String, Set<String>> newAssignment = new HashMap<>();
for (int i = 0; i < numGroups; i++) {
newAssignment.put(computeVirtualGroupId(i, virtualGroupName), new
HashSet<>());
}
- // Assign all instances from scratch in a balanced manner.
- return distributeUnassignedZones(newAssignment, allInstances,
instanceToZoneMapping);
+ // Assign all zones from scratch in a balanced manner.
+ distributeUnassignedZones(newAssignment, new
ArrayList<>(zoneMapping.keySet()), zoneMapping);
+ return constructResult(newAssignment, zoneMapping);
}
- // Build a deep copy of the current assignment to avoid mutating it
directly.
- Map<String, Set<String>> updatedAssignment = deepCopy(virtualZoneMapping);
-
- // 2. Identify unassigned instances.
- // Based on the existing assignment, build zone -> virtual group mapping
for quick reference.
- Map<String, String> zoneToVirtualGroupMapping = new HashMap<>();
- Set<String> assignedInstances = new HashSet<>();
- for (Map.Entry<String, Set<String>> entry : updatedAssignment.entrySet()) {
+ // Build instance-to-zone mapping for quick zone lookups.
+ Map<String, String> instanceToZoneMapping = new HashMap<>();
+ for (Map.Entry<String, Set<String>> entry : zoneMapping.entrySet()) {
for (String instance : entry.getValue()) {
- String zone = instanceToZoneMapping.get(instance);
- assignedInstances.add(instance);
- zoneToVirtualGroupMapping.put(zone, entry.getKey());
+ instanceToZoneMapping.put(instance, entry.getKey());
}
}
- Set<String> unassigned = new HashSet<>(allInstances);
- unassigned.removeAll(assignedInstances);
-
- // 3. Attempt to place unassigned instances in existing virtual groups if
their zones are
- // already present in the assignment.
- Iterator<String> iterator = unassigned.iterator();
- while (iterator.hasNext()) {
- String instance = iterator.next();
- String zone = instanceToZoneMapping.get(instance);
- if (zoneToVirtualGroupMapping.containsKey(zone)) {
-
updatedAssignment.get(zoneToVirtualGroupMapping.get(zone)).add(instance);
- iterator.remove();
+
+ // Copy zoneMapping for tracking which zones are unassigned.
+ Map<String, Set<String>> unassignedZoneToInstances =
copyZoneMapping(zoneMapping);
+
+ // Build virtual group -> zone mapping and remove assigned zones from the
unassigned list
+ Map<String, Set<String>> virtualGroupToZoneMapping = new HashMap<>();
+ for (Map.Entry<String, Set<String>> entry :
virtualGroupToInstancesMap.entrySet()) {
+ virtualGroupToZoneMapping.putIfAbsent(entry.getKey(), new HashSet<>());
+ for (String instance : entry.getValue()) {
+ String zone = instanceToZoneMapping.get(instance);
+ virtualGroupToZoneMapping.get(entry.getKey()).add(zone);
+ unassignedZoneToInstances.remove(zone);
Review Comment:
you can do early exit here.
--
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]
