You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by hu...@apache.org on 2019/10/23 06:29:18 UTC
[helix] branch master updated: Fix null response for instance
stoppable check when connection refused. (#504)
This is an automated email from the ASF dual-hosted git repository.
hulee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git
The following commit(s) were added to refs/heads/master by this push:
new 824bc95 Fix null response for instance stoppable check when connection refused. (#504)
824bc95 is described below
commit 824bc95c818f791f38315d10b206567fb22908d3
Author: Huizhi L <ih...@gmail.com>
AuthorDate: Tue Oct 22 23:29:09 2019 -0700
Fix null response for instance stoppable check when connection refused. (#504)
Issue: Instance stoppable check endpoint /clusters//instances//stoppable returns null when connection between helix rest server and storage node.
This diff fixes this by: return a StoppableCheck object when connection refused.
---
.../rest/server/json/instance/StoppableCheck.java | 4 +++
.../rest/server/service/InstanceServiceImpl.java | 12 +++++----
.../rest/server/service/TestInstanceService.java | 31 ++++++++++++++++++++++
3 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/json/instance/StoppableCheck.java b/helix-rest/src/main/java/org/apache/helix/rest/server/json/instance/StoppableCheck.java
index 44458b3..b2e72bf 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/server/json/instance/StoppableCheck.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/server/json/instance/StoppableCheck.java
@@ -38,6 +38,10 @@ public class StoppableCheck {
Category(String prefix) {
this.prefix = prefix;
}
+
+ public String getPrefix() {
+ return prefix;
+ }
}
@JsonProperty("stoppable")
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/service/InstanceServiceImpl.java b/helix-rest/src/main/java/org/apache/helix/rest/server/service/InstanceServiceImpl.java
index 37559e5..9d855d8 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/server/service/InstanceServiceImpl.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/server/service/InstanceServiceImpl.java
@@ -21,6 +21,7 @@ package org.apache.helix.rest.server.service;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -214,16 +215,17 @@ public class InstanceServiceImpl implements InstanceService {
}
private StoppableCheck performCustomInstanceCheck(String clusterId, String instanceName,
- String baseUrl, Map<String, String> customPayLoads) throws IOException {
+ String baseUrl, Map<String, String> customPayLoads) {
LOG.info("Perform instance level client side health checks for {}/{}", clusterId, instanceName);
try {
return new StoppableCheck(
_customRestClient.getInstanceStoppableCheck(baseUrl, customPayLoads),
StoppableCheck.Category.CUSTOM_INSTANCE_CHECK);
- } catch (IOException e) {
- LOG.error("Failed to perform custom client side instance level health checks for {}/{}",
- clusterId, instanceName, e);
- throw e;
+ } catch (IOException ex) {
+ LOG.error("Custom client side instance level health check for {}/{} failed.", clusterId,
+ instanceName, ex);
+ return new StoppableCheck(false, Arrays.asList(instanceName),
+ StoppableCheck.Category.CUSTOM_INSTANCE_CHECK);
}
}
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/service/TestInstanceService.java b/helix-rest/src/test/java/org/apache/helix/rest/server/service/TestInstanceService.java
index d142b54..8b218eb 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/server/service/TestInstanceService.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/server/service/TestInstanceService.java
@@ -37,6 +37,7 @@ import org.apache.helix.model.RESTConfig;
import org.apache.helix.rest.client.CustomRestClient;
import org.apache.helix.rest.common.HelixDataAccessorWrapper;
import org.apache.helix.rest.server.json.instance.StoppableCheck;
+import org.codehaus.jackson.map.ObjectMapper;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.testng.Assert;
@@ -110,6 +111,36 @@ public class TestInstanceService {
verify(_customRestClient, times(0)).getPartitionStoppableCheck(any(), any(), any());
}
+ @Test
+ public void testGetInstanceStoppableCheckConnectionRefused() throws IOException {
+ InstanceService service =
+ new InstanceServiceImpl(_dataAccessor, _configAccessor, _customRestClient) {
+ @Override
+ protected Map<String, Boolean> getInstanceHealthStatus(String clusterId,
+ String instanceName, List<HealthCheck> healthChecks) {
+ return Collections.emptyMap();
+ }
+ };
+
+ when(_customRestClient.getInstanceStoppableCheck(anyString(), anyMap()))
+ .thenThrow(IOException.class);
+
+ Map<String, String> dataMap =
+ ImmutableMap.of("instance", TEST_INSTANCE, "selection_base", "zone_based");
+ String jsonContent = new ObjectMapper().writeValueAsString(dataMap);
+ StoppableCheck actual =
+ service.getInstanceStoppableCheck(TEST_CLUSTER, TEST_INSTANCE, jsonContent);
+
+ int expectedFailedChecksSize = 1;
+ String expectedFailedCheck =
+ StoppableCheck.Category.CUSTOM_INSTANCE_CHECK.getPrefix() + TEST_INSTANCE;
+
+ Assert.assertNotNull(actual);
+ Assert.assertFalse(actual.isStoppable());
+ Assert.assertEquals(actual.getFailedChecks().size(), expectedFailedChecksSize);
+ Assert.assertEquals(actual.getFailedChecks().get(0), expectedFailedCheck);
+ }
+
// TODO re-enable the test when partition health checks get decoupled
@Test(enabled = false)
public void testGetInstanceStoppableCheckWhenPartitionsCheckFail() throws IOException {