leveldb icon indicating copy to clipboard operation
leveldb copied to clipboard

Do not crash if filesystem can't fsync

Open l0rinc opened this issue 7 months ago • 3 comments

Cherry-pick of https://github.com/bitcoin-core/leveldb-subtree/commit/d42e63d49d9df05b12cd00af4ffc5f2b3edf7e21

l0rinc avatar Apr 04 '25 11:04 l0rinc

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Apr 04 '25 11:04 google-cla[bot]

My understanding of the issue was that the reason to make this change was that some file systems fail (CIFS, at the time) instantly when fsync() is called on directories. As this is an inherent property of the file system, retrying wouldn't change anything. Returning the error to the caller is what we want to avoid as the software should work with these file systems, even if this means being able to make fewer guarantees.

That said, this was added as an application-specific work-around for a 2018 (no, even older) bug in CIFS it may not be needed anymore, nor necessary to upstream.

laanwj avatar Apr 29 '25 06:04 laanwj

Returning the error to the caller is what we want to avoid as the software should work with these file systems, even if this means being able to make fewer guarantees.

Understandable. The problem is that this solution makes fewer guarantees on every file system, not just CIFS. fsync for CIFS becoming a no-op in the kernel really is the best solution.

felipecrv avatar Apr 29 '25 20:04 felipecrv