Re: [PATCH] Add --disable-fsync option to both commit and pull (non-local) commands.

[James Antill <james fedoraproject org>]

On Tue, 2014-06-03 at 13:13 -0700, Colin Walters wrote:
> Thanks a lot for looking at this and for the patch!
>
> There are clear uses for disabling fsync().  I wrote up some of
> that rationale in the commit message and tweaked the whitespace a bit:
> https://git.gnome.org/browse/ostree/commit/?id=f22fa92aef0cf7334d09addacfdfc70c9f10e075

 Cool.

> We should add a configuration file option for this somewhere, right?
> Otherwise people doing "ostree admin upgrade" (or via a higher level
> tool like "rpm-ostree upgrade", or an eventual PackageKit backend)
> won't have a way to set this.  (Or even if we made it a command
> line option, it'd be annoying to re-specify).

 Seems reasonable.

> We have per-remote configs now, perhaps a disable-fsync=true
> option?  Or avoid double negatives and fsync=false?

 I can't think why it would be a per-remote config. ... were you
thinking of any usecase specifically? Avoiding double negatives does
seem good though, I did think about that with the option patch but
disable-fsync already existed for pull-local.

> Below are my timings.  My laptop has a fairly high end
> Samsung 840 Pro 512GB SSD, with the RHEL7 defaults of
> XFS on LVM.  No thin provisioning.
>
> >   ostree default (fsync after every write):
> > ./write-files.py sync = ~7.5 minutes
>
> real    0m45.190s

 Wow. ... that is faster :).

> >   batch fsync calls up into groups of 222
> > ./write-files.py gsync = ~7:30
>
> real    0m38.885s

 That's also interesting that you got a difference here.

> > However while the later is
> > still ~18x slower than no fsync, it's a huge improvement and might be
> > possible to implement if we need the fsyncs to stay around by default.
>
> I think it'd very much be worth investigation into improving the
> fsync path performance.  Hmm.  This is a good thread:
>
> http://oldblog.antirez.com/post/fsync-different-thread-useless.html

 This is about doing an fsync() in another context while you continue to
append to the file ... AFAIK in the path we are on, it's a pure write
once and then fsync(). So we should be good.

> There's also of course:
> http://shaver.off.net/diary/2008/05/25/fsyncers-and-curveballs/

 It's amusing that they think the kernel fsync behaviour will be fixed
within a year or two :)

> Your "async_num" is an interesting approach.  It's implementing
> something similar to what Firefox ended up doing where they
> have an internal queue and only call fsync() after a certain
> amount of change.
>
> Basically we don't need to fsync() immediately after you add a bookmark.
> But we do want to eventually.  After you add a hundred?  After 5
> minutes?

 Right, with the batching and the dir. syncs. I was hoping that the
kernel would get the message and start writing all the data to disk as
soon as we called fsync on anything, alas. no :).
 With the forking bit, I was just more explicit with my prodding of the
kernel (and it seems to get the message then ;). Alas. AFAIK in 2014 we
still can't do aio much better than this (have you used the asyncronous
part of gio?).

> The other option is to implement a custom journal - record which objects
> have been sync'd, and on recovering from a crash, delete and re-fetch
> Or re-checksum as you suggested.

 No need to be that fancy. As soon as you can group a lot of objects
into a single file, like git's pack files, then one fsync for the file
should solve the problem well enough.
 But, yeh, git certainly takes the approach of "don't fsync, as we can
always fsck/pull again later".