[GitHub] [zookeeper] hanm commented on a change in pull request #1316: ZOOKEEPER-3793: Request throttling is broken when RequestThrottler is disabled or configured incorrectly..

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

hanm commented on a change in pull request #1316: ZOOKEEPER-3793: Request throttling is broken when RequestThrottler is disabled or configured incorrectly..
URL: https://github.com/apache/zookeeper/pull/1316#discussion_r406884310
 
 

 ##########
 File path: zookeeper-server/src/test/java/org/apache/zookeeper/server/RequestThrottlerTest.java
 ##########
 @@ -307,4 +308,37 @@ public void testLargeRequestThrottling() throws Exception {
         metrics = MetricsUtils.currentServerMetrics();
         Assert.assertEquals(2, (long) metrics.get("stale_replies"));
     }
+
+    @Test
+    public void testGlobalOutstandingRequestThrottlingWithRequestThrottlerDisabled() throws Exception {
+        System.setProperty(ZooKeeperServer.GLOBAL_OUTSTANDING_LIMIT, GLOBAL_OUTSTANDING_LIMIT);
+        ServerMetrics.getMetrics().resetAll();
+
+        // Here we disable RequestThrottler and let incoming requests queued at first request processor.
+        RequestThrottler.setMaxRequests(0);
+        resumeProcess = new CountDownLatch(1);
+        int totalRequests = 10;
+        submitted = new CountDownLatch(totalRequests);
+
+        for (int i = 0; i < totalRequests; i++) {
+            zk.create("/request_throttle_test- " + i, ("/request_throttle_test- "
+                    + i).getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT, (rc, path, ctx, name) -> {
+            }, null);
+        }
+
+        submitted.await(5, TimeUnit.SECONDS);
+
+        resumeProcess.countDown();
+
+        // We should start throttling instead of queuing more requests.
+        //
+        // We always allow up to GLOBAL_OUTSTANDING_LIMIT + 1 number of requests coming in request processing pipeline
+        // before throttling. For the next request, we will throttle by disabling receiving future requests but we still
+        // allow this single request coming in. So the total number of queued requests in processing pipeline would
+        // be GLOBAL_OUTSTANDING_LIMIT + 2.
+        assertEquals(Integer.parseInt(GLOBAL_OUTSTANDING_LIMIT) + 2,
+                (long) MetricsUtils.currentServerMetrics().get("prep_processor_request_queued"));
+
+        System.clearProperty(ZooKeeperServer.GLOBAL_OUTSTANDING_LIMIT);
 
 Review comment:
   updated

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


With regards,
Apache Git Services