ZOOKEEPER-2563: A revisit to setquota

- remove some useless methods in the `ZooKeeperMain.java`
- cannot set the quota on the path under` /zookeeper/quota`, because this path is reserved for storing the quota info. if we do something like this:
`setquota -n 5 /zookeeper/quota`
which will let us only have five chances to use the setquota, that violates the quota semantics.
- handle the Exception when users set a quota on the parent/child node.([ZOOKEEPER-2565](https://issues.apache.org/jira/browse/ZOOKEEPER-2565)).
- more detail in the [ZOOKEEPER-2563](https://issues.apache.org/jira/browse/ZOOKEEPER-2563)

Author: maoling <maoling199210191@sina.com>

Reviewers: andor@apache.org

Closes #936 from maoling/ZOOKEEPER-2563 and squashes the following commits:

9d46e862c [maoling] strength the doc,especially for the concept:namespaces
3777359bd [maoling] ZOOKEEPER-2563: A revisit to setquota

zookeeper-docs/src/main/resources/markdown/zookeeperCLI.md

@@ -497,7 +497,7 @@ Set the Acl permission for one node.
 Set the quota in one path.
 
 ```bash
-# -n to limit the number of child nodes(namespaces)
+# -n to limit the number of child nodes(included itself)
 [zkshell: 18] setquota -n 2 /quota_test
 [zkshell: 19] create /quota_test/child_1
 	Created /quota_test/child_1
@@ -509,7 +509,7 @@ Set the quota in one path.
 	2019-03-07 11:22:36,680 [myid:1] - WARN  [SyncThread:0:DataTree@374] - Quota exceeded: /quota_test count=3 limit=2
 	2019-03-07 11:22:41,861 [myid:1] - WARN  [SyncThread:0:DataTree@374] - Quota exceeded: /quota_test count=4 limit=2
 
-# -b to limit the bytes/size of one path
+# -b to limit the bytes(data length) of one path
 [zkshell: 22] setquota -b 5 /brokers
 [zkshell: 23] set /brokers "I_love_zookeeper"
 # Notice:don't have a hard constraint,just log the warning info

zookeeper-docs/src/main/resources/markdown/zookeeperQuotas.md

@@ -31,6 +31,9 @@ ZooKeeper has both namespace and bytes quotas. You can use the ZooKeeperMain cla
 ZooKeeper prints _WARN_ messages if users exceed the quota assigned to them. The messages
 are printed in the log of the ZooKeeper.
 
+Notice: What the `namespace` quota means is the count quota which limits the number of children
+under the path(included itself).
+
     $ bin/zkCli.sh -server host:port**
 
 The above command gives you a command line option of using quotas.
@@ -39,12 +42,30 @@ The above command gives you a command line option of using quotas.
 
 ### Setting Quotas
 
-You can use _setquota_ to set a quota on a ZooKeeper node. It has an option of setting quota with
-`-n` (for namespace)
-and `-b` (for bytes).
+- You can use `setquota` to set a quota on a ZooKeeper node. It has an option of setting quota with
+`-n` (for namespace/count) and `-b` (for bytes/data length).
+
+- The ZooKeeper quota is stored in ZooKeeper itself in **/zookeeper/quota**. To disable other people from
+changing the quotas, users can set the ACL for **/zookeeper/quota** ,so that only admins are able to read and write to it.
+
+- If the quota doesn't exist in the specified path,create the quota, otherwise update the quota.
+
+- The Scope of the quota users set is all the nodes under the path specified (included itself).
+
+- In order to simplify the calculation of quota in the current directory/hierarchy structure, a complete tree path(from root to leaf node)
+can be set only one quota. In the situation when setting a quota in a path which its parent or child node already has a quota. `setquota` will
+reject and tell the specified parent or child path, users can adjust allocations of quotas(delete/move-up/move-down the quota)
+according to specific circumstances.
+
+- Combined with the Chroot, the quota will have a better isolation effectiveness between different applications.For example:
+
+    ```bash
+    # Chroot is:
+    192.168.0.1:2181,192.168.0.2:2181,192.168.0.3:2181/apps/app1
+    setquota -n 100000 /apps/app1
+    ```
 
-The ZooKeeper quota is stored in ZooKeeper itself in /zookeeper/quota. To disable other people from
-changing the quota's set the ACL for /zookeeper/quota such that only admins are able to read and write to it.
+- Users cannot set the quota on the path under **/zookeeper/quota**
 
 <a name="Listing+Quotas"></a>
 

zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeperMain.java

@@ -43,8 +43,6 @@ import org.apache.zookeeper.cli.MalformedCommandException;
 import org.apache.zookeeper.cli.VersionCommand;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.apache.zookeeper.ZooDefs.Ids;
-import org.apache.zookeeper.data.Stat;
 
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -381,214 +379,6 @@ public class ZooKeeperMain {
       }
     }
 
-    /**
-     * trim the quota tree to recover unwanted tree elements
-     * in the quota's tree
-     * @param zk the zookeeper client
-     * @param path the path to start from and go up and see if their
-     * is any unwanted parent in the path.
-     * @return true if sucessful
-     * @throws KeeperException
-     * @throws IOException
-     * @throws InterruptedException
-     */
-    private static boolean trimProcQuotas(ZooKeeper zk, String path)
-        throws KeeperException, IOException, InterruptedException
-    {
-        if (Quotas.quotaZookeeper.equals(path)) {
-            return true;
-        }
-        List<String> children = zk.getChildren(path, false);
-        if (children.size() == 0) {
-            zk.delete(path, -1);
-            String parent = path.substring(0, path.lastIndexOf('/'));
-            return trimProcQuotas(zk, parent);
-        } else {
-            return true;
-        }
-    }
-
-    /**
-     * this method deletes quota for a node.
-     * @param zk the zookeeper client
-     * @param path the path to delete quota for
-     * @param bytes true if number of bytes needs to
-     * be unset
-     * @param numNodes true if number of nodes needs
-     * to be unset
-     * @return true if quota deletion is successful
-     * @throws KeeperException
-     * @throws IOException
-     * @throws InterruptedException
-     */
-    public static boolean delQuota(ZooKeeper zk, String path,
-            boolean bytes, boolean numNodes)
-        throws KeeperException, IOException, InterruptedException
-    {
-        String parentPath = Quotas.quotaZookeeper + path;
-        String quotaPath = Quotas.quotaZookeeper + path + "/" + Quotas.limitNode;
-        if (zk.exists(quotaPath, false) == null) {
-            System.out.println("Quota does not exist for " + path);
-            return true;
-        }
-        byte[] data = null;
-        try {
-            data = zk.getData(quotaPath, false, new Stat());
-        } catch(KeeperException.NoNodeException ne) {
-            System.err.println("quota does not exist for " + path);
-            return true;
-        }
-        StatsTrack strack = new StatsTrack(new String(data));
-        if (bytes && !numNodes) {
-            strack.setBytes(-1L);
-            zk.setData(quotaPath, strack.toString().getBytes(), -1);
-        } else if (!bytes && numNodes) {
-            strack.setCount(-1);
-            zk.setData(quotaPath, strack.toString().getBytes(), -1);
-        } else if (bytes && numNodes) {
-            // delete till you can find a node with more than
-            // one child
-            List<String> children = zk.getChildren(parentPath, false);
-            /// delete the direct children first
-            for (String child: children) {
-                zk.delete(parentPath + "/" + child, -1);
-            }
-            // cut the tree till their is more than one child
-            trimProcQuotas(zk, parentPath);
-        }
-        return true;
-    }
-
-    private static void checkIfParentQuota(ZooKeeper zk, String path)
-        throws InterruptedException, KeeperException
-    {
-        final String[] splits = path.split("/");
-        String quotaPath = Quotas.quotaZookeeper;
-        for (String str: splits) {
-            if (str.length() == 0) {
-                // this should only be for the beginning of the path
-                // i.e. "/..." - split(path)[0] is empty string before first '/'
-                continue;
-            }
-            quotaPath += "/" + str;
-            List<String> children =  null;
-            try {
-                children = zk.getChildren(quotaPath, false);
-            } catch(KeeperException.NoNodeException ne) {
-                LOG.debug("child removed during quota check", ne);
-                return;
-            }
-            if (children.size() == 0) {
-                return;
-            }
-            for (String child: children) {
-                if (Quotas.limitNode.equals(child)) {
-                    throw new IllegalArgumentException(path + " has a parent "
-                            + quotaPath + " which has a quota");
-                }
-            }
-        }
-    }
-
-    /**
-     * this method creates a quota node for the path
-     * @param zk the ZooKeeper client
-     * @param path the path for which quota needs to be created
-     * @param bytes the limit of bytes on this path
-     * @param numNodes the limit of number of nodes on this path
-     * @return true if its successful and false if not.
-     */
-    public static boolean createQuota(ZooKeeper zk, String path,
-            long bytes, int numNodes)
-        throws KeeperException, IOException, InterruptedException
-    {
-        // check if the path exists. We cannot create
-        // quota for a path that already exists in zookeeper
-        // for now.
-        Stat initStat = zk.exists(path, false);
-        if (initStat == null) {
-            throw new IllegalArgumentException(path + " does not exist.");
-        }
-        // now check if their is already existing
-        // parent or child that has quota
-
-        String quotaPath = Quotas.quotaZookeeper;
-        // check for more than 2 children --
-        // if zookeeper_stats and zookeeper_qutoas
-        // are not the children then this path
-        // is an ancestor of some path that
-        // already has quota
-        String realPath = Quotas.quotaZookeeper + path;
-        try {
-            List<String> children = zk.getChildren(realPath, false);
-            for (String child: children) {
-                if (!child.startsWith("zookeeper_")) {
-                    throw new IllegalArgumentException(path + " has child " +
-                            child + " which has a quota");
-                }
-            }
-        } catch(KeeperException.NoNodeException ne) {
-            // this is fine
-        }
-
-        //check for any parent that has been quota
-        checkIfParentQuota(zk, path);
-
-        // this is valid node for quota
-        // start creating all the parents
-        if (zk.exists(quotaPath, false) == null) {
-            try {
-                zk.create(Quotas.procZookeeper, null, Ids.OPEN_ACL_UNSAFE,
-                        CreateMode.PERSISTENT);
-                zk.create(Quotas.quotaZookeeper, null, Ids.OPEN_ACL_UNSAFE,
-                        CreateMode.PERSISTENT);
-            } catch(KeeperException.NodeExistsException ne) {
-                // do nothing
-            }
-        }
-
-        // now create the direct children
-        // and the stat and quota nodes
-        String[] splits = path.split("/");
-        StringBuilder sb = new StringBuilder();
-        sb.append(quotaPath);
-        for (int i=1; i<splits.length; i++) {
-            sb.append("/" + splits[i]);
-            quotaPath = sb.toString();
-            try {
-                zk.create(quotaPath, null, Ids.OPEN_ACL_UNSAFE ,
-                        CreateMode.PERSISTENT);
-            } catch(KeeperException.NodeExistsException ne) {
-                //do nothing
-            }
-        }
-        String statPath = quotaPath + "/" + Quotas.statNode;
-        quotaPath = quotaPath + "/" + Quotas.limitNode;
-        StatsTrack strack = new StatsTrack(null);
-        strack.setBytes(bytes);
-        strack.setCount(numNodes);
-        try {
-            zk.create(quotaPath, strack.toString().getBytes(),
-                    Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-            StatsTrack stats = new StatsTrack(null);
-            stats.setBytes(0L);
-            stats.setCount(0);
-            zk.create(statPath, stats.toString().getBytes(),
-                    Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
-        } catch(KeeperException.NodeExistsException ne) {
-            byte[] data = zk.getData(quotaPath, false , new Stat());
-            StatsTrack strackC = new StatsTrack(new String(data));
-            if (bytes != -1L) {
-                strackC.setBytes(bytes);
-            }
-            if (numNodes != -1) {
-                strackC.setCount(numNodes);
-            }
-            zk.setData(quotaPath, strackC.toString().getBytes(), -1);
-        }
-        return true;
-    }
-
     protected boolean processCmd(MyCommandOptions co) throws CliException, IOException, InterruptedException {
         boolean watch = false;
         try {

zookeeper-server/src/main/java/org/apache/zookeeper/cli/SetQuotaCommand.java

@@ -17,7 +17,7 @@
  */
 package org.apache.zookeeper.cli;
 
-import java.io.IOException;
+import java.util.ArrayList;
 import java.util.List;
 import org.apache.commons.cli.*;
 import org.apache.zookeeper.*;
@@ -65,13 +65,17 @@ public class SetQuotaCommand extends CliCommand {
     public boolean exec() throws CliException {
         // get the args
         String path = args[1];
+        if (path.startsWith(Quotas.quotaZookeeper)) {
+            err.println("cannot set a quota under the path: " + Quotas.quotaZookeeper);
+            return false;
+        }
 
         if (cl.hasOption("b")) {
             // we are setting the bytes quota
             long bytes = Long.parseLong(cl.getOptionValue("b"));
             try {
                 createQuota(zk, path, bytes, -1);
-            } catch (KeeperException|IOException|InterruptedException ex) {
+            } catch (KeeperException|InterruptedException|IllegalArgumentException ex) {
                 throw new CliWrapperException(ex);
             }
         } else if (cl.hasOption("n")) {
@@ -79,7 +83,7 @@ public class SetQuotaCommand extends CliCommand {
             int numNodes = Integer.parseInt(cl.getOptionValue("n"));
             try {
                 createQuota(zk, path, -1L, numNodes);
-            } catch (KeeperException|IOException|InterruptedException ex) {
+            } catch (KeeperException|InterruptedException|IllegalArgumentException ex) {
                 throw new CliWrapperException(ex);
             }
         } else {
@@ -91,7 +95,7 @@ public class SetQuotaCommand extends CliCommand {
 
     public static boolean createQuota(ZooKeeper zk, String path,
             long bytes, int numNodes)
-            throws KeeperException, IOException, InterruptedException, MalformedPathException {
+            throws KeeperException, InterruptedException, IllegalArgumentException, MalformedPathException {
         // check if the path exists. We cannot create
         // quota for a path that already exists in zookeeper
         // for now.
@@ -113,18 +117,9 @@ public class SetQuotaCommand extends CliCommand {
         // are not the children then this path
         // is an ancestor of some path that
         // already has quota
-        String realPath = Quotas.quotaZookeeper + path;
-        try {
-            List<String> children = zk.getChildren(realPath, false);
-            for (String child : children) {
-                if (!child.startsWith("zookeeper_")) {
-                    throw new IllegalArgumentException(path + " has child "
-                            + child + " which has a quota");
-                }
-            }
-        } catch (KeeperException.NoNodeException ne) {
-            // this is fine
-        }
+
+        //check if the child node has a quota.
+        checkIfChildQuota(zk, path);
 
         //check for any parent that has been quota
         checkIfParentQuota(zk, path);
@@ -184,6 +179,40 @@ public class SetQuotaCommand extends CliCommand {
         return true;
     }
 
+    private static void checkIfChildQuota(ZooKeeper zk, String path) throws KeeperException, InterruptedException {
+        String realPath = Quotas.quotaZookeeper + path;
+
+        try {
+            ZKUtil.visitSubTreeDFS(zk, realPath, false, new AsyncCallback.StringCallback() {
+
+                @Override
+                public void processResult(int rc, String quotaPath, Object ctx, String name) {
+                    List<String> children = new ArrayList<>();
+                    try {
+                        children = zk.getChildren(quotaPath, false);
+                    } catch (KeeperException.NoNodeException ne) {
+                        LOG.debug("child removed during quota check", ne);
+                        return;
+                    } catch (InterruptedException | KeeperException e) {
+                        e.printStackTrace();
+                    }
+
+                    if (children.size() == 0) {
+                        return;
+                    }
+                    for (String child : children) {
+                        if (!quotaPath.equals(Quotas.quotaZookeeper + path) && Quotas.limitNode.equals(child)) {
+                            throw new IllegalArgumentException(path + " has a child "
+                                    + quotaPath.substring(Quotas.quotaZookeeper.length()) + " which has a quota");
+                        }
+                    }
+                }
+            });
+        } catch (KeeperException.NoNodeException ne) {
+            // this is fine
+        }
+    }
+
     private static void checkIfParentQuota(ZooKeeper zk, String path)
             throws InterruptedException, KeeperException {
         final String[] splits = path.split("/");
@@ -206,9 +235,9 @@ public class SetQuotaCommand extends CliCommand {
                 return;
             }
             for (String child : children) {
-                if (Quotas.limitNode.equals(child)) {
+                if (!quotaPath.equals(Quotas.quotaZookeeper + path) && Quotas.limitNode.equals(child)) {
                     throw new IllegalArgumentException(path + " has a parent "
-                            + quotaPath + " which has a quota");
+                            + quotaPath.substring(Quotas.quotaZookeeper.length()) + " which has a quota");
                 }
             }
         }

zookeeper-server/src/test/java/org/apache/zookeeper/test/QuorumQuotaTest.java

@@ -24,6 +24,7 @@ import org.apache.zookeeper.StatsTrack;
 import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.ZooKeeperMain;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.cli.SetQuotaCommand;
 import org.apache.zookeeper.data.Stat;
 import org.junit.Assert;
 import org.junit.Test;
@@ -41,7 +42,7 @@ public class QuorumQuotaTest extends QuorumBase {
             zk.create("/a/" + i, "some".getBytes(), Ids.OPEN_ACL_UNSAFE,
                     CreateMode.PERSISTENT);
         }
-        ZooKeeperMain.createQuota(zk, "/a", 1000L, 5000);
+        SetQuotaCommand.createQuota(zk, "/a", 1000L, 5000);
         String statPath = Quotas.quotaZookeeper + "/a"+ "/" + Quotas.statNode;
         byte[] data = zk.getData(statPath, false, new Stat());
         StatsTrack st = new StatsTrack(new String(data));

zookeeper-server/src/test/java/org/apache/zookeeper/test/ZooKeeperQuotaTest.java

@@ -25,8 +25,9 @@ import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.Quotas;
 import org.apache.zookeeper.StatsTrack;
 import org.apache.zookeeper.ZooKeeper;
-import org.apache.zookeeper.ZooKeeperMain;
 import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.cli.MalformedPathException;
+import org.apache.zookeeper.cli.SetQuotaCommand;
 import org.apache.zookeeper.data.Stat;
 import org.apache.zookeeper.server.ZooKeeperServer;
 import org.junit.Assert;
@@ -52,7 +53,7 @@ public class ZooKeeperQuotaTest extends ClientBase {
 
         zk.create("/a/b/v/d", "some".getBytes(), Ids.OPEN_ACL_UNSAFE,
                 CreateMode.PERSISTENT);
-        ZooKeeperMain.createQuota(zk, path, 5L, 10);
+        SetQuotaCommand.createQuota(zk, path, 5L, 10);
 
         // see if its set
         String absolutePath = Quotas.quotaZookeeper + path + "/" + Quotas.limitNode;
@@ -76,4 +77,77 @@ public class ZooKeeperQuotaTest extends ClientBase {
         Assert.assertNotNull("Quota is still set",
             server.getZKDatabase().getDataTree().getMaxPrefixWithQuota(path) != null);
     }
+
+    @Test
+    public void testSetQuota() throws IOException,
+            InterruptedException, KeeperException, MalformedPathException {
+        final ZooKeeper zk = createClient();
+
+        String path = "/c1";
+        String nodeData = "foo";
+        zk.create(path, nodeData.getBytes(), Ids.OPEN_ACL_UNSAFE,
+                CreateMode.PERSISTENT);
+
+        int count = 10;
+        long bytes = 5L;
+        SetQuotaCommand.createQuota(zk, path, bytes, count);
+
+        //check the limit
+        String absoluteLimitPath = Quotas.quotaZookeeper + path + "/" + Quotas.limitNode;
+        byte[] data = zk.getData(absoluteLimitPath, false, null);
+        StatsTrack st = new StatsTrack(new String(data));
+        Assert.assertEquals(bytes, st.getBytes());
+        Assert.assertEquals(count, st.getCount());
+        //check the stats
+        String absoluteStatPath = Quotas.quotaZookeeper + path + "/" + Quotas.statNode;
+        data = zk.getData(absoluteStatPath, false, null);
+        st = new StatsTrack(new String(data));
+        Assert.assertEquals(nodeData.length(), st.getBytes());
+        Assert.assertEquals(1, st.getCount());
+
+        //create another node
+        String path2 = "/c1/c2";
+        String nodeData2 = "bar";
+        zk.create(path2, nodeData2.getBytes(), Ids.OPEN_ACL_UNSAFE,
+                CreateMode.PERSISTENT);
+
+        absoluteStatPath = Quotas.quotaZookeeper + path + "/" + Quotas.statNode;
+        data = zk.getData(absoluteStatPath, false, null);
+        st = new StatsTrack(new String(data));
+        //check the stats
+        Assert.assertEquals(nodeData.length() + nodeData2.length(), st.getBytes());
+        Assert.assertEquals(2, st.getCount());
+    }
+
+    @Test
+    public void testSetQuotaWhenSetQuotaOnParentOrChildPath() throws IOException,
+            InterruptedException, KeeperException, MalformedPathException {
+        final ZooKeeper zk = createClient();
+
+        zk.create("/c1", "some".getBytes(), Ids.OPEN_ACL_UNSAFE,
+                CreateMode.PERSISTENT);
+        zk.create("/c1/c2", "some".getBytes(), Ids.OPEN_ACL_UNSAFE,
+                CreateMode.PERSISTENT);
+        zk.create("/c1/c2/c3", "some".getBytes(), Ids.OPEN_ACL_UNSAFE,
+                CreateMode.PERSISTENT);
+        zk.create("/c1/c2/c3/c4", "some".getBytes(), Ids.OPEN_ACL_UNSAFE,
+                CreateMode.PERSISTENT);
+        zk.create("/c1/c2/c3/c4/c5", "some".getBytes(), Ids.OPEN_ACL_UNSAFE,
+                CreateMode.PERSISTENT);
+
+        //set the quota on the path:/c1/c2/c3
+        SetQuotaCommand.createQuota(zk, "/c1/c2/c3", 5L, 10);
+
+        try {
+            SetQuotaCommand.createQuota(zk, "/c1", 5L, 10);
+        } catch (IllegalArgumentException e) {
+            Assert.assertEquals("/c1 has a child /c1/c2/c3 which has a quota", e.getMessage());
+        }
+
+        try {
+            SetQuotaCommand.createQuota(zk, "/c1/c2/c3/c4/c5", 5L, 10);
+        } catch (IllegalArgumentException e) {
+            Assert.assertEquals("/c1/c2/c3/c4/c5 has a parent /c1/c2/c3 which has a quota", e.getMessage());
+        }
+    }
 }