disable 'fsync' by default, but force fsync() in more situations

[justinmk]

fsync() is very slow on my system. It randomly causes :write and :quit to take 3+ seconds.

This PR does several things:

• Make shada and swapfile respect the 'fsync' option.
• Force fsync() (regardless of 'fsync' option) in these cases:
  • Idle (CursorHold).
  • Exit caused by deadly signal.
  • SIGPWR signal.
  • Explicit :preserve command.
• Disable the 'fsync' option by default.

Also mitigates #6725.

ref ludovicchabant/vim-gutentags#167

[jamessan]

CursorHold is based on 'updatetime' but it looks like the decision is being based on 'updatecount'.

[justinmk]

No it's 'updatetime'. before_blocking() calls updatescript(0) which is special-cased.

[justinmk]

After reflection, I think it should also happen for 'updatecount'.

[jamessan]

Just to clarify, this change means that when :w is issued, the file will be flushed after 'updatecount' characters are typed? How does that affects things like https://github.com/tpope/tpope/blob/e8f5d80cf46df8b962cd947a6575bd0621fd6da6/.vimrc#L329-L330 ?

The save would cause 'swapfile' to be unset, but the file hasn't been fsync()d yet.

[justinmk]

when :w is issued, the file will be flushed after 'updatecount' characters are typed?

The file itself is not fsync'd, only its "memfile" is, and only if the buffer is &modified.

It would be nice to fsync the file asynchronously or after idle, but (I would guess) that involves managing file descriptor lifetimes. (And that would be my preference, so then 'fsync' can be enabled by default.)

My assumption is that swapfiles are enough to protect against data loss (within 'updatetime'). Also this suggests that fsync() by itself is not always effective (maybe libuv takes extra measures?).

I left 9139bf8 as a separate commit if we want to revert it. But again, the goal should be to fsync asynchronously. #6725 also needs to be fixed.

references:

• https://thunk.org/tytso/blog/2009/03/15/dont-fear-the-fsync/ (incl. comments)
• https://unix.stackexchange.com/a/384450/

[jamessan]

The file itself is not fsync'd, only its "memfile" is, and only if the buffer is &modified.

neovim/src/nvim/fileio.c

Lines 3378 to 3387 in ad60927

	// On many journalling file systems there is a bug that causes both the
	// original and the backup file to be lost when halting the system right
	// after writing the file. That's because only the meta-data is
	// journalled. Syncing the file slows down the system, but assures it has
	// been written to disk and we don't lose it.
	// For a device do try the fsync() but don't complain if it does not work
	// (could be a pipe).
	// If the 'fsync' option is FALSE, don't fsync(). Useful for laptops.
	int error;
	if (p_fs && (error = os_fsync(fd)) != 0 && !device) {

Now the file being saved is not fsync'd on write and the memfile is only sync'd after a delay.

It would be nice to fsync the file asynchronously or after idle,

Redis does something like that and there was some noise recently about issues PostgreSQL saw related to that.

but (I would guess) that involves managing file descriptor lifetimes.

Or, as tytso suggested in your reference, do the write and fsync async (together).

[justinmk]

The file itself is not fsync'd, only its "memfile" is, and only if the buffer is &modified.

Now the file being saved is not fsync'd on write and the memfile is only sync'd after a delay.

Yes, if 'fsync' is not set. That's what I said, right? :)

Or, as tytso suggested in your reference, do the write and fsync async (together).

Seems higher risk.

• write-now-without-fsync only risks data loss if the OS crashes.
• delayed-write-and-fsync risks data loss if the application crashes.

Different idea: what if we called sync() (to "sync everything") on 'updatetime' (or some other "idle" timer)?

[lewis6991]

I'm quite confused by this change.

@justinmk is fsync still a problem for you?

For context, I think this change is causing me issues with #23152

[justinmk]

@lewis6991 it depends on the filesystem. This PR isn't perfect, but in general we shouldn't need to eagerly fsync(), instead we should lazily fsync() at 'updatetime' (swapfiles at least, but ideally a queue of all writes).

[lewis6991]

The default value of updatetime is 4 seconds though? That seems quite a lot.