RAID-Z: Increase read IOPS of metadata blocks

[ironMann]

If IO fits in a single block, then corresponding raidz block consists of (nparity+1)
blocks. Additionally, all parity blocks are equal to the data block, effectively
providing mirroring. This is expected and desired behavior, since it preserves
reliability criteria of selected raid-z level, and should enable better IOPS for
small metadata blocks.

This can be exploited in the read path. Such block are usually metadata and
by selecting the least loaded child vdev from (nparity + 1) should yield more
IOPS. The higher the RAID-Z level, the more "mirror" devices there are to choose from.
Total number of child devices does not matter.

This optimization seems similar to "RAIDZ Hybrid Allocator", but does not require
on-disk data change.

Additionally:

• Heuristics from vdev_mirror 9f50093
• mark last offset position of vdev in zio_vdev_child_io(). This should better
  track offset position for all reads/writes, hopefully improving heuristics for
  both mirror and raidz.

[DeHackEd]

The more appropriate style is to put extern variable declarations in a (new, if necessary) header to be included by both the mirror and raidz code. This means the compiler will complain in case anybody does anything funny like change the variable type in only one file.

[ironMann]

Yea, this is definitely not the right place for these. I wanted to keep the changes contained in the first iteration.
These tunables are kinda out of place in the raidz code, but since this is a sort of mirroring situation, I think they could be used here as well. Maybe doc would need updating to reflect this.

[behlendorf]

While I hate the idea of even more tunings it may make the most sense to add a new set of zfs_vdev_raidz_* options. I can imagine a situation where a system has two pools, one mirrored and one raidz. For example, an SSD mirrored root pool and a HDD based raidz primary store may need to be tuned differently.

[adilger]

@behlendorf or with the hybrid pool from @don-brady there may be both SSD VDEVs and HDD VDEVs in the same pool.

[ironMann]

I'll wait for performance data to see what tuning is relevant here. We might be fine with just looking at vdev queue size here. Maybe penalizing rotational vdevs by some factor would be enough.

[behlendorf]

This is awesome, I like how you've generalized the existing mirror logic in to zio_vdev_child_io(). This whole change ended up bring much smaller than I would have guessed and fell out really cleanly.

Do you have any early performance results which show that this is actually a performance win? One potentially good test case would be to do a find in a large directory which is cold. Assuming the fatzap leaves are small enough, they may exceed a single block. Another would be to use xattrtest found in ./tests/zfs-tests/cmd/xattrtest/, when xattr=on you'll generate a ton of small objects if you set one small xattr-per-file.

The Amazon SIMD test failure was unrelated, resubmitting test run.

[adilger]

@ironMann I don't think this will replace the work that @don-brady is doing on hybrid pools, because that will allow both SSD and HDD VDEVs in the same pool, but it is definitely interesting regardless. The balancing of IO load is something that can definitely help in multiple ways. Not only for the single IO request for the mirrored block, but also to reduce the load on other devices that might be a bit slower so they can do something else, instead of piling more work on them.

[ironMann]

@behlendorf This is still very wip. I wanted to check if we can exploit this mirroring without changing the allocator. I was only able to confirm that bonnie++ test case with large dir tree is going through this code. I still have to check if rest of raidz reliability code will work properly in case of cksum error since the parity is accessed first. Also, if there are any consequences to the whole zio pipeline. I'm somewhat surprised how well this tested.
I'll run some tests when I find suitable HDD test hardware. I'll focus on correctness first.

[ironMann]

@adilger This is only intended to exploit one specific case where metadata might be mirrored in raid-z pool. In theory this looks good, but I still have to get real performance measurements in. Hopefully a good deal of metadata heavy workloads would see benefit from this IO balancing.

Pushing the balancing to extremes, with much faster vectorized raid-z operations, it's possible to load balance every read operation by reading parity vdevs instead of data from backlogged vdevs. Missing data reconstruction will add some latency (algorithm can be made incremental to reduce latency), but not as much as waiting for overloaded vdev. Just an idea.

[thegreatgazoo]

I agree it's a good idea to proactively use redundancy for load balancing. But I want to see some data. The reason why raidz[2-3] always store parity on the first 2-3 drives is that parity would still be spread over all drives since blocks can begin at any child drive - see the big block of comment in vdev_raidz_map_alloc(). The comment actually argued that there was not sufficient proof that it was a good idea to alternate parity drives for raidz1.

On the other hand, we might exploit it to optimize the write path, for write zio no larger than 1 << ashift:

1. In vdev_raidz_map_alloc(), don't even bother to allocate buffer/abd for parity.
2. Skip parity generation and write the data buffer/abd instead.

[ironMann]

@thegreatgazoo You're right about allocations. I already started optimizing this in a separate patch. But the worst case of unneeded parity buffers is normal read (without failed devices).

[ahrens]

I agree with what @thegreatgazoo said.

Any improvement in iops is based on there being uneven load on the devices in the RAIDZ group, and vdev_raidz_child_load() being able to notice and counteract that unevenness.

Given a pool with uneven sized blocks, the blocks will tend to be spread over the devices in an uneven manner, and thus under moderate load all devices will be roughly evenly busy. At least, that's my theory. Gathering data that shows otherwise would be enlightening :-)

For the write path, were you able to measure the performance improvement (e.g. CPU usage) of not having to compute the parity? That seems like it could be a good win for single-sector blocks, but those blocks typically constitute a small % of the data written.

[ironMann]

@ahrens
I've only managed to run a read test on a 10 HDD raid-z2 pool. The test consisted of grepping through large, predominantly small file, directory tree; as suggested by @behlendorf in comment. I've seen 10% to 15% wall time improvement, depending on setup. Also, during the test, %util of all drives went to 100% during the whole test (otherwise between 95% and 100%), indicating better load balancing.

I have not seen noticeable difference for write path, since writes take much longer than allocation and memcopy. This might be more relevant for non rotational devices. Luckily, it is a simple enough change for the write path.

Lastly, I went into refactoring of vdev_raidz_io_done() and vdev_raidz_combrec() to ensure that a read failure on 'mirrored' devices is handled properly. Unfortunately, I had to push this down my prio queue for now, hopefully to be continued soon...

[codecov]

Codecov Report

Merging #5095 into master will decrease coverage by 3.28%.
The diff coverage is 92.72%.

@@            Coverage Diff             @@
##           master    #5095      +/-   ##
==========================================
- Coverage   75.26%   71.97%   -3.29%     
==========================================
  Files         296      278      -18     
  Lines       95516    94024    -1492     
==========================================
- Hits        71888    67673    -4215     
- Misses      23628    26351    +2723

Flag	Coverage Δ
#kernel	70.1% <94.05%> (-4.49%)	⬇️
#user	65.77% <84.25%> (-1.69%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cba6fc6...e637627. Read the comment docs.

[behlendorf]

@ironMann if you're still interested in working on this proposed performance optimization please go ahead and open a new PR whenever you're ready. In the meanwhile, I'm going to close this PR.

[thegreatgazoo]

Just a note for those interested that P Q R blocks equal D0 when there's only 1 data block is because the coefficients for D0 in P/Q/R computation are all 1.

[jumbi77]

@thegreatgazoo Are you maybe able and willing to pick up and finish the PR? That would be really great. Sadly i guess @ironMann isnt working on zfs anymore.

But much thanks to both of you anyway, i really appreciate your contributions.