Sure, Charan please have a small writeup so we can discus in next Thursday.
On Tue, May 9, 2017 at 10:27 PM, Enrico Olivelli <eolive...@gmail.com>
wrote:
>
>
> Il mer 10 mag 2017, 01:59 Sijie Guo <guosi...@gmail.com> ha scritto:
>
>> On Sat, May 6, 2017 at 9:51 AM, Charan Reddy G <reddychara...@gmail.com>
>> wrote:
>>
>> > 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 don't think the synchronized is necessary here. Atomic structures can
>> help here.
>>
>>
>> >
>> >
>> > 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.
>> >
>>
>> Since you are going to change the checkpointing logic, can you explain
>> more
>> in a separate email thread? I'd like to make sure the changes will not
>> cause any performance degradation.
>>
>
> Maybe a BookKeeper Proposal will be useful?
>
>>
>>
>> >
>> > 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
>> >
>>
> --
>
>
> -- Enrico Olivelli
>
--
Jvrao
---
First they ignore you, then they laugh at you, then they fight you, then
you win. - Mahatma Gandhi
