merlimat commented on a change in pull request #754: Issue #753: Allow option
to disable data sync on journal
URL: https://github.com/apache/bookkeeper/pull/754#discussion_r152632489
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
##########
@@ -914,9 +918,25 @@ public void run() {
forceWriteBatchEntriesStats.registerSuccessfulValue(toFlush.size());
forceWriteBatchBytesStats.registerSuccessfulValue(batchSize);
- forceWriteRequests.put(new
ForceWriteRequest(logFile, logId, lastFlushPosition, toFlush,
- (lastFlushPosition > maxJournalSize),
false));
- toFlush = new LinkedList<QueueEntry>();
+ boolean shouldRolloverJournal = (lastFlushPosition
> maxJournalSize);
+ if (syncData) {
+ // Trigger data sync to disk in the
"Force-Write" thread. Callback will be triggered after data is committed to disk
+ forceWriteRequests.put(new
ForceWriteRequest(logFile, logId, lastFlushPosition, toFlush,
shouldRolloverJournal, false));
+ toFlush = new LinkedList<QueueEntry>();
+ } else {
+ // Data is already written on the file (though
it might still be in the OS page-cache)
+ lastLogMark.setCurLogMark(logId,
lastFlushPosition);
+ for (int i = 0; i < toFlush.size(); i++) {
+ cbThreadPool.execute((QueueEntry)
toFlush.get(i));
+ }
+
+ toFlush.clear();
+ if (shouldRolloverJournal) {
+ forceWriteRequests.put(new
ForceWriteRequest(logFile, logId, lastFlushPosition,
+ new LinkedList<>(),
shouldRolloverJournal, false));
+ }
+ }
+
Review comment:
> Why not simply move the logic into JournalChnnel:forceWrite()?
One reason is simplicity (the logic in the force-sync thread and request is
already complicated, and I didn't want to add more in there).
The other is to avoid one additional context switch before sending the ack.
If we say that the write is "good" (for whatever definition of "good" is
configured in the bookie), then we should acknowledge immediately.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
