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
