Hey,
In EntryLogger, I'm wondering what is the need of making flushCurrentLog
method synchronized? (if we make logChannel variable volatile or of type
AtomicReference and bytesWrittenSinceLastFlush of type AtomicLong). Anyhow
in BufferedChannel.flush, flushInternal() is called from synchrnoized block
of BufferedChannel.
EntryLogger.flushCurrentLog
synchronized void flushCurrentLog() throws IOException {
if (logChannel != null) {
logChannel.flush(true);
bytesWrittenSinceLastFlush = 0;
LOG.debug("Flush and sync current entry logger {}.",
logChannel.getLogId());
}
}
BufferedChannel.flush
public void flush(boolean shouldForceWrite) throws IOException {
synchronized(this) {
flushInternal();
}
if (shouldForceWrite) {
forceWrite(false);
}
}
I'm currently working on "(BOOKKEEPER-1041) Multiple active entrylogs". For
this feature I need to make changes to checkpoint logic. Currently with
BOOKKEEPER-564 change, we are scheduling checkpoint only when current
entrylog file is rotated. So we dont call 'flushCurrentLog' when we
checkpoint. But for BOOKKEEPER-1041 feature, since there are going to be
multiple active entrylogs, scheduling checkpoint when entrylog file is
rotated, is not an option. So I need to call flushCurrentLogs when
checkpoint is made for every 'flushinterval' period. Here it would be
optimal if flushCurrentLog is not in synchronized block, also I don't see a
reason for why it has to be in synchrnoized block to begin with. In
BufferedChannel.flush method 'forceWrite' method (which is not in
synchronized block of BufferedChannel) is gonna take considerable time,
since it flush/forceWrites file to disk. So if EntryLogger's lock is not
held during the forceWrite of BufferedChannel, then progress can be made in
EntryLogger.addEntry.
JV and I had gone through this code mutiple times and we couldn't find a
reason for it to be synchrnoized. Please let us know if we are missing
something here.
Thanks,
Charan
