helix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject [1/3] helix git commit: [HELIX-776] REST2.0: Add delete command to updateInstanceConfig
Date Thu, 01 Nov 2018 00:24:30 GMT
Repository: helix
Updated Branches:
  refs/heads/master b235c4ee5 -> ceba1a55a


[HELIX-776] REST2.0: Add delete command to updateInstanceConfig

For instance configs, REST2.0 did not expose the REST API for deletion of fields. This RB
adds update and delete commands to updateInstanceConfig and an integration test thereof.
Changelist:
1. Add delete command to updateInstanceConfig in InstanceAccessor
2. Add integration tests


Project: http://git-wip-us.apache.org/repos/asf/helix/repo
Commit: http://git-wip-us.apache.org/repos/asf/helix/commit/6090732b
Tree: http://git-wip-us.apache.org/repos/asf/helix/tree/6090732b
Diff: http://git-wip-us.apache.org/repos/asf/helix/diff/6090732b

Branch: refs/heads/master
Commit: 6090732be6b88863017a93106fa692dc7350520b
Parents: b235c4e
Author: Hunter Lee <hulee@linkedin.com>
Authored: Wed Oct 31 14:20:18 2018 -0700
Committer: Hunter Lee <hulee@linkedin.com>
Committed: Wed Oct 31 14:20:18 2018 -0700

----------------------------------------------------------------------
 .../java/org/apache/helix/ConfigAccessor.java   |   6 +
 .../resources/helix/InstanceAccessor.java       |  34 +++++-
 .../helix/rest/server/TestInstanceAccessor.java | 114 ++++++++++++++++++-
 3 files changed, 146 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/helix/blob/6090732b/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java
----------------------------------------------------------------------
diff --git a/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java b/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java
index 2755113..53f42fb 100644
--- a/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java
+++ b/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java
@@ -765,6 +765,12 @@ public class ConfigAccessor {
             .forParticipant(instanceName).build();
     String zkPath = scope.getZkPath();
 
+    if (!zkClient.exists(zkPath)) {
+      throw new HelixException(
+          "updateInstanceConfig failed. Given InstanceConfig does not already exist. instance:
"
+              + instanceName);
+    }
+
     if (overwrite) {
       ZKUtil.createOrReplace(zkClient, zkPath, instanceConfig.getRecord(), true);
     } else {

http://git-wip-us.apache.org/repos/asf/helix/blob/6090732b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstanceAccessor.java
----------------------------------------------------------------------
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstanceAccessor.java
b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstanceAccessor.java
index 98af0ee..38ee3b5 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstanceAccessor.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/helix/InstanceAccessor.java
@@ -41,10 +41,12 @@ import org.apache.helix.model.ClusterConfig;
 import org.apache.helix.model.CurrentState;
 import org.apache.helix.model.Error;
 import org.apache.helix.model.HealthStat;
+import org.apache.helix.model.HelixConfigScope;
 import org.apache.helix.model.InstanceConfig;
 import org.apache.helix.model.LiveInstance;
 import org.apache.helix.model.Message;
 import org.apache.helix.model.ParticipantHistory;
+import org.apache.helix.model.builder.HelixConfigScopeBuilder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.codehaus.jackson.JsonNode;
@@ -321,7 +323,19 @@ public class InstanceAccessor extends AbstractHelixResource {
   @POST
   @Path("{instanceName}/configs")
   public Response updateInstanceConfig(@PathParam("clusterId") String clusterId,
-      @PathParam("instanceName") String instanceName, String content) {
+      @PathParam("instanceName") String instanceName, @QueryParam("command") String commandStr,
+      String content) {
+    Command command;
+    if (commandStr == null || commandStr.isEmpty()) {
+      command = Command.update; // Default behavior to keep it backward-compatible
+    } else {
+      try {
+        command = getCommand(commandStr);
+      } catch (HelixException ex) {
+        return badRequest(ex.getMessage());
+      }
+    }
+
     ZNRecord record;
     try {
       record = toZNRecord(content);
@@ -332,11 +346,25 @@ public class InstanceAccessor extends AbstractHelixResource {
     InstanceConfig instanceConfig = new InstanceConfig(record);
     ConfigAccessor configAccessor = getConfigAccessor();
     try {
-      configAccessor.updateInstanceConfig(clusterId, instanceName, instanceConfig);
+      switch (command) {
+      case update:
+        configAccessor.updateInstanceConfig(clusterId, instanceName, instanceConfig);
+        break;
+      case delete: {
+        HelixConfigScope instanceScope =
+            new HelixConfigScopeBuilder(HelixConfigScope.ConfigScopeProperty.PARTICIPANT)
+                .forCluster(clusterId).forParticipant(instanceName).build();
+        configAccessor.remove(instanceScope, record);
+      }
+        break;
+      default:
+        return badRequest(String.format("Unsupported command: %s", command));
+      }
     } catch (HelixException ex) {
       return notFound(ex.getMessage());
     } catch (Exception ex) {
-      _logger.error(String.format("Error in update instance config for instance: %s", instanceName),
ex);
+      _logger.error(String.format("Error in update instance config for instance: %s", instanceName),
+          ex);
       return serverError(ex);
     }
     return OK();

http://git-wip-us.apache.org/repos/asf/helix/blob/6090732b/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstanceAccessor.java
----------------------------------------------------------------------
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstanceAccessor.java
b/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstanceAccessor.java
index 24d2910..94c28b2 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstanceAccessor.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/server/TestInstanceAccessor.java
@@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableMap;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -201,20 +202,56 @@ public class TestInstanceAccessor extends AbstractTestClass {
         new HashSet<>(Arrays.asList(CLUSTER_NAME + dbName + "0", CLUSTER_NAME + dbName
+ "3")));
   }
 
+  /**
+   * Test "update" command for updateInstanceConfig endpoint.
+   * @throws IOException
+   */
   @Test(dependsOnMethods = "updateInstance")
   public void updateInstanceConfig() throws IOException {
     System.out.println("Start test :" + TestHelper.getTestMethodName());
     String instanceName = CLUSTER_NAME + "localhost_12918";
     InstanceConfig instanceConfig = _configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName);
     ZNRecord record = instanceConfig.getRecord();
-    record.getSimpleFields().put("TestSimple", "value");
-    record.getMapFields().put("TestMap", ImmutableMap.of("key", "value"));
-    record.getListFields().put("TestList", Arrays.asList("e1", "e2", "e3"));
 
+    // Generate a record containing three keys (k0, k1, k2) for all fields
+    String value = "value";
+    for (int i = 0; i < 3; i++) {
+      String key = "k" + i;
+      record.getSimpleFields().put(key, value);
+      record.getMapFields().put(key, ImmutableMap.of(key, value));
+      record.getListFields().put(key, Arrays.asList(key, value));
+    }
+
+    // 1. Add these fields by way of "update"
     Entity entity =
         Entity.entity(OBJECT_MAPPER.writeValueAsString(record), MediaType.APPLICATION_JSON_TYPE);
-    post("clusters/" + CLUSTER_NAME + "/instances/" + instanceName + "/configs", null, entity,
-        Response.Status.OK.getStatusCode());
+    post("clusters/" + CLUSTER_NAME + "/instances/" + instanceName + "/configs",
+        Collections.singletonMap("command", "update"), entity, Response.Status.OK.getStatusCode());
+
+    // Check that the fields have been added
+    Assert.assertEquals(record.getSimpleFields(),
+        _configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord()
+            .getSimpleFields());
+    Assert.assertEquals(record.getListFields(),
+        _configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord().getListFields());
+    Assert.assertEquals(record.getMapFields(),
+        _configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord().getMapFields());
+
+    String newValue = "newValue";
+    // 2. Modify the record and update
+    for (int i = 0; i < 3; i++) {
+      String key = "k" + i;
+      record.getSimpleFields().put(key, newValue);
+      record.getMapFields().put(key, ImmutableMap.of(key, newValue));
+      record.getListFields().put(key, Arrays.asList(key, newValue));
+    }
+
+    entity =
+        Entity.entity(OBJECT_MAPPER.writeValueAsString(record), MediaType.APPLICATION_JSON_TYPE);
+    post("clusters/" + CLUSTER_NAME + "/instances/" + instanceName + "/configs",
+        Collections.singletonMap("command", "update"), entity, Response.Status.OK.getStatusCode());
+
+    // Check that the fields have been modified
     Assert.assertEquals(record.getSimpleFields(),
         _configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord()
             .getSimpleFields());
@@ -223,4 +260,71 @@ public class TestInstanceAccessor extends AbstractTestClass {
     Assert.assertEquals(record.getMapFields(),
         _configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord().getMapFields());
   }
+
+  /**
+   * Test the "delete" command of updateInstanceConfig.
+   * @throws IOException
+   */
+  @Test(dependsOnMethods = "updateInstanceConfig")
+  public void deleteInstanceConfig() throws IOException {
+    System.out.println("Start test :" + TestHelper.getTestMethodName());
+    String instanceName = CLUSTER_NAME + "localhost_12918";
+    ZNRecord record = new ZNRecord(instanceName);
+
+    // Generate a record containing three keys (k1, k2, k3) for all fields for deletion
+    String value = "value";
+    for (int i = 1; i < 4; i++) {
+      String key = "k" + i;
+      record.getSimpleFields().put(key, value);
+      record.getMapFields().put(key, ImmutableMap.of(key, value));
+      record.getListFields().put(key, Arrays.asList(key, value));
+    }
+
+    // First, add these fields by way of "update"
+    Entity entity =
+        Entity.entity(OBJECT_MAPPER.writeValueAsString(record), MediaType.APPLICATION_JSON_TYPE);
+    post("clusters/" + CLUSTER_NAME + "/instances/" + instanceName + "/configs",
+        Collections.singletonMap("command", "delete"), entity, Response.Status.OK.getStatusCode());
+
+    // Check that the keys k1 and k2 have been deleted, and k0 remains
+    for (int i = 0; i < 4; i++) {
+      String key = "k" + i;
+      if (i == 0) {
+        Assert.assertTrue(_configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord()
+            .getSimpleFields().containsKey(key));
+        Assert.assertTrue(_configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord()
+            .getListFields().containsKey(key));
+        Assert.assertTrue(_configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord()
+            .getMapFields().containsKey(key));
+        continue;
+      }
+      Assert.assertFalse(_configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord()
+          .getSimpleFields().containsKey(key));
+      Assert.assertFalse(_configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord()
+          .getListFields().containsKey(key));
+      Assert.assertFalse(_configAccessor.getInstanceConfig(CLUSTER_NAME, instanceName).getRecord()
+          .getMapFields().containsKey(key));
+    }
+  }
+
+  /**
+   * Check that updateInstanceConfig fails when there is no pre-existing InstanceConfig ZNode.
This
+   * is because InstanceConfig should have been created when the instance was added, and
this REST
+   * endpoint is not meant for creation.
+   */
+  @Test(dependsOnMethods = "deleteInstanceConfig")
+  public void checkUpdateFails() throws IOException {
+    System.out.println("Start test :" + TestHelper.getTestMethodName());
+    String instanceName = CLUSTER_NAME + "non_existent_instance";
+    InstanceConfig instanceConfig = new InstanceConfig(INSTANCE_NAME + "TEST");
+    ZNRecord record = instanceConfig.getRecord();
+    record.getSimpleFields().put("TestSimple", "value");
+    record.getMapFields().put("TestMap", ImmutableMap.of("key", "value"));
+    record.getListFields().put("TestList", Arrays.asList("e1", "e2", "e3"));
+
+    Entity entity =
+        Entity.entity(OBJECT_MAPPER.writeValueAsString(record), MediaType.APPLICATION_JSON_TYPE);
+    post("clusters/" + CLUSTER_NAME + "/instances/" + instanceName + "/configs", null, entity,
+        Response.Status.NOT_FOUND.getStatusCode());
+  }
 }


Mime
View raw message