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 2019/08/23 11:11:14 UTC
[GitHub] [bookkeeper] eolivelli commented on a change in pull request #2150:
Improve journal throughput when journalSyncData is disabled.
eolivelli commented on a change in pull request #2150: Improve journal throughput when journalSyncData is disabled.
URL: https://github.com/apache/bookkeeper/pull/2150#discussion_r317046786
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
##########
@@ -1050,25 +1050,11 @@ public void run() {
.registerSuccessfulValue(batchSize);
boolean shouldRolloverJournal = (lastFlushPosition > maxJournalSize);
- if (syncData) {
Review comment:
With this change syncData flag has effect only here at line 1026
https://github.com/apache/bookkeeper/blob/aece5fa866dfbfbae5fb7a53f88322a94bf29246/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java#L1026
So with this change having syncData = false is like having all of the writes as DEFERRED_SYNC on the bookie side
is it expected ?
I think we should cover this change with some test, we already have some kind of advanced tests on journal flush, I added them while working on DEFERRED_SYNC.
----------------------------------------------------------------
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
With regards,
Apache Git Services