hangc0276 commented on code in PR #4058:
URL: https://github.com/apache/bookkeeper/pull/4058#discussion_r1305264179


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/LedgerUnderreplicationManager.java:
##########
@@ -240,6 +240,7 @@ boolean initializeLostBookieRecoveryDelay(int 
lostBookieRecoveryDelay)
      * @param cb
      * @throws ReplicationException.UnavailableException
      */
+    @Deprecated

Review Comment:
   We'd better add a default empty implementation for the interface, otherwise, 
all the implementation classes need to implement this method



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java:
##########
@@ -558,14 +556,26 @@ private void scheduleReplicasCheckTask() {
         executor.scheduleAtFixedRate(auditorReplicasCheckTask, initialDelay, 
interval, TimeUnit.SECONDS);
     }
 
-    private class UnderReplicatedLedgersChangedCb implements 
GenericCallback<Void> {
-        @Override
-        public void operationComplete(int rc, Void result) {
+    private void scheduleReplicateStatTask() {
+        long interval = conf.getAuditorUnderReplicasStatInterval();
+        if (interval <= 0) {
+            LOG.info("Under Replicas Stats disabled");
+            return;
+        }
+        auditorStatExecutor = Executors.newSingleThreadScheduledExecutor(

Review Comment:
   Why not reuse the existing executor?



##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java:
##########
@@ -558,14 +556,26 @@ private void scheduleReplicasCheckTask() {
         executor.scheduleAtFixedRate(auditorReplicasCheckTask, initialDelay, 
interval, TimeUnit.SECONDS);
     }
 
-    private class UnderReplicatedLedgersChangedCb implements 
GenericCallback<Void> {
-        @Override
-        public void operationComplete(int rc, Void result) {
+    private void scheduleReplicateStatTask() {
+        long interval = conf.getAuditorUnderReplicasStatInterval();
+        if (interval <= 0) {
+            LOG.info("Under Replicas Stats disabled");
+            return;
+        }
+        auditorStatExecutor = Executors.newSingleThreadScheduledExecutor(
+                new ThreadFactory() {
+                    @Override
+                    public Thread newThread(Runnable r) {
+                        Thread t = new Thread(r, "AuditorStat-" + 
bookieIdentifier);
+                        t.setDaemon(true);
+                        return t;
+                    }
+                });
+        auditorStatExecutor.scheduleAtFixedRate(() -> {
             Iterator<UnderreplicatedLedger> underreplicatedLedgersInfo = 
ledgerUnderreplicationManager
                     .listLedgersToRereplicate(null);
             
auditorStats.getUnderReplicatedLedgersGuageValue().set(Iterators.size(underreplicatedLedgersInfo));

Review Comment:
   We update the Gauge metric with a specific interval, and I'm not sure 
whether it is meaningful for the metric on the dashboard.



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to