On May 10, 2017 12:42 PM, "Venkateswara Rao Jujjuri" <jujj...@gmail.com> wrote:
Looked at the source and it is fdatasync() https://android.googlesource.com/platform/libcore/+/ 2496a68/luni/src/main/java/java/nio/FileChannelImpl.java if (metadata) { Libcore.os.fsync(fd); } else { Libcore.os.fdatasync(fd); } So, agreed. fdatasync() fluesh metadata when the file size etc changes, and avoids metadata flush if no data related changes but but that tells me there may be no situation for us to call force(true) in the code. But our code calls it with true in some code path, should we change everything to false for consistency I don't remember that we called force(true) on file channel. Do you mean the buffered channel? JV On Tue, May 9, 2017 at 4:40 PM, Sijie Guo <guosi...@gmail.com> wrote: > We don't force flush metadata in bookies. Because: > > - flush metadata requires more I/O operations. > - flush metadata usually updates file metadata information (e.g. access > permissions, last modification/access time). > > The difference of force(false) and force(true) is the difference between > fdatasync() and fsync(). The former one update last modification time, > while the latter one doesn't. We don't care about the metadata information > like access/modification time, that's the reason we only called > force(false). > > - Sijie > > > On Sat, May 6, 2017 at 9:10 AM, Charan Reddy G <reddychara...@gmail.com> > wrote: > >> By metadata I mean metadata of file >> >> in BufferedChannel.java >> >> public void flush(boolean shouldForceWrite) throws IOException { >> synchronized(this) { >> flushInternal(); >> } >> if (shouldForceWrite) { >> forceWrite(false); <------------- false here >> } >> } >> >> public long forceWrite(boolean forceMetadata) throws IOException { >> // This is the point up to which we had flushed to the file >> system page cache >> // before issuing this force write hence is guaranteed to be made >> durable by >> // the force write, any flush that happens after this may or may >> // not be flushed >> long positionForceWrite = writeBufferStartPosition.get(); >> fileChannel.force(forceMetadata); <--------------- >> forcemetadata false >> return positionForceWrite; >> } >> >> On Fri, May 5, 2017 at 5:56 PM, Charan Reddy G <reddychara...@gmail.com> >> wrote: >> >>> Hey, >>> >>> In GarbageCollectorThread.doCompactEntryLogs { >>> >>> ... >>> >>> try { >>> compactEntryLog(scannerFactory, meta); >>> scannerFactory.flush(); <--------------- this >>> will eventually call entrylogger.flushCurrentLog and it force writes the >>> content of the BufferedChannel but not the metadata of the file >>> >>> LOG.info("Removing entry log {} after compaction", >>> meta.getEntryLogId()); >>> removeEntryLog(meta.getEntryLogId()); <----------- >>> this will delete the compacted entrylog >>> >>> } >>> ... >>> >>> >>> in doCompactEntryLogs, we first write the non-deleted entries of the >>> entryLog which is getting compacted to the currentLog in entryLogger, then >>> we flush the entrylogger before deleting the compacted entrylog. But when >>> currentLog is flushed by the entrylogger, it flushes only the content of >>> the file but not the metadata. After flush is completed the compacted >>> entrylog is deleted. It is not ok to not to force flush the metadata of the >>> currentLog for the persisted (checkpointed) data. The filesystem behaviour >>> is unexpected in this case and there is possibility of data loss if the >>> Bookie crashes before closing that logchannel. >>> >>> Thanks, >>> Charan >>> >> >> > -- Jvrao --- First they ignore you, then they laugh at you, then they fight you, then you win. - Mahatma Gandhi
