[GitHub] [bookkeeper] michaeljmarshall commented on a change in pull request #2623: Issue 2622: Fix usage of JournalQueueSize and JournalCbQueueSize; JournalCbQueueSize should be bounded

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

michaeljmarshall commented on a change in pull request #2623:
URL: https://github.com/apache/bookkeeper/pull/2623#discussion_r581647867



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
##########
@@ -390,7 +390,6 @@ public int process(boolean shouldForceWrite) throws IOException {
                     if (qe != null) {
                         cbThreadPool.execute(qe);
                     }
-                    journalStats.getJournalCbQueueSize().inc();

Review comment:
       It's possible that we actually do want to increment the `JournalCbQueueSize` here, if we're looking to measure the number of call backs being run (or waiting to run on the `cbThreadPool`). The running of this `QueueEntry` will trigger the following:
   
   ```java
           public void run() {
               if (LOG.isDebugEnabled()) {
                   LOG.debug("Acknowledge Ledger: {}, Entry: {}", ledgerId, entryId);
               }
               journalCbQueueSize.dec();
               journalAddEntryStats.registerSuccessfulEvent(MathUtils.elapsedNanos(enqueueTime), TimeUnit.NANOSECONDS);
               cb.writeComplete(0, ledgerId, entryId, null, ctx);
               recycle();
           }
   ```
   
   The PR currently assumes that we're measuring the number of call backs in the queue until the call back is executed and decrements the metric.




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