zymap commented on code in PR #3843:
URL: https://github.com/apache/bookkeeper/pull/3843#discussion_r1142873585
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java:
##########
@@ -1197,7 +1177,13 @@ journalFormatVersionToWrite, getBufferedChannelBuilder(),
toFlush.add(qe);
numEntriesToFlush++;
- qe = null;
+
+ if (localQueueEntriesIdx < localQueueEntriesLen) {
+ qe = localQueueEntries[localQueueEntriesIdx++];
+ } else {
+ Arrays.fill(localQueueEntries, 0, localQueueEntriesLen,
null);
Review Comment:
Same question with above. At least we can do it in the same loop when you
take the queue from the array.
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java:
##########
@@ -512,16 +507,8 @@ public void run() {
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
LOG.info("ForceWrite thread interrupted");
- // close is idempotent
- if (!localRequests.isEmpty()) {
- ForceWriteRequest req =
localRequests.get(localRequests.size() - 1);
- req.shouldClose = true;
- req.closeFileIfNecessary();
- }
Review Comment:
Won't this cause the journal to discard one flush?
##########
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:
At least we can do it in the line 491 loop to avoid introducing another loop
##########
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?
--
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]
