Skip to content

Commit 791c08f

Browse files
Add preserveOrder check for CustomChecks
1 parent 8cc47cd commit 791c08f

File tree

2 files changed

+40
-13
lines changed

2 files changed

+40
-13
lines changed

helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/MaintenanceManagementService.java

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ public Map<String, StoppableCheck> batchGetInstancesStoppableChecks(String clust
394394
toBeStoppedInstances, preserveOrder);
395395
// custom check, includes partition check.
396396
batchCustomInstanceStoppableCheck(clusterId, instancesForCustomInstanceLevelChecks,
397-
toBeStoppedInstances, finalStoppableChecks, getMapFromJsonPayload(jsonContent));
397+
toBeStoppedInstances, finalStoppableChecks, getMapFromJsonPayload(jsonContent), preserveOrder);
398398
return finalStoppableChecks;
399399
}
400400

@@ -507,7 +507,7 @@ private List<String> batchHelixInstanceStoppableCheck(String clusterId,
507507

508508
private List<String> batchCustomInstanceStoppableCheck(String clusterId, List<String> instances,
509509
Set<String> toBeStoppedInstances, Map<String, StoppableCheck> finalStoppableChecks,
510-
Map<String, String> customPayLoads) {
510+
Map<String, String> customPayLoads, boolean preserveOrder) {
511511
if (instances.isEmpty()) {
512512
// if all instances failed at previous checks, then all following checks are not required.
513513
return instances;
@@ -536,7 +536,7 @@ private List<String> batchCustomInstanceStoppableCheck(String clusterId, List<St
536536
Map<String, StoppableCheck> clusterLevelCustomCheckResult =
537537
performAggregatedCustomCheck(clusterId, instanceIdsForCustomCheck,
538538
restConfig.getCompleteConfiguredHealthUrl().get(), customPayLoads,
539-
toBeStoppedInstances);
539+
toBeStoppedInstances, preserveOrder);
540540
List<String> instancesForNextCheck = new ArrayList<>();
541541
clusterLevelCustomCheckResult.forEach((instance, stoppableCheck) -> {
542542
addStoppableCheck(finalStoppableChecks, instance, stoppableCheck);
@@ -553,9 +553,13 @@ private List<String> batchCustomInstanceStoppableCheck(String clusterId, List<St
553553
List<String> instancesForCustomPartitionLevelChecks = instanceIdsForCustomCheck;
554554
if (!_skipHealthCheckCategories.contains(StoppableCheck.Category.CUSTOM_INSTANCE_CHECK)) {
555555
Map<String, Future<StoppableCheck>> customInstanceLevelChecks = instances.stream().collect(
556-
Collectors.toMap(Function.identity(), instance -> POOL.submit(
557-
() -> performCustomInstanceCheck(clusterId, instance, restConfig.getBaseUrl(instance),
558-
customPayLoads))));
556+
Collectors.toMap(
557+
Function.identity(),
558+
instance -> POOL.submit(() -> performCustomInstanceCheck(clusterId, instance, restConfig.getBaseUrl(instance), customPayLoads)),
559+
(existing, replacement) -> existing,
560+
// Use LinkedHashMap when preserveOrder is true to maintain the original order of instances
561+
preserveOrder ? LinkedHashMap::new : HashMap::new
562+
));
559563
instancesForCustomPartitionLevelChecks =
560564
filterInstancesForNextCheck(customInstanceLevelChecks, finalStoppableChecks);
561565
}
@@ -638,7 +642,7 @@ private Map<String, MaintenanceManagementInstanceInfo> batchInstanceHealthCheck(
638642
// custom check, includes custom Instance check and partition check.
639643
instancesForNext =
640644
batchCustomInstanceStoppableCheck(clusterId, instancesForNext, Collections.emptySet(),
641-
finalStoppableChecks, healthCheckConfig);
645+
finalStoppableChecks, healthCheckConfig, false);
642646
} else {
643647
throw new UnsupportedOperationException(healthCheck + " is not supported yet!");
644648
}
@@ -785,8 +789,10 @@ private Map<String, StoppableCheck> performPartitionsCheck(List<String> instance
785789

786790
private Map<String, StoppableCheck> performAggregatedCustomCheck(String clusterId,
787791
List<String> instances, String url, Map<String, String> customPayLoads,
788-
Set<String> toBeStoppedInstances) {
789-
Map<String, StoppableCheck> aggregatedStoppableChecks = new HashMap<>();
792+
Set<String> toBeStoppedInstances, boolean preserveOrder) {
793+
// Use LinkedHashMap when preserveOrder is true to maintain the original order of instances
794+
Map<String, StoppableCheck> aggregatedStoppableChecks = preserveOrder ?
795+
new LinkedHashMap<>() : new HashMap<>();
790796
try {
791797
Map<String, List<String>> customCheckResult =
792798
_customRestClient.getAggregatedStoppableCheck(url, instances, toBeStoppedInstances,
@@ -799,9 +805,13 @@ private Map<String, StoppableCheck> performAggregatedCustomCheck(String clusterI
799805
}
800806
} catch (IOException ex) {
801807
LOG.error("Custom client side aggregated health check for {} failed.", clusterId, ex);
802-
return instances.stream().collect(Collectors.toMap(Function.identity(),
808+
return instances.stream().collect(Collectors.toMap(
809+
Function.identity(),
803810
instance -> new StoppableCheck(false, Collections.singletonList(instance),
804-
StoppableCheck.Category.CUSTOM_AGGREGATED_CHECK)));
811+
StoppableCheck.Category.CUSTOM_AGGREGATED_CHECK),
812+
(existing, replacement) -> existing,
813+
// Use LinkedHashMap when preserveOrder is true to maintain the original order of instances
814+
preserveOrder ? LinkedHashMap::new : HashMap::new));
805815
}
806816
return aggregatedStoppableChecks;
807817
}

helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/StoppableInstancesSelector.java

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,25 @@ private StoppableInstancesSelector(String clusterId, List<String> orderOfZone,
7575
_dataAccessor = dataAccessor;
7676
}
7777

78+
/**
79+
* Evaluates and collects stoppable instances within a specified or determined zone based on the order of zones.
80+
* If _orderOfZone is specified, the method targets the first non-empty zone; otherwise, it targets the zone with
81+
* the highest instance count. The method iterates through instances, performing stoppable checks, and records
82+
* reasons for non-stoppability.
83+
*
84+
* @param instances A list of instance to be evaluated.
85+
* @param toBeStoppedInstances A list of instances presumed to be already stopped
86+
* @return An ObjectNode containing:
87+
* - 'stoppableNode': List of instances that can be stopped.
88+
* - 'instance_not_stoppable_with_reasons': A map with the instance name as the key and
89+
* a list of reasons for non-stoppability as the value.
90+
* @throws IOException
91+
*/
92+
public ObjectNode getStoppableInstancesInSingleZone(List<String> instances,
93+
List<String> toBeStoppedInstances) throws IOException {
94+
return getStoppableInstancesInSingleZone(instances, toBeStoppedInstances, false);
95+
}
96+
7897
/**
7998
* Evaluates and collects stoppable instances within a specified or determined zone based on the order of zones.
8099
* If _orderOfZone is specified, the method targets the first non-empty zone; otherwise, it targets the zone with
@@ -115,7 +134,6 @@ public ObjectNode getStoppableInstancesInSingleZone(List<String> instances,
115134
*
116135
* @param instances A list of instance to be evaluated.
117136
* @param toBeStoppedInstances A list of instances presumed to be already stopped
118-
* @param preserveOrder Indicates whether to preserve the original order of instances
119137
* @return An ObjectNode containing:
120138
* - 'stoppableNode': List of instances that can be stopped.
121139
* - 'instance_not_stoppable_with_reasons': A map with the instance name as the key and
@@ -153,7 +171,6 @@ public ObjectNode getStoppableInstancesCrossZones(List<String> instances,
153171
*
154172
* @param instances A list of instance to be evaluated.
155173
* @param toBeStoppedInstances A list of instances presumed to be already stopped
156-
* @param preserveOrder Indicates whether to preserve the original order of instances
157174
* @return An ObjectNode containing:
158175
* - 'stoppableNode': List of instances that can be stopped.
159176
* - 'instance_not_stoppable_with_reasons': A map with the instance name as the key and

0 commit comments

Comments
 (0)