You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2021/12/08 03:49:51 UTC
[GitHub] [helix] junkaixue commented on a change in pull request #1918: Add take/free instance implementation and test
junkaixue commented on a change in pull request #1918:
URL: https://github.com/apache/helix/pull/1918#discussion_r764527556
##########
File path: helix-rest/src/main/java/org/apache/helix/rest/server/service/InstanceService.java
##########
@@ -49,6 +47,7 @@ InstanceInfo getInstanceInfo(String clusterId, String instanceName,
* @return An instance of {@link StoppableCheck} easily convertible to JSON
* @throws IOException in case of network failure
*/
+ @Deprecated
Review comment:
Let's not deprecate them. They are still can be leveraged as module function just for checks.
##########
File path: helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/MaintenanceManagementService.java
##########
@@ -268,30 +301,92 @@ public StoppableCheck getInstanceStoppableCheck(String clusterId, String instanc
public Map<String, StoppableCheck> batchGetInstancesStoppableChecks(String clusterId,
List<String> instances, String jsonContent) throws IOException {
- Map<String, StoppableCheck> finalStoppableChecks = new HashMap<>();
// helix instance check
+ Map<String, StoppableCheck> finalStoppableChecks = new HashMap<>();
+
List<String> instancesForCustomInstanceLevelChecks =
batchHelixInstanceStoppableCheck(clusterId, instances, finalStoppableChecks);
- // custom check
+ // custom check, includes partition check.
batchCustomInstanceStoppableCheck(clusterId, instancesForCustomInstanceLevelChecks,
finalStoppableChecks, getMapFromJsonPayload(jsonContent));
return finalStoppableChecks;
}
+ private MaintenanceManagementInstanceInfo takeFreeSingleInstanceHelper(String clusterId,
+ String instanceName, List<String> healthChecks, Map<String, String> healthCheckConfig,
+ List<String> operations, Map<String, String> operationConfig, boolean performOperation,
+ boolean isTakeInstance) {
+
+ if (operations == null) {
+ operations = new ArrayList<>();
+ }
+
+ try {
+ MaintenanceManagementInstanceInfo instanceInfo;
+ if (isTakeInstance) {
+ if (healthChecks == null) {
+ healthChecks = new ArrayList<>();
+ }
+ instanceInfo =
+ batchInstanceHealthCheck(clusterId, ImmutableList.of(instanceName), healthChecks,
+ healthCheckConfig).getOrDefault(instanceName, new MaintenanceManagementInstanceInfo(
+ MaintenanceManagementInstanceInfo.OperationalStatus.SUCCESS));
+ if (!instanceInfo.isSuccessful()) {
Review comment:
I remember we should be able to let user define which are the ones can be skipped even if failed.
##########
File path: helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/MaintenanceManagementService.java
##########
@@ -268,30 +301,92 @@ public StoppableCheck getInstanceStoppableCheck(String clusterId, String instanc
public Map<String, StoppableCheck> batchGetInstancesStoppableChecks(String clusterId,
List<String> instances, String jsonContent) throws IOException {
- Map<String, StoppableCheck> finalStoppableChecks = new HashMap<>();
// helix instance check
+ Map<String, StoppableCheck> finalStoppableChecks = new HashMap<>();
+
List<String> instancesForCustomInstanceLevelChecks =
batchHelixInstanceStoppableCheck(clusterId, instances, finalStoppableChecks);
- // custom check
+ // custom check, includes partition check.
batchCustomInstanceStoppableCheck(clusterId, instancesForCustomInstanceLevelChecks,
finalStoppableChecks, getMapFromJsonPayload(jsonContent));
return finalStoppableChecks;
}
+ private MaintenanceManagementInstanceInfo takeFreeSingleInstanceHelper(String clusterId,
+ String instanceName, List<String> healthChecks, Map<String, String> healthCheckConfig,
+ List<String> operations, Map<String, String> operationConfig, boolean performOperation,
+ boolean isTakeInstance) {
+
+ if (operations == null) {
+ operations = new ArrayList<>();
+ }
+
+ try {
+ MaintenanceManagementInstanceInfo instanceInfo;
+ if (isTakeInstance) {
Review comment:
Why we need this?
--
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: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org