Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove O_RSYNC, O_SYNC, and/or O_DSYNC? #98

Open
sunfishcode opened this issue Feb 9, 2023 · 3 comments
Open

Remove O_RSYNC, O_SYNC, and/or O_DSYNC? #98

sunfishcode opened this issue Feb 9, 2023 · 3 comments

Comments

@sunfishcode
Copy link
Member

These flags are all about providing data integrity guarantees, however to my knowledge, no one has yet investigated the degree to which these guarantees could be made in a portable manner. This suggests that perhaps we should remove them, until we have an idea of what we're actually able to guarantee.

Here are some notes from my survey of documentation I could find:

Very few OS's claim to support O_RSYNC. Some claim to support O_SYNC and many claim to support O_DSYNC or something that sounds equivalent to it, like FILE_FLAG_WRITE_THROUGH on Windows. So if we're going to support them portably, it's important to have a way to implement them without OS support if needed. That raises some questions:

  • Is doing an fsync after every write sufficient to implement O_SYNC if a host OS doesn't support O_SYNC? In theory it's not identical because there is a window between when a write has returned and other processes are observing the newly written data, but it's not written to persistent storage yet, so an abrupt power failure may undo changes that had temporarily looked like they had been written. Does that mean we can't implement O_SYNC this way?

  • Is doing an fdatasync after every data write sufficient to implement O_DSYNC? In theory it has the same problem as O_SYNC.

  • If doing an fsync (for O_SYNC|O_RSYNC) or fdatasync (for O_DSYNC|O_RSYNC) before every read or data read sufficient to implement O_RSYNC? Does an fsync or fdatasync on one file descriptor flush data written through an independent separately-opened file descriptor?

@sunfishcode
Copy link
Member Author

I talked about related issues with @tschneidereit, who had an idea about how we might approach this.

Instead of trying to define the semantics of fsync, fdatasync, O_SYNC, O_DSYNC, and O_RSYNC, what if we instead just said that the behavior of all of these is dependent on the underlying filesystem implementation? Users that want their data stored reliably would then be responsible for chosing filesystems with reliable fsync behavior.

There still are some dark corners around exactly what an application's responsibilities are around atomic renaming or other specific use cases, however overall it seems like we could develop guidance to applications and guidance for users to help them arrange the desired data integrity properties.

So I'm now slightly leaning toward keeping these flags.

zoraaver added a commit to zoraaver/wasm-micro-runtime that referenced this issue Sep 21, 2023
It's not yet clear whether these flags should be added to the standard
so disallow use of them for the time being. See
WebAssembly/wasi-filesystem#98 for more
details. These changes are also necessary to ensure the rust WASI tests
pass since they assert that path_open returns ENOTSUP when passing
O_DSYNC/O_RSYNC/O_SYNC.
zoraaver added a commit to zoraaver/wasm-micro-runtime that referenced this issue Sep 21, 2023
It's not yet clear whether these flags should be added to the standard
so disallow use of them for the time being. See
WebAssembly/wasi-filesystem#98 for more
details. These changes are also necessary to ensure the rust WASI tests
pass since they assert that path_open returns ENOTSUP when passing
O_DSYNC/O_RSYNC/O_SYNC.
zoraaver added a commit to zoraaver/wasm-micro-runtime that referenced this issue Sep 21, 2023
It's not yet clear whether these flags should be added to the standard
so disallow use of them for the time being. See
WebAssembly/wasi-filesystem#98 for more
details. These changes are also necessary to ensure the rust WASI tests
pass since they assert that path_open returns ENOTSUP when passing
O_DSYNC/O_RSYNC/O_SYNC.
zoraaver added a commit to zoraaver/wasm-micro-runtime that referenced this issue Sep 26, 2023
It's not yet clear whether these flags should be added to the standard
so disallow use of them by default for the time being. See
WebAssembly/wasi-filesystem#98 for more
details. These changes are also necessary to ensure the rust WASI tests
pass since they assert that path_open returns ENOTSUP when passing
O_DSYNC/O_RSYNC/O_SYNC.
zoraaver added a commit to zoraaver/wasm-micro-runtime that referenced this issue Sep 26, 2023
It's not yet clear whether these flags should be added to the standard
so disallow use of them by default for the time being. See
WebAssembly/wasi-filesystem#98 for more
details. These changes are also necessary to ensure the rust WASI tests
pass since they assert that path_open returns ENOTSUP when passing
O_DSYNC/O_RSYNC/O_SYNC.
zoraaver added a commit to zoraaver/wasm-micro-runtime that referenced this issue Sep 26, 2023
It's not yet clear whether these flags should be added to the standard
so disallow use of them by default for the time being. See
WebAssembly/wasi-filesystem#98 for more
details. These changes are also necessary to ensure the rust WASI tests
pass since they assert that path_open returns ENOTSUP when passing
O_DSYNC/O_RSYNC/O_SYNC.
zoraaver added a commit to zoraaver/wasm-micro-runtime that referenced this issue Sep 26, 2023
It's not yet clear whether these flags should be added to the standard
so disallow use of them by default for the time being. See
WebAssembly/wasi-filesystem#98 for more
details. These changes are also necessary to ensure the rust WASI tests
pass since they assert that path_open returns ENOTSUP when passing
O_DSYNC/O_RSYNC/O_SYNC.
@yamt
Copy link

yamt commented Sep 27, 2023

is this topic intended to cover preview1 as well? or only about preview2 and onwards?

zoraaver added a commit to zoraaver/wasm-micro-runtime that referenced this issue Sep 27, 2023
It's not yet clear whether these flags should be added to the standard
so disallow use of them by default for the time being. See
WebAssembly/wasi-filesystem#98 for more
details. These changes are also necessary to ensure the rust WASI tests
pass since they assert that path_open returns ENOTSUP when passing
O_DSYNC/O_RSYNC/O_SYNC.
zoraaver added a commit to zoraaver/wasm-micro-runtime that referenced this issue Sep 27, 2023
It's not yet clear whether these flags should be added to the standard
so disallow use of them by default for the time being. See
WebAssembly/wasi-filesystem#98 for more
details. These changes are also necessary to ensure the rust WASI tests
pass since they assert that path_open returns ENOTSUP when passing
O_DSYNC/O_RSYNC/O_SYNC.
@pchickey
Copy link
Contributor

This discussion is about the wasi-filesystem wit proposal, which is targeting being part of preview 2. Preview 1 is done and we aren't going to add or remove any parts of it at this point, though we can (and should!) still add to the spec text in order to help behavior be consistent or at least well-defined across implementations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants