Add Module Parameter Regarding Log Size Limit

[jxdking]

This pull request is based on the original #11876

I add a module parameter regarding total log data size. These can prevent slog device overflow, and long hang on fsync.
zfs_wrlog_data_max

Motivation and Context

It resolves long hang regarding fsync under some specific work load. The issue can be reliably produced with fio.
Here is the script.
fio --name=fio-seq-write --rw=write --bs=1M --direct=1 --numjobs=1 --time_based=1 --runtime=30 --fsync=1000 --filename=fio-seq-write --size=20M --ioengine=libaio --iodepth=16
The command repeatedly overwrites a small file. Since the dirty data space never exceed zfs_dirty_data_sync_percent, write operations are cached in the memory and are never throttled. You will see over 1GB/s write speed on reasonable hardware. If slog device cannot keep up the speed, all the write operations are backed up in ram. That cause long system hang and huge memory usage.
The solution is find a way to "kick" txg sync based on current total size of zil log data (log could be either on memory, or on the slog device), and also throttle write when txg sync doesn't help.

Description

I add a module parameter regarding total log data size.

zfs_wrlog_data_max
The upper limit of TX_WRITE log data. Once it is reached,
write operation is blocked, until log data is cleared out
after txg sync. It only counts TX_WRITE log with WR_COPIED
or WR_NEED_COPY.

How Has This Been Tested?

Tested with fio, please see the example above, and observe the slog device usage.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[jxdking]

I remove the module parameter of the sync threshold of zilog size from the original commit. I don't think it is useful to user.

Thus, right now, I only add one module parameter regarding the total zilog size.

[behlendorf]

Thanks for refactoring this, the rational here makes good sense to me just a few suggested changes.

[behlendorf]

@jxdking unrelated to this PR, but since you've been working to improve the txg sync / ZIL performance I thought you might also be interested in taking a look at issue 11140. Thus far it hasn't been carefully analyzed but initial indications are that for this workload the txg sync / ZIL performance is seriously impacting performance.

[jxdking]

I thought you might also be interested in taking a look at issue 11140. Thus far it hasn't been carefully analyzed but initial indications are that for this workload the txg sync / ZIL performance is seriously impacting performance.

I find zfs is vulnerable to fsync "abuse". When applications call fsync, applications expect most of dirty data is already on stable media, and calling fsync is just to flush the last portion in the cache. In case of zfs, unfortunately, most of dirty data may be still in memory (like the cache). That may cause delay.
Though I am not sure if it applies to issue 11140, when I have time, I will do some test regarding flatpaks.

[jxdking]

@behlendorf , I just pushed a new commit in which I resolved all the suggestions of the code review. Thanks.

[prakashsurya]

I haven't independently tested this, but just looking at the code changes, this looks reasonable to me.

[prakashsurya]

With this default value, what do we expect the performance impact to be? I'm just wondering if there's any cases where a user may not tune this for their workload, resulting in worse performance than prior to this change.

[jxdking]

If a workload hits zfs_dirty_data_max limit first. You will not see any performance difference after this patch.
Can a workload hits zfs_wrlog_data_max limit before hit zfs_dirty_data_max?
Yes. If a workload involves write and then rewrite the same data space within the same txg, the zilog size will be greater than dirty data size. It will result in worse performance after this patch if it hits this limit.
I find most of this kind workloads are results of synthetic benchmarks. benchmark tool often writes and rewrites over a pre-allocated file.
In real world, database workload may be in this category. I have heard of put entire database inside memory (L1ARC). Repeatedly write without triggering zfs_dirty_data_sync_percent is one of the key feature. If that's the case, zfs_wrlog_data_max should be adjust to the size of slog device.
The reason to limit zilog size is described as following.
As mentioned above, if a workload can accumulate a huge size of zilog without hit zfs_dirty_data_sync_percent through repeatedly over-write, that could be tens GB big size zilog, once a fsync occurred, there is no way to write out this amount zilog data in a short period of time. That may cause system hangs.
The proposed new module parameter is more like a safety feature than a performance improvement.
I am open to change the initial value, such like 25% of the total available memory. I find sometimes zilog could overflow the memory if memory could not fit that amount of data.

[prakashsurya]

Yea, makes sense to me. I'm not necessarily opposed to this default value, just wanted to ask.. I'm good as-is.