Why is bclone_wait_dirty disabled by default?

[mermerico]

Hey, I'm an inexperienced user so it's very likely I'm the problem here, but I wanted to have a discussion about the default setting of blcone_wait_dirty. I had a particular use case where a file was being modified right before a clone so without turning on bclone_wait_dirty the clone would consistently fail. It seems like many applications are moving to using reflinks by default (see here and here) so I'm expecting I won't be the last one. What is the thinking around keeping bclone_wait_dirty off by default?

An additional piece of context is that with bclone_wait_dirty enabled, the total time between writing to a file and having a clone finish is ~5ms. With bclone_wait_dirty off not only does your application now need to implement a retry loop, it can also take several seconds for cloning to become available after a write.

The code has the following to say:

zfs/module/zfs/zfs_vnops.c

Line 70 in 46b82de

* When set zfs_clone_range() waits for dirty data to be written to disk.

When set zfs_clone_range() waits for dirty data to be written to disk.
This allows the clone operation to reliably succeed when a file is modified
and then immediately cloned. For small files this may be slower than making
a copy of the file and is therefore not the default. However, in certain
scenarios this behavior may be desirable so a tunable is provided.

My naive thinking is that it would be more desirable to be slow in these scenarios than to fail. In performance critical use cases the application should could use file size to decide whether to clone or to copy. And as mentioned above, turning bclone_wait_dirty on actually makes things faster by several orders of magnitude.

[mermerico]

Tagging @behlendorf since it seems like he contributed a lot of this code (thank you!!)

[behlendorf]

Thanks for bringing this up, it's a fair point. When this option was originally added the intention was to set a safe default and evaluate the performance in the wild with existing applications. From what I've seen, and you discovered as well, for many common cases we're simply better off immediately kicking off the sync and waiting. I've gone ahead and opened a PR which changes the default and hopefully will spark some discussion.

[mermerico]

That's great to hear, thanks!!