[GitHub] [bookkeeper] merlimat commented on a diff in pull request #3843: Use BatchedArrayBlockingQueue in Journal

["merlimat (via GitHub)" <gi...@apache.org>]

merlimat commented on code in PR #3843:
URL: https://github.com/apache/bookkeeper/pull/3843#discussion_r1142884988


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java:
##########
@@ -472,35 +472,30 @@ public void run() {
                 }
             }
 
-            final List<ForceWriteRequest> localRequests = new ArrayList<>();
+            final ForceWriteRequest[] localRequests = new ForceWriteRequest[conf.getJournalQueueSize()];
 
             while (running) {
                 try {
-                    int numReqInLastForceWrite = 0;
+                    int numEntriesInLastForceWrite = 0;
 
-                    int requestsCount = forceWriteRequests.drainTo(localRequests);
-                    if (requestsCount == 0) {
-                        ForceWriteRequest fwr = forceWriteRequests.take();
-                        localRequests.add(fwr);
-                        requestsCount = 1;
-                    }
+                    int requestsCount = forceWriteRequests.takeAll(localRequests);
 
                     journalStats.getForceWriteQueueSize().addCount(-requestsCount);
 
                     // Sync and mark the journal up to the position of the last entry in the batch
-                    ForceWriteRequest lastRequest = localRequests.get(requestsCount - 1);
+                    ForceWriteRequest lastRequest = localRequests[requestsCount - 1];
                     syncJournal(lastRequest);
 
                     // All the requests in the batch are now fully-synced. We can trigger sending the
                     // responses
                     for (int i = 0; i < requestsCount; i++) {
-                        ForceWriteRequest req = localRequests.get(i);
-                        numReqInLastForceWrite += req.process();
+                        ForceWriteRequest req = localRequests[i];
+                        numEntriesInLastForceWrite += req.process();
                         req.recycle();
                     }
-
+                    Arrays.fill(localRequests, 0, requestsCount, null);

Review Comment:
   > Why do we need to fill the array with null? It will overwrite by the next, no?
   
   While not strictly necessary, it is mostly to make sure the object can be GCed 



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

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