ZOOKEEPER-4915: Default znode.container.maxNeverUsedIntervalMs to 5 minutes

Author: kezhuw
Closes #2248 from kezhuw/ZOOKEEPER-4915-znode.container.maxNeverUsedIntervalMs-default-value

Author: kezhuw

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

@@ -2160,8 +2160,9 @@ Both subsystems need to have sufficient amount of threads to achieve peak read t
     maximum interval in milliseconds that a container that has never had
     any children is retained. Should be long enough for your client to
     create the container, do any needed work and then create children.
-    Default is "0" which is used to indicate that containers
-    that have never had any children are never deleted.
+    Default is "300000"(a.k.a. 5 minutes) since 3.10.0, for earlier versions,
+    it is "0" which is used to indicate that containers that have never had
+    any children are never deleted.
 
 <a name="sc_debug_observability_config"></a>
 

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

@@ -50,6 +50,20 @@ public class ContainerManager {
     private final Timer timer;
     private final AtomicReference<TimerTask> task = new AtomicReference<>(null);
 
+    /**
+     * @param zkDb the ZK database
+     * @param requestProcessor request processor - used to inject delete
+     *                         container requests
+     */
+    public ContainerManager(ZKDatabase zkDb, RequestProcessor requestProcessor) {
+        this(
+            zkDb, requestProcessor,
+            Integer.getInteger("znode.container.checkIntervalMs", (int) TimeUnit.MINUTES.toMillis(1)),
+            Integer.getInteger("znode.container.maxPerMinute", 10000),
+            Long.getLong("znode.container.maxNeverUsedIntervalMs", TimeUnit.MINUTES.toMillis(5))
+        );
+    }
+
     /**
      * @param zkDb the ZK database
      * @param requestProcessor request processor - used to inject delete
@@ -155,6 +169,18 @@ protected long getMinIntervalMs() {
         return TimeUnit.MINUTES.toMillis(1) / maxPerMinute;
     }
 
+    int getMaxPerMinute() {
+        return maxPerMinute;
+    }
+
+    int getCheckIntervalMs() {
+        return checkIntervalMs;
+    }
+
+    long getMaxNeverUsedIntervalMs() {
+        return maxNeverUsedIntervalMs;
+    }
+
     // VisibleForTesting
     protected Collection<String> getCandidates() {
         Set<String> candidates = new HashSet<>();

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

@@ -20,7 +20,6 @@
 
 import java.io.IOException;
 import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
 import javax.management.JMException;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.zookeeper.audit.ZKAuditProvider;
@@ -168,13 +167,7 @@ public void runFromConfig(ServerConfig config) throws IOException, AdminServerEx
                 secureCnxnFactory.startup(zkServer, needStartZKServer);
             }
 
-            containerManager = new ContainerManager(
-                zkServer.getZKDatabase(),
-                zkServer.firstProcessor,
-                Integer.getInteger("znode.container.checkIntervalMs", (int) TimeUnit.MINUTES.toMillis(1)),
-                Integer.getInteger("znode.container.maxPerMinute", 10000),
-                Long.getLong("znode.container.maxNeverUsedIntervalMs", 0)
-            );
+            containerManager = new ContainerManager(zkServer.getZKDatabase(), zkServer.firstProcessor);
             containerManager.start();
             ZKAuditProvider.addZKStartStopAuditLog();
 

zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LeaderZooKeeperServer.java

@@ -19,7 +19,6 @@
 package org.apache.zookeeper.server.quorum;
 
 import java.io.IOException;
-import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
 import javax.management.JMException;
 import org.apache.zookeeper.KeeperException.SessionExpiredException;
@@ -78,13 +77,7 @@ protected void setupRequestProcessors() {
     }
 
     private synchronized void setupContainerManager() {
-        containerManager = new ContainerManager(
-            getZKDatabase(),
-            prepRequestProcessor,
-            Integer.getInteger("znode.container.checkIntervalMs", (int) TimeUnit.MINUTES.toMillis(1)),
-            Integer.getInteger("znode.container.maxPerMinute", 10000),
-            Long.getLong("znode.container.maxNeverUsedIntervalMs", 0)
-        );
+        containerManager = new ContainerManager(getZKDatabase(), prepRequestProcessor);
     }
 
     @Override

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

@@ -25,6 +25,7 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 import java.io.IOException;
+import java.time.Duration;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -77,10 +78,30 @@ public void addCommittedProposal(Request request) {
     @AfterEach
     @Override
     public void tearDown() throws Exception {
+        System.clearProperty("znode.container.checkIntervalMs");
+        System.clearProperty("znode.container.maxPerMinute");
+        System.clearProperty("znode.container.maxNeverUsedInterval");
+
         super.tearDown();
         zk.close();
     }
 
+    @Test
+    public void testContainerManagerDefaults() {
+        ContainerManager containerManager = new ContainerManager(serverFactory.getZooKeeperServer().getZKDatabase(), serverFactory.getZooKeeperServer().firstProcessor);
+        assertEquals((int) Duration.ofMinutes(1).toMillis(), containerManager.getCheckIntervalMs());
+        assertEquals(10000, containerManager.getMaxPerMinute());
+        assertEquals((int) Duration.ofMinutes(5).toMillis(), containerManager.getMaxNeverUsedIntervalMs());
+
+        System.setProperty("znode.container.checkIntervalMs", "1000");
+        System.setProperty("znode.container.maxPerMinute", "1000");
+        System.setProperty("znode.container.maxNeverUsedIntervalMs", "10000");
+        containerManager = new ContainerManager(serverFactory.getZooKeeperServer().getZKDatabase(), serverFactory.getZooKeeperServer().firstProcessor);
+        assertEquals(1000, containerManager.getCheckIntervalMs());
+        assertEquals(1000, containerManager.getMaxPerMinute());
+        assertEquals(10000, containerManager.getMaxNeverUsedIntervalMs());
+    }
+
     @Test
     @Timeout(value = 30)
     public void testCreate() throws KeeperException, InterruptedException {