Skip to content

Commit

Permalink
Modify the API logic from whitelisting stoppable checks to blacklisti…
Browse files Browse the repository at this point in the history
…ng stoppable checks
  • Loading branch information
MarkGaox committed Nov 3, 2023
1 parent 3e63642 commit 2f3b0fb
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ public class MaintenanceManagementService {
private final HelixDataAccessorWrapper _dataAccessor;
private final Set<String> _nonBlockingHealthChecks;
private final Set<StoppableCheck.Category> _skipHealthCheckCategories;
// Set the default value of _stoppableHealthCheckList to be the list of all stoppable checks to
// Set the default value of _skipStoppableHealthCheckList to be an empty list to
// maintain the backward compatibility with users who don't use MaintenanceManagementServiceBuilder
// to create the MaintenanceManagementService object.
private List<HealthCheck> _stoppableHealthCheckList = HealthCheck.STOPPABLE_CHECK_LIST;
private List<HealthCheck> _skipStoppableHealthCheckList = Collections.emptyList();

public MaintenanceManagementService(ZKHelixDataAccessor dataAccessor,
ConfigAccessor configAccessor, boolean skipZKRead, String namespace) {
Expand Down Expand Up @@ -151,7 +151,7 @@ public MaintenanceManagementService(ZKHelixDataAccessor dataAccessor,
private MaintenanceManagementService(ZKHelixDataAccessor dataAccessor,
ConfigAccessor configAccessor, CustomRestClient customRestClient, boolean skipZKRead,
Set<String> nonBlockingHealthChecks, Set<StoppableCheck.Category> skipHealthCheckCategories,
List<HealthCheck> stoppableHealthCheckList, String namespace) {
List<HealthCheck> skipStoppableHealthCheckList, String namespace) {
_dataAccessor =
new HelixDataAccessorWrapper(dataAccessor, customRestClient,
namespace);
Expand All @@ -162,8 +162,8 @@ private MaintenanceManagementService(ZKHelixDataAccessor dataAccessor,
nonBlockingHealthChecks == null ? Collections.emptySet() : nonBlockingHealthChecks;
_skipHealthCheckCategories =
skipHealthCheckCategories == null ? Collections.emptySet() : skipHealthCheckCategories;
_stoppableHealthCheckList = stoppableHealthCheckList == null ? HealthCheck.STOPPABLE_CHECK_LIST
: stoppableHealthCheckList;
_skipStoppableHealthCheckList = skipStoppableHealthCheckList == null ? Collections.emptyList()
: skipStoppableHealthCheckList;
_namespace = namespace;
}

Expand Down Expand Up @@ -635,8 +635,10 @@ private boolean isNonBlockingCheck(StoppableCheck stoppableCheck) {
private StoppableCheck performHelixOwnInstanceCheck(String clusterId, String instanceName,
Set<String> toBeStoppedInstances) {
LOG.info("Perform helix own custom health checks for {}/{}", clusterId, instanceName);
List<HealthCheck> healthChecksToExecute = new ArrayList<>(HealthCheck.STOPPABLE_CHECK_LIST);
healthChecksToExecute.removeAll(_skipStoppableHealthCheckList);
Map<String, Boolean> helixStoppableCheck =
getInstanceHealthStatus(clusterId, instanceName, _stoppableHealthCheckList,
getInstanceHealthStatus(clusterId, instanceName, healthChecksToExecute,
toBeStoppedInstances);

return new StoppableCheck(helixStoppableCheck, StoppableCheck.Category.HELIX_OWN_CHECK);
Expand Down Expand Up @@ -803,7 +805,7 @@ public static class MaintenanceManagementServiceBuilder {
private CustomRestClient _customRestClient;
private Set<String> _nonBlockingHealthChecks;
private Set<StoppableCheck.Category> _skipHealthCheckCategories = Collections.emptySet();
private List<HealthCheck> _stoppableHealthCheckList = Collections.emptyList();
private List<HealthCheck> _skipStoppableHealthCheckList = Collections.emptyList();

public MaintenanceManagementServiceBuilder setConfigAccessor(ConfigAccessor configAccessor) {
_configAccessor = configAccessor;
Expand Down Expand Up @@ -844,17 +846,17 @@ public MaintenanceManagementServiceBuilder setSkipHealthCheckCategories(
return this;
}

public MaintenanceManagementServiceBuilder setStoppableHealthCheckList(
List<HealthCheck> stoppableHealthCheckList) {
_stoppableHealthCheckList = stoppableHealthCheckList;
public MaintenanceManagementServiceBuilder setSkipStoppableHealthCheckList(
List<HealthCheck> skipStoppableHealthCheckList) {
_skipStoppableHealthCheckList = skipStoppableHealthCheckList;
return this;
}

public MaintenanceManagementService build() {
validate();
return new MaintenanceManagementService(_dataAccessor, _configAccessor, _customRestClient,
_skipZKRead, _nonBlockingHealthChecks, _skipHealthCheckCategories,
_stoppableHealthCheckList, _namespace);
_skipStoppableHealthCheckList, _namespace);
}

private void validate() throws IllegalArgumentException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public enum InstancesProperties {
selection_base,
zone_order,
to_be_stopped_instances,
stoppable_check_list,
skip_stoppable_check_list,
customized_values,
instance_stoppable_parallel,
instance_not_stoppable_with_reasons
Expand Down Expand Up @@ -234,9 +234,9 @@ private Response batchGetStoppableInstances(String clusterId, JsonNode node, boo
List<String> orderOfZone = null;
String customizedInput = null;
List<String> toBeStoppedInstances = Collections.emptyList();
// By default, if stoppable_check_list is unset, all checks are performed to maintain
// By default, if skip_stoppable_check_list is unset, all checks are performed to maintain
// backward compatibility with existing clients.
List<HealthCheck> stoppableCheckList = HealthCheck.STOPPABLE_CHECK_LIST;
List<HealthCheck> skipStoppableCheckList = Collections.emptyList();
if (node.get(InstancesAccessor.InstancesProperties.customized_values.name()) != null) {
customizedInput =
node.get(InstancesAccessor.InstancesProperties.customized_values.name()).toString();
Expand Down Expand Up @@ -269,16 +269,16 @@ private Response batchGetStoppableInstances(String clusterId, JsonNode node, boo
}
}

if (node.get(InstancesProperties.stoppable_check_list.name()) != null) {
if (node.get(InstancesProperties.skip_stoppable_check_list.name()) != null) {
List<String> list = OBJECT_MAPPER.readValue(
node.get(InstancesProperties.stoppable_check_list.name()).toString(),
node.get(InstancesProperties.skip_stoppable_check_list.name()).toString(),
OBJECT_MAPPER.getTypeFactory().constructCollectionType(List.class, String.class));
try {
stoppableCheckList =
skipStoppableCheckList =
list.stream().map(HealthCheck::valueOf).collect(Collectors.toList());
} catch (IllegalArgumentException e) {
String message =
"'stoppable_check_list' has invalid check names: " + list
"'skip_stoppable_check_list' has invalid check names: " + list
+ ". Supported checks: " + HealthCheck.STOPPABLE_CHECK_LIST;
_logger.error(message, e);
return badRequest(message);
Expand All @@ -296,7 +296,7 @@ private Response batchGetStoppableInstances(String clusterId, JsonNode node, boo
.setCustomRestClient(CustomRestClientFactory.get())
.setSkipHealthCheckCategories(skipHealthCheckCategories)
.setNamespace(namespace)
.setStoppableHealthCheckList(stoppableCheckList)
.setSkipStoppableHealthCheckList(skipStoppableCheckList)
.build();

ClusterService clusterService =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ public void testInstanceStoppableCrossZoneBasedWithSelectedCheckList() throws IO
InstancesAccessor.InstanceHealthSelectionBase.cross_zone_based.name(),
InstancesAccessor.InstancesProperties.instances.name(), "instance0", "instance1",
"instance2", "instance3", "instance4", "instance5", "invalidInstance",
InstancesAccessor.InstancesProperties.stoppable_check_list.name(), "DUMMY_TEST_NO_EXISTS");
InstancesAccessor.InstancesProperties.skip_stoppable_check_list.name(), "DUMMY_TEST_NO_EXISTS");

new JerseyUriRequestBuilder("clusters/{}/instances?command=stoppable").format(STOPPABLE_CLUSTER)
.isBodyReturnExpected(true)
Expand All @@ -226,7 +226,7 @@ public void testInstanceStoppableCrossZoneBasedWithSelectedCheckList() throws IO
InstancesAccessor.InstancesProperties.instances.name(), "instance0", "instance1",
"instance2", "instance3", "instance4", "instance5", "invalidInstance",
InstancesAccessor.InstancesProperties.zone_order.name(), "zone2", "zone1",
InstancesAccessor.InstancesProperties.stoppable_check_list.name(), "EMPTY_RESOURCE_ASSIGNMENT", "INSTANCE_NOT_ALIVE");
InstancesAccessor.InstancesProperties.skip_stoppable_check_list.name(), "INSTANCE_NOT_ENABLED", "INSTANCE_NOT_STABLE");
Response response = new JerseyUriRequestBuilder(
"clusters/{}/instances?command=stoppable&skipHealthCheckCategories=CUSTOM_INSTANCE_CHECK,CUSTOM_PARTITION_CHECK").format(
STOPPABLE_CLUSTER).post(this, Entity.entity(content, MediaType.APPLICATION_JSON_TYPE));
Expand Down

0 comments on commit 2f3b0fb

Please sign in to comment.