[GitHub] [zookeeper] muse-dev[bot] commented on a change in pull request #934: ZOOKEEPER-3301:Enforce the quota limit

[GitBox <gi...@apache.org>]

muse-dev[bot] commented on a change in pull request #934:
URL: https://github.com/apache/zookeeper/pull/934#discussion_r511423797



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/PrepRequestProcessor.java
##########
@@ -393,6 +393,7 @@ protected void pRequest2Txn(int type, long zxid, Request request, Record record,
             validatePath(path, request.sessionId);
             nodeRecord = getRecordForPath(path);
             zks.checkACL(request.cnxn, nodeRecord.acl, ZooDefs.Perms.WRITE, request.authInfo, path, null);
+            zks.checkQuota(path, setDataRequest.getData(), OpCode.setData);

Review comment:
       *THREAD_SAFETY_VIOLATION:*  Unprotected write. Non-private method `PrepRequestProcessor.pRequest2Txn(...)` indirectly writes to field `server.ZooKeeperServer.RATE_LOGGER.count` outside of synchronization.
    Reporting because this access may occur on a background thread.

##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
##########
@@ -1946,6 +1960,123 @@ public void checkACL(ServerCnxn cnxn, List<ACL> acl, int perm, List<Id> ids, Str
         throw new KeeperException.NoAuthException();
     }
 
+    /**
+     * check a path whether exceeded the quota.
+     *
+     * @param path
+     *            the path of the node
+     * @param data
+     *            the data of the path
+     * @param type
+     *            currently, create and setData need to check quota
+     */
+
+    public void checkQuota(String path, byte[] data, int type) throws KeeperException.QuotaExceededException {
+        if (!enforceQuota) {
+            return;
+        }
+        long dataBytes = (data == null) ? 0 : data.length;
+        ZKDatabase zkDatabase = getZKDatabase();

Review comment:
       *THREAD_SAFETY_VIOLATION:*  Read/Write race. Non-private method `ZooKeeperServer.checkQuota(...)` indirectly reads without synchronization from `this.zkDb`. Potentially races with write in method `ZooKeeperServer.startdata()`.
    Reporting because this access may occur on a background thread.

##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
##########
@@ -1946,6 +1960,123 @@ public void checkACL(ServerCnxn cnxn, List<ACL> acl, int perm, List<Id> ids, Str
         throw new KeeperException.NoAuthException();
     }
 
+    /**
+     * check a path whether exceeded the quota.
+     *
+     * @param path
+     *            the path of the node
+     * @param data
+     *            the data of the path
+     * @param type
+     *            currently, create and setData need to check quota
+     */
+
+    public void checkQuota(String path, byte[] data, int type) throws KeeperException.QuotaExceededException {
+        if (!enforceQuota) {
+            return;
+        }
+        long dataBytes = (data == null) ? 0 : data.length;
+        ZKDatabase zkDatabase = getZKDatabase();
+        String lastPrefix = zkDatabase.getDataTree().getMaxPrefixWithQuota(path);
+        if (StringUtils.isEmpty(lastPrefix)) {
+            return;
+        }
+
+        switch (type) {
+            case OpCode.create:
+                checkQuota(lastPrefix, dataBytes, 1);

Review comment:
       *THREAD_SAFETY_VIOLATION:*  Unprotected write. Non-private method `ZooKeeperServer.checkQuota(...)` indirectly writes to field `server.ZooKeeperServer.RATE_LOGGER.count` outside of synchronization.
    Reporting because this access may occur on a background thread.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org