From 948c39c7a3c512756838b6ea5fb28a201004850c Mon Sep 17 00:00:00 2001
From: hrsakai <hsakai@yahoo-corp.jp>
Date: Fri, 17 Sep 2021 09:44:33 +0900
Subject: [PATCH 1/4] Make maxConcurrentZkRequests for gc configurable

---
 .../ScanAndCompareGarbageCollector.java       | 12 +++++++----
 .../bookkeeper/conf/ServerConfiguration.java  | 21 +++++++++++++++++++
 conf/bk_server.conf                           |  3 +++
 site/_data/config/bk_server.yaml              |  3 +++
 4 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
index faac25949ec..5774e8dabdb 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
@@ -73,7 +73,6 @@
 public class ScanAndCompareGarbageCollector implements GarbageCollector {
 
     static final Logger LOG = LoggerFactory.getLogger(ScanAndCompareGarbageCollector.class);
-    static final int MAX_CONCURRENT_METADATA_REQUESTS = 1000;
 
     private final LedgerManager ledgerManager;
     private final CompactableLedgerStorage ledgerStorage;
@@ -85,6 +84,7 @@ public class ScanAndCompareGarbageCollector implements GarbageCollector {
     private final boolean verifyMetadataOnGc;
     private int activeLedgerCounter;
     private StatsLogger statsLogger;
+    private final int maxConcurrentZkRequests;
 
     public ScanAndCompareGarbageCollector(LedgerManager ledgerManager, CompactableLedgerStorage ledgerStorage,
             ServerConfiguration conf, StatsLogger statsLogger) throws IOException {
@@ -99,12 +99,14 @@ public ScanAndCompareGarbageCollector(LedgerManager ledgerManager, CompactableLe
         if (gcOverReplicatedLedgerIntervalMillis > 0) {
             this.enableGcOverReplicatedLedger = true;
         }
-        LOG.info("Over Replicated Ledger Deletion : enabled=" + enableGcOverReplicatedLedger + ", interval="
-                + gcOverReplicatedLedgerIntervalMillis);
+        this.maxConcurrentZkRequests = conf.getGcOverreplicatedLedgerMaxConcurrentZkRequests();
+        LOG.info("Over Replicated Ledger Deletion : enabled={}, interval={}, maxConcurrentZkRequest={}",
+                enableGcOverReplicatedLedger, gcOverReplicatedLedgerIntervalMillis, maxConcurrentZkRequests);
 
         verifyMetadataOnGc = conf.getVerifyMetadataOnGC();
 
         this.activeLedgerCounter = 0;
+
     }
 
     public int getNumActiveLedgers() {
@@ -129,6 +131,8 @@ public void gc(GarbageCleaner garbageCleaner) {
             boolean checkOverreplicatedLedgers = (enableGcOverReplicatedLedger && curTime
                     - lastOverReplicatedLedgerGcTimeMillis > gcOverReplicatedLedgerIntervalMillis);
             if (checkOverreplicatedLedgers) {
+                LOG.info("Start removing over-replicated ledgers. activeLedgerCounter={}", activeLedgerCounter);
+
                 // remove all the overreplicated ledgers from the local bookie
                 Set<Long> overReplicatedLedgers = removeOverReplicatedledgers(bkActiveLedgers, garbageCleaner);
                 if (overReplicatedLedgers.isEmpty()) {
@@ -216,7 +220,7 @@ public void gc(GarbageCleaner garbageCleaner) {
     private Set<Long> removeOverReplicatedledgers(Set<Long> bkActiveledgers, final GarbageCleaner garbageCleaner)
             throws Exception {
         final Set<Long> overReplicatedLedgers = Sets.newHashSet();
-        final Semaphore semaphore = new Semaphore(MAX_CONCURRENT_METADATA_REQUESTS);
+        final Semaphore semaphore = new Semaphore(this.maxConcurrentZkRequests);
         final CountDownLatch latch = new CountDownLatch(bkActiveledgers.size());
         // instantiate zookeeper client to initialize ledger manager
 
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
index 14666550daa..5bc4412b901 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
@@ -111,6 +111,7 @@ public class ServerConfiguration extends AbstractConfiguration<ServerConfigurati
     protected static final String GC_WAIT_TIME = "gcWaitTime";
     protected static final String IS_FORCE_GC_ALLOW_WHEN_NO_SPACE = "isForceGCAllowWhenNoSpace";
     protected static final String GC_OVERREPLICATED_LEDGER_WAIT_TIME = "gcOverreplicatedLedgerWaitTime";
+    protected static final String GC_OVERREPLICATED_LEDGER_MAX_CONCURRENT_ZK_REQUESTS = "gcOverreplicatedLedgerMaxConcurrentZkRequests";
     protected static final String USE_TRANSACTIONAL_COMPACTION = "useTransactionalCompaction";
     protected static final String VERIFY_METADATA_ON_GC = "verifyMetadataOnGC";
     // Scrub Parameters
@@ -418,6 +419,26 @@ public ServerConfiguration setGcOverreplicatedLedgerWaitTime(long gcWaitTime, Ti
         return this;
     }
 
+    /**
+     * Max number of concurrent requests to zk in garbage collection of overreplicated ledgers.
+     *
+     * @return max number of concurrent requests to zk
+     */
+    public int getGcOverreplicatedLedgerMaxConcurrentZkRequests() {
+        return this.getInt(GC_OVERREPLICATED_LEDGER_MAX_CONCURRENT_ZK_REQUESTS, 1000);
+    }
+
+    /**
+     * Max number of concurrent requests to zk in garbage collection of overreplicated ledgers. Default: 1000
+     *
+     * @param gcOverreplicatedLedgerMaxConcurrentZkRequests
+     * @return server configuration
+     */
+    public ServerConfiguration setGcOverreplicatedLedgerMaxConcurrentZkRequests(int gcOverreplicatedLedgerMaxConcurrentZkRequests) {
+        this.setProperty(GC_OVERREPLICATED_LEDGER_MAX_CONCURRENT_ZK_REQUESTS, Integer.toString(gcOverreplicatedLedgerMaxConcurrentZkRequests));
+        return this;
+    }
+
     /**
      * Get whether to use transactional compaction and using a separate log for compaction or not.
      *
diff --git a/conf/bk_server.conf b/conf/bk_server.conf
index 3941fe1d9d1..4bf683468d7 100755
--- a/conf/bk_server.conf
+++ b/conf/bk_server.conf
@@ -576,6 +576,9 @@ ledgerDirectories=/tmp/bk-data
 # since we read the metadata for all the ledgers on the bookie from zk
 # gcOverreplicatedLedgerWaitTime=86400000
 
+# Max number of concurrent requests to zk in garbage collection of overreplicated ledgers.
+# gcOverreplicatedLedgerMaxConcurrentZkRequests=1000
+
 # Whether force compaction is allowed when the disk is full or almost full.
 # Forcing GC may get some space back, but may also fill up disk space more quickly.
 # This is because new log files are created before GC, while old garbage
diff --git a/site/_data/config/bk_server.yaml b/site/_data/config/bk_server.yaml
index fd4c9ea2108..89e9c46175b 100644
--- a/site/_data/config/bk_server.yaml
+++ b/site/_data/config/bk_server.yaml
@@ -380,6 +380,9 @@ groups:
   - param: gcOverreplicatedLedgerWaitTime
     description: How long the interval to trigger next garbage collection of overreplicated ledgers, in milliseconds. This should not be run very frequently since we read the metadata for all the ledgers on the bookie from zk.
     default: 86400000
+  - param: gcOverreplicatedLedgerMaxConcurrentZkRequests
+    description: Max number of concurrent requests to zk in garbage collection of overreplicated ledgers.
+    default: 1000
   - param: isForceGCAllowWhenNoSpace
     description: Whether force compaction is allowed when the disk is full or almost full. Forcing GC may get some space back, but may also fill up disk space more quickly. This is because new log files are created before GC, while old garbage log files are deleted after GC.
     default: 'false'

From f562f08eaab072e2614a3ac81412254e22cca62d Mon Sep 17 00:00:00 2001
From: hrsakai <hsakai@yahoo-corp.jp>
Date: Fri, 17 Sep 2021 13:11:10 +0900
Subject: [PATCH 2/4] Fix format

---
 .../org/apache/bookkeeper/conf/ServerConfiguration.java  | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
index 5bc4412b901..2712217b003 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
@@ -111,7 +111,8 @@ public class ServerConfiguration extends AbstractConfiguration<ServerConfigurati
     protected static final String GC_WAIT_TIME = "gcWaitTime";
     protected static final String IS_FORCE_GC_ALLOW_WHEN_NO_SPACE = "isForceGCAllowWhenNoSpace";
     protected static final String GC_OVERREPLICATED_LEDGER_WAIT_TIME = "gcOverreplicatedLedgerWaitTime";
-    protected static final String GC_OVERREPLICATED_LEDGER_MAX_CONCURRENT_ZK_REQUESTS = "gcOverreplicatedLedgerMaxConcurrentZkRequests";
+    protected static final String GC_OVERREPLICATED_LEDGER_MAX_CONCURRENT_ZK_REQUESTS =
+            "gcOverreplicatedLedgerMaxConcurrentZkRequests";
     protected static final String USE_TRANSACTIONAL_COMPACTION = "useTransactionalCompaction";
     protected static final String VERIFY_METADATA_ON_GC = "verifyMetadataOnGC";
     // Scrub Parameters
@@ -434,8 +435,10 @@ public int getGcOverreplicatedLedgerMaxConcurrentZkRequests() {
      * @param gcOverreplicatedLedgerMaxConcurrentZkRequests
      * @return server configuration
      */
-    public ServerConfiguration setGcOverreplicatedLedgerMaxConcurrentZkRequests(int gcOverreplicatedLedgerMaxConcurrentZkRequests) {
-        this.setProperty(GC_OVERREPLICATED_LEDGER_MAX_CONCURRENT_ZK_REQUESTS, Integer.toString(gcOverreplicatedLedgerMaxConcurrentZkRequests));
+    public ServerConfiguration setGcOverreplicatedLedgerMaxConcurrentZkRequests(
+            int gcOverreplicatedLedgerMaxConcurrentZkRequests) {
+        this.setProperty(GC_OVERREPLICATED_LEDGER_MAX_CONCURRENT_ZK_REQUESTS,
+                Integer.toString(gcOverreplicatedLedgerMaxConcurrentZkRequests));
         return this;
     }
 

From b742d5dcf06a36a9127ce05a651bffc2f13d005e Mon Sep 17 00:00:00 2001
From: hrsakai <hsakai@yahoo-corp.jp>
Date: Fri, 17 Sep 2021 17:09:53 +0900
Subject: [PATCH 3/4] Rename to
 GC_OVERREPLICATED_LEDGER_MAX_CONCURRENT_REQUESTS

---
 .../ScanAndCompareGarbageCollector.java       | 10 ++++----
 .../bookkeeper/conf/ServerConfiguration.java  | 24 +++++++++----------
 conf/bk_server.conf                           |  4 ++--
 site/_data/config/bk_server.yaml              |  4 ++--
 4 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
index 5774e8dabdb..70f7016749f 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
@@ -84,7 +84,7 @@ public class ScanAndCompareGarbageCollector implements GarbageCollector {
     private final boolean verifyMetadataOnGc;
     private int activeLedgerCounter;
     private StatsLogger statsLogger;
-    private final int maxConcurrentZkRequests;
+    private final int maxConcurrentRequests;
 
     public ScanAndCompareGarbageCollector(LedgerManager ledgerManager, CompactableLedgerStorage ledgerStorage,
             ServerConfiguration conf, StatsLogger statsLogger) throws IOException {
@@ -99,9 +99,9 @@ public ScanAndCompareGarbageCollector(LedgerManager ledgerManager, CompactableLe
         if (gcOverReplicatedLedgerIntervalMillis > 0) {
             this.enableGcOverReplicatedLedger = true;
         }
-        this.maxConcurrentZkRequests = conf.getGcOverreplicatedLedgerMaxConcurrentZkRequests();
-        LOG.info("Over Replicated Ledger Deletion : enabled={}, interval={}, maxConcurrentZkRequest={}",
-                enableGcOverReplicatedLedger, gcOverReplicatedLedgerIntervalMillis, maxConcurrentZkRequests);
+        this.maxConcurrentRequests = conf.getGcOverreplicatedLedgerMaxConcurrentRequests();
+        LOG.info("Over Replicated Ledger Deletion : enabled={}, interval={}, maxConcurrentRequest={}",
+                enableGcOverReplicatedLedger, gcOverReplicatedLedgerIntervalMillis, maxConcurrentRequests);
 
         verifyMetadataOnGc = conf.getVerifyMetadataOnGC();
 
@@ -220,7 +220,7 @@ public void gc(GarbageCleaner garbageCleaner) {
     private Set<Long> removeOverReplicatedledgers(Set<Long> bkActiveledgers, final GarbageCleaner garbageCleaner)
             throws Exception {
         final Set<Long> overReplicatedLedgers = Sets.newHashSet();
-        final Semaphore semaphore = new Semaphore(this.maxConcurrentZkRequests);
+        final Semaphore semaphore = new Semaphore(this.maxConcurrentRequests);
         final CountDownLatch latch = new CountDownLatch(bkActiveledgers.size());
         // instantiate zookeeper client to initialize ledger manager
 
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
index 2712217b003..8fffdde0d7e 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
@@ -111,8 +111,8 @@ public class ServerConfiguration extends AbstractConfiguration<ServerConfigurati
     protected static final String GC_WAIT_TIME = "gcWaitTime";
     protected static final String IS_FORCE_GC_ALLOW_WHEN_NO_SPACE = "isForceGCAllowWhenNoSpace";
     protected static final String GC_OVERREPLICATED_LEDGER_WAIT_TIME = "gcOverreplicatedLedgerWaitTime";
-    protected static final String GC_OVERREPLICATED_LEDGER_MAX_CONCURRENT_ZK_REQUESTS =
-            "gcOverreplicatedLedgerMaxConcurrentZkRequests";
+    protected static final String GC_OVERREPLICATED_LEDGER_MAX_CONCURRENT_REQUESTS =
+            "gcOverreplicatedLedgerMaxConcurrentRequests";
     protected static final String USE_TRANSACTIONAL_COMPACTION = "useTransactionalCompaction";
     protected static final String VERIFY_METADATA_ON_GC = "verifyMetadataOnGC";
     // Scrub Parameters
@@ -421,24 +421,24 @@ public ServerConfiguration setGcOverreplicatedLedgerWaitTime(long gcWaitTime, Ti
     }
 
     /**
-     * Max number of concurrent requests to zk in garbage collection of overreplicated ledgers.
+     * Max number of concurrent requests in garbage collection of overreplicated ledgers.
      *
-     * @return max number of concurrent requests to zk
+     * @return max number of concurrent requests
      */
-    public int getGcOverreplicatedLedgerMaxConcurrentZkRequests() {
-        return this.getInt(GC_OVERREPLICATED_LEDGER_MAX_CONCURRENT_ZK_REQUESTS, 1000);
+    public int getGcOverreplicatedLedgerMaxConcurrentRequests() {
+        return this.getInt(GC_OVERREPLICATED_LEDGER_MAX_CONCURRENT_REQUESTS, 1000);
     }
 
     /**
-     * Max number of concurrent requests to zk in garbage collection of overreplicated ledgers. Default: 1000
+     * Max number of concurrent requests in garbage collection of overreplicated ledgers. Default: 1000
      *
-     * @param gcOverreplicatedLedgerMaxConcurrentZkRequests
+     * @param gcOverreplicatedLedgerMaxConcurrentRequests
      * @return server configuration
      */
-    public ServerConfiguration setGcOverreplicatedLedgerMaxConcurrentZkRequests(
-            int gcOverreplicatedLedgerMaxConcurrentZkRequests) {
-        this.setProperty(GC_OVERREPLICATED_LEDGER_MAX_CONCURRENT_ZK_REQUESTS,
-                Integer.toString(gcOverreplicatedLedgerMaxConcurrentZkRequests));
+    public ServerConfiguration setGcOverreplicatedLedgerMaxConcurrentRequests(
+            int gcOverreplicatedLedgerMaxConcurrentRequests) {
+        this.setProperty(GC_OVERREPLICATED_LEDGER_MAX_CONCURRENT_REQUESTS,
+                Integer.toString(gcOverreplicatedLedgerMaxConcurrentRequests));
         return this;
     }
 
diff --git a/conf/bk_server.conf b/conf/bk_server.conf
index 4bf683468d7..e2abc5f0e69 100755
--- a/conf/bk_server.conf
+++ b/conf/bk_server.conf
@@ -576,8 +576,8 @@ ledgerDirectories=/tmp/bk-data
 # since we read the metadata for all the ledgers on the bookie from zk
 # gcOverreplicatedLedgerWaitTime=86400000
 
-# Max number of concurrent requests to zk in garbage collection of overreplicated ledgers.
-# gcOverreplicatedLedgerMaxConcurrentZkRequests=1000
+# Max number of concurrent requests in garbage collection of overreplicated ledgers.
+# gcOverreplicatedLedgerMaxConcurrentRequests=1000
 
 # Whether force compaction is allowed when the disk is full or almost full.
 # Forcing GC may get some space back, but may also fill up disk space more quickly.
diff --git a/site/_data/config/bk_server.yaml b/site/_data/config/bk_server.yaml
index 89e9c46175b..1176f34044f 100644
--- a/site/_data/config/bk_server.yaml
+++ b/site/_data/config/bk_server.yaml
@@ -380,8 +380,8 @@ groups:
   - param: gcOverreplicatedLedgerWaitTime
     description: How long the interval to trigger next garbage collection of overreplicated ledgers, in milliseconds. This should not be run very frequently since we read the metadata for all the ledgers on the bookie from zk.
     default: 86400000
-  - param: gcOverreplicatedLedgerMaxConcurrentZkRequests
-    description: Max number of concurrent requests to zk in garbage collection of overreplicated ledgers.
+  - param: gcOverreplicatedLedgerMaxConcurrentRequests
+    description: Max number of concurrent requests in garbage collection of overreplicated ledgers.
     default: 1000
   - param: isForceGCAllowWhenNoSpace
     description: Whether force compaction is allowed when the disk is full or almost full. Forcing GC may get some space back, but may also fill up disk space more quickly. This is because new log files are created before GC, while old garbage log files are deleted after GC.

From 0118a2785e927130b91329097497a7274177a872 Mon Sep 17 00:00:00 2001
From: hrsakai <hsakai@yahoo-corp.jp>
Date: Sat, 18 Sep 2021 22:18:55 +0900
Subject: [PATCH 4/4] Fix typo

---
 .../bookkeeper/bookie/ScanAndCompareGarbageCollector.java      | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
index 70f7016749f..1d28dd13e6b 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
@@ -100,13 +100,12 @@ public ScanAndCompareGarbageCollector(LedgerManager ledgerManager, CompactableLe
             this.enableGcOverReplicatedLedger = true;
         }
         this.maxConcurrentRequests = conf.getGcOverreplicatedLedgerMaxConcurrentRequests();
-        LOG.info("Over Replicated Ledger Deletion : enabled={}, interval={}, maxConcurrentRequest={}",
+        LOG.info("Over Replicated Ledger Deletion : enabled={}, interval={}, maxConcurrentRequests={}",
                 enableGcOverReplicatedLedger, gcOverReplicatedLedgerIntervalMillis, maxConcurrentRequests);
 
         verifyMetadataOnGc = conf.getVerifyMetadataOnGC();
 
         this.activeLedgerCounter = 0;
-
     }
 
     public int getNumActiveLedgers() {