Fsync

[carlhoerberg]

Implementing fsync (and fdatasync) (fflush in windows) in the File class.

The spec goes through even with a simple File#flush, suggestions on how to test it better are welcome.

References:
https://linux.die.net/man/2/fsync
https://linux.die.net/man/2/fdatasync
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/fflush?view=vs-2017
https://ruby-doc.org/core-2.5.1/IO.html#method-i-fsync

[ysbaddaden]

Thanks!

I knew fsync but had to lookup the man page to understand fdatasync. I wonder if we should have both syscalls as distinct methods, or if a single method, for example fsync(flush_metadata = true) wouldn't be more explicit?

[carlhoerberg]

Ruby have them distinct (if that's any indicator), and the "closer to metal" the better i feel like at least. For anyone that knows what fdatasync is and want to do it, and search the docs and nothing come up might wonder why it's not implemented, unless they search the source code of course.

…

On 26 Sep 2018, at 10:40, Julien Portalier ***@***.***> wrote: @ysbaddaden commented on this pull request. Thanks! I wonder I knew fsync but had to lookup the man page to understand fdatasync. if we should have both syscalls as distinct methods, or if a single method, for example fsync(flush_metadata = true) wouldn't be more explicit? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6793 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAK_TlN871A0V55F3NdgjrmcJwzU4G9Tks5uez2VgaJpZM4W3-Ub>.

[RX14]

fdatasync isn't a thing in windows, so fdatasync would just call fsync. I'd be fine with having it as a parameter of fsync.

[carlhoerberg]

That's true, but also "fsync" is not a thing in windows either so I don't know about that argument. Yes, we can call fsync if fdatasync is not supported by the OS (can be more than just windows). I for one very much appreciate closeness to the actual posix syscalls and less arbitrarily named flags. Is it a goal to minimise number of methods?

…

On 26 Sep 2018, at 13:12, Chris Hobbs ***@***.***> wrote: fdatasync isn't a thing in windows, so fdatasync would just call fsync. I'd be fine with having it as a parameter of fsync. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#6793 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAK_TnhNRyMo8biVlC0crrcg2B3rtxB8ks5ue2ECgaJpZM4W3-Ub>.

[ysbaddaden]

I also like being close to POSIX, but Crystal aims to be a little more intuitive and simplify such details —while still having them accessible when possible.

A real thing is that we don't want aliases. Since fdatasync has no equivalent in Windows, fdatasync and fsync would end up being exactly the same. Hence my proposal to have a single fsync method with a named parameter that will be used on POSIX systems, and overlooked on Windows with a documentation to explain that.

[RX14]

anyone searching for fdatasync will quickly fall back to searching for fsync, and then find the parameter documented.

[ysbaddaden]

Note that POSIX AIO has a single aio_fsync function that can be passed O_SYNC or O_DSYNC meaning respectively fsync or fdatasync.

[carlhoerberg]

Updated with docs and a single method with a flush_metadata parameter

[ysbaddaden]

Thank you!

[RX14]

Perhaps flush_metadata should even be false by default...

[carlhoerberg]

Don't you think it would be a bit confusing for a method called fsync to actually do a fdatasync by default?

On another note, it's only when doing small rewrites of an existing file that fdatasync has an advantage over fsync (about 30% faster when doing 1KB rewrites). Doing append only writes or large rewrites (~1MB) makes fsync and fdatasync equally fast.