[GitHub] [bookkeeper] dlg99 commented on a change in pull request #2805: replication stat num-under-replicated-ledgers changed as with the pro…

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

dlg99 commented on a change in pull request #2805:
URL: https://github.com/apache/bookkeeper/pull/2805#discussion_r715078552



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java
##########
@@ -998,6 +1006,13 @@ public void run() {
         }), initialDelay, interval, TimeUnit.SECONDS);
     }
 
+    private class UnderReplicatedLedgersChangedCb implements GenericCallback<Void> {
+        @Override
+        public void operationComplete(int rc, Void result) {
+            numUnderReplicatedLedger.registerSuccessfulValue(-1);

Review comment:
       Please double check me, my understanding is:
   numUnderReplicatedLedger is an OpStatsLogger. This works for registerSuccessfulValue only with Prometheus, afaict Codahale simply treats it as a duration in millis (similar to the registerSuccessfulEvent but with time init set in millis) and adds into a histogram.
   But that's a side problem.
   
   If your intent is to "know the progress of replication from the stat" then you are getting into a situation when the metric can increase and decrease during the reporting interval and you won't see the progress.
   I'd suggest to have a gauge with number of currently under replicated ledgers (numUnderreplicatedLedgersElapsedRecoveryGracePeriod is doing this right now) and a counter incremented for successfully replicated ledgers. Note that ledger can get out of under replicated state by either being re-replicated or if a missing bookie returns back into the cluster.
   
   




-- 
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: issues-unsubscribe@bookkeeper.apache.org

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