You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2017/11/22 07:37:30 UTC
[GitHub] eolivelli commented on a change in pull request #754: Issue #753: Allow option to disable data sync on journal
eolivelli 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_r152486564
##########
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:
I think this impl is good as it does not change other logics.
**We MUST add a strong advice that bookies with this flag are very dangerous as they does not guarantee a repeatable behaviour upon restart.**
They will eventually lose "fence bits" and will not guarantee the correctes of the LAC protocol, as writes are not guaranteed to have been really persisted to storage.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services