This is an automated email from the ASF dual-hosted git repository.
jiajunwang 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 b13d872 Add Helix rest Zookeeper delete API to allow removing
ephemeral ZNode (#1190)
b13d872 is described below
commit b13d872bfc2711c10b90504caeea167ebc49257a
Author: Jiajun Wang <[email protected]>
AuthorDate: Mon Aug 3 17:10:46 2020 -0700
Add Helix rest Zookeeper delete API to allow removing ephemeral ZNode
(#1190)
Add a new Helix rest API in the ZookeeperAccessor for deleting an ephemeral
ZNode.
Note that before we have ACL/audit support in the Helix rest, allowing raw
ZK write operation is dangerous.
This API is introduced prematurely for resolving the issue of "zombie"
participant (the instance has an active zk connection, but refuse to do any
work). Currently, the existence of such a node may block the normal state
transitions and then impact the cluster's availability. This PR restricts that
only an ephemeral node can be deleted to minimize the risk.
---
.../resources/zookeeper/ZooKeeperAccessor.java | 51 +++++++++++++++++++---
.../helix/rest/server/TestZooKeeperAccessor.java | 30 ++++++++++++-
2 files changed, 73 insertions(+), 8 deletions(-)
diff --git
a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/zookeeper/ZooKeeperAccessor.java
b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/zookeeper/ZooKeeperAccessor.java
index b73c9bb..87e13e1 100644
---
a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/zookeeper/ZooKeeperAccessor.java
+++
b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/zookeeper/ZooKeeperAccessor.java
@@ -21,7 +21,7 @@ package org.apache.helix.rest.server.resources.zookeeper;
import java.util.List;
import java.util.Map;
-import javax.ws.rs.DefaultValue;
+import javax.ws.rs.DELETE;
import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
@@ -47,7 +47,7 @@ import org.slf4j.LoggerFactory;
* ZooKeeperAccessor provides methods for accessing ZooKeeper resources
(ZNodes).
* It provides basic ZooKeeper features supported by ZkClient.
*/
-@Path("/zookeeper")
+@Path("/zookeeper{path: /.+}")
public class ZooKeeperAccessor extends AbstractResource {
private static final Logger LOG =
LoggerFactory.getLogger(ZooKeeperAccessor.class.getName());
private BaseDataAccessor<byte[]> _zkBaseDataAccessor;
@@ -61,7 +61,6 @@ public class ZooKeeperAccessor extends AbstractResource {
}
@GET
- @Path("{path: .+}")
public Response get(@PathParam("path") String path, @QueryParam("command")
String commandStr) {
ZooKeeperCommand cmd = getZooKeeperCommandIfPresent(commandStr);
if (cmd == null) {
@@ -73,9 +72,6 @@ public class ZooKeeperAccessor extends AbstractResource {
(ServerContext)
_application.getProperties().get(ContextPropertyKeys.SERVER_CONTEXT.name());
_zkBaseDataAccessor = _serverContext.getByteArrayZkBaseDataAccessor();
- // Need to prepend a "/" since JAX-RS regex removes it
- path = "/" + path;
-
// Check that the path supplied is valid
if (!ZkValidationUtil.isPathValid(path)) {
String errMsg = "The given path is not a valid ZooKeeper path: " + path;
@@ -101,6 +97,23 @@ public class ZooKeeperAccessor extends AbstractResource {
}
}
+ @DELETE
+ public Response delete(@PathParam("path") String path) {
+ // Lazily initialize ZkBaseDataAccessor
+ ServerContext _serverContext =
+ (ServerContext)
_application.getProperties().get(ContextPropertyKeys.SERVER_CONTEXT.name());
+ _zkBaseDataAccessor = _serverContext.getByteArrayZkBaseDataAccessor();
+
+ // Check that the path supplied is valid
+ if (!ZkValidationUtil.isPathValid(path)) {
+ String errMsg = "The given path is not a valid ZooKeeper path: " + path;
+ LOG.info(errMsg);
+ return badRequest(errMsg);
+ }
+
+ return delete(_zkBaseDataAccessor, path);
+ }
+
/**
* Checks if a ZNode exists in the given path.
* @param zkBaseDataAccessor
@@ -196,6 +209,32 @@ public class ZooKeeperAccessor extends AbstractResource {
return JSONRepresentation(result);
}
+ /**
+ * Delete the ZNode at the given path if exists.
+ * @param zkBaseDataAccessor
+ * @param path
+ * @return The delete result and the operated path.
+ */
+ private Response delete(BaseDataAccessor zkBaseDataAccessor, String path) {
+ Stat stat = zkBaseDataAccessor.getStat(path, AccessOption.PERSISTENT);
+ if (stat == null) {
+ return notFound();
+ } else if (stat.getEphemeralOwner() <= 0) {
+ // TODO: Remove this restriction once we have audit and ACL for the API
calls.
+ // TODO: This method is added pre-maturely to support removing the live
instance of a zombie
+ // TODO: instance. It is risky to allow all deleting requests before
audit and ACL are done.
+ throw new
WebApplicationException(Response.status(Response.Status.FORBIDDEN)
+ .entity(String.format("Deleting a non-ephemeral node is not
allowed.")).build());
+ }
+
+ if (zkBaseDataAccessor.remove(path, AccessOption.PERSISTENT)) {
+ return OK();
+ } else {
+ throw new
WebApplicationException(Response.status(Response.Status.INTERNAL_SERVER_ERROR)
+ .entity(String.format("Failed to delete %s.", path)).build());
+ }
+ }
+
private ZooKeeperCommand getZooKeeperCommandIfPresent(String command) {
return Enums.getIfPresent(ZooKeeperCommand.class, command).orNull();
}
diff --git
a/helix-rest/src/test/java/org/apache/helix/rest/server/TestZooKeeperAccessor.java
b/helix-rest/src/test/java/org/apache/helix/rest/server/TestZooKeeperAccessor.java
index 7d18c25..9515ce6 100644
---
a/helix-rest/src/test/java/org/apache/helix/rest/server/TestZooKeeperAccessor.java
+++
b/helix-rest/src/test/java/org/apache/helix/rest/server/TestZooKeeperAccessor.java
@@ -24,7 +24,6 @@ import java.util.Base64;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-
import javax.ws.rs.core.Response;
import org.apache.helix.AccessOption;
@@ -57,7 +56,7 @@ public class TestZooKeeperAccessor extends AbstractTestClass {
throws ZkMarshallingError {
return new String(bytes);
}
- });
+ }, ZkBaseDataAccessor.ZkClientType.DEDICATED);
}
@AfterClass
@@ -192,4 +191,31 @@ public class TestZooKeeperAccessor extends
AbstractTestClass {
// Clean up
_testBaseDataAccessor.remove(path, AccessOption.PERSISTENT);
}
+
+ @Test
+ public void testDelete() {
+ String path = "/path";
+ String deletePath = path + "/delete";
+
+ try {
+ // 1. Create a persistent node. Delete shall fail.
+ _testBaseDataAccessor.create(deletePath, null, AccessOption.PERSISTENT);
+ new JerseyUriRequestBuilder("zookeeper{}").format(deletePath)
+
.expectedReturnStatusCode(Response.Status.FORBIDDEN.getStatusCode()).delete(this);
+ Assert.assertTrue(_testBaseDataAccessor.exists(deletePath,
AccessOption.PERSISTENT));
+ // 2. Try to delete a non-exist ZNode
+ new JerseyUriRequestBuilder("zookeeper{}").format(deletePath + "/foobar")
+
.expectedReturnStatusCode(Response.Status.NOT_FOUND.getStatusCode()).delete(this);
+ // 3. Create an ephemeral node. Delete shall be done successfully.
+ _testBaseDataAccessor.remove(deletePath, AccessOption.PERSISTENT);
+ _testBaseDataAccessor.create(deletePath, null, AccessOption.EPHEMERAL);
+ // Verify with the REST endpoint
+ new JerseyUriRequestBuilder("zookeeper{}").format(deletePath)
+
.expectedReturnStatusCode(Response.Status.OK.getStatusCode()).delete(this);
+ Assert.assertFalse(_testBaseDataAccessor.exists(deletePath,
AccessOption.PERSISTENT));
+ } finally {
+ // Clean up
+ _testBaseDataAccessor.remove(path, AccessOption.PERSISTENT);
+ }
+ }
}
