OpenZFS 8005 - poor performance of 1MB writes on certain RAID-Z configurations

Authored by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Saso Kiselkov <saso.kiselkov@nexenta.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Don Brady <don.brady@intel.com>
Ported-by: Matt Ahrens <mahrens@delphix.com>
RAID-Z requires that space be allocated in multiples of P+1 sectors,
because this is the minimum size block that can have the required amount
of parity.  Thus blocks on RAIDZ1 must be allocated in a multiple of 2
sectors; on RAIDZ2 multiple of 3; and on RAIDZ3 multiple of 4.  A sector
is a unit of 2^ashift bytes, typically 512B or 4KB.

To satisfy this constraint, the allocation size is rounded up to the
proper multiple, resulting in up to 3 "pad sectors" at the end of some
blocks.  The contents of these pad sectors are not used, so we do not
need to read or write these sectors.  However, some storage hardware
performs much worse (around 1/2 as fast) on mostly-contiguous writes
when there are small gaps of non-overwritten data between the writes.
Therefore, ZFS creates "optional" zio's when writing RAID-Z blocks that
include pad sectors.  If writing a pad sector will fill the gap between
two (required) writes, we will issue the optional zio, thus doubling
performance.  The gap-filling performance improvement was introduced in
July 2009.

Writing the optional zio is done by the io aggregation code in
vdev_queue.c.  The problem is that it is also subject to the limit on
the size of aggregate writes, zfs_vdev_aggregation_limit, which is by
default 128KB.  For a given block, if the amount of data plus padding
written to a leaf device exceeds zfs_vdev_aggregation_limit, the
optional zio will not be written, resulting in a ~2x performance
degradation.

The problem occurs only for certain values of ashift, compressed block
size, and RAID-Z configuration (number of parity and data disks).  It
cannot occur with the default recordsize=128KB.  If compression is
enabled, all configurations with recordsize=1MB or larger will be
impacted to some degree.

The problem notably occurs with recordsize=1MB, compression=off, with 10
disks in a RAIDZ2 or RAIDZ3 group (with 512B or 4KB sectors).  Therefore
this problem has been known as "the 1MB 10-wide RAIDZ2 (or 3) problem".

The problem also occurs with the following configurations:

With recordsize=512KB or 256KB, compression=off, the problem occurs only
in rarely-used configurations:
* 4-wide RAIDZ1 with recordsize=512KB and ashift=12 (4KB sectors)
* 4-wide RAIDZ2 (either recordsize, either ashift)
* 5-wide RAIDZ2 with recordsize=512KB (either ashift)
* 6-wide RAIDZ2 with recordsize=512KB (either ashift)

With recordsize=1MB, compression=off, ashift=9 (512B sectors)
* RAIDZ1 with 4 or 8 disks
* RAIDZ2 with 4, 8, or 10 disks
* RAIDZ3 with 6, 8, 9, or 10 disks

With recordsize=1MB, compression=off, ashift=12 (4KB sectors)
* RAIDZ1 with 7 or 8 disks
* RAIDZ2 with 4, 5, or 10 disks
* RAIDZ3 with 6, 9, or 10 disks

With recordsize=2MB and larger (which can only be selected by changing
kernel tunables), many configurations are affected, including with
higher numbers of disks (up to 18 disks with recordsize=2MB).

Increase zfs_vdev_aggregation_limit to allow the optional zio to be
aggregated, thus eliminating the problem.  Setting it to 256KB fixes all
commonly-used configurations.

The solution is to aggregate optional zio's regardless of the
aggregation size limit.

Analysis sponsored by Intel Corp.

OpenZFS-issue: https://www.illumos.org/issues/8005
OpenZFS-commit: https://github.com/openzfs/openzfs/pull/321
Closes #5931

diff --git a/module/zfs/vdev_queue.c b/module/zfs/vdev_queue.c
index bf2e24cb2134feac7dea7779bfc8c38e3b5eb402..d969cfeed53e574317ec4c29e90a411bdd9e03a4 100644 (file)
--- a/module/zfs/vdev_queue.c
+++ b/module/zfs/vdev_queue.c
@@ -24,7 +24,7 @@
  */
 
 /*
- * Copyright (c) 2012, 2014 by Delphix. All rights reserved.
+ * Copyright (c) 2012, 2017 by Delphix. All rights reserved.
  */
 
 #include <sys/zfs_context.h>
@@ -554,7 +554,7 @@ vdev_queue_aggregate(vdev_queue_t *vq, zio_t *zio)
 
        /*
         * Walk backwards through sufficiently contiguous I/Os
-        * recording the last non-option I/O.
+        * recording the last non-optional I/O.
         */
        while ((dio = AVL_PREV(t, first)) != NULL &&
            (dio->io_flags & ZIO_FLAG_AGG_INHERIT) == flags &&
@@ -576,10 +576,14 @@ vdev_queue_aggregate(vdev_queue_t *vq, zio_t *zio)
 
        /*
         * Walk forward through sufficiently contiguous I/Os.
+        * The aggregation limit does not apply to optional i/os, so that
+        * we can issue contiguous writes even if they are larger than the
+        * aggregation limit.
         */
        while ((dio = AVL_NEXT(t, last)) != NULL &&
            (dio->io_flags & ZIO_FLAG_AGG_INHERIT) == flags &&
-           IO_SPAN(first, dio) <= limit &&
+           (IO_SPAN(first, dio) <= limit ||
+           (dio->io_flags & ZIO_FLAG_OPTIONAL)) &&
            IO_GAP(last, dio) <= maxgap) {
                last = dio;
                if (!(last->io_flags & ZIO_FLAG_OPTIONAL))
@@ -610,10 +614,16 @@ vdev_queue_aggregate(vdev_queue_t *vq, zio_t *zio)
        }
 
        if (stretch) {
-               /* This may be a no-op. */
+               /*
+                * We are going to include an optional io in our aggregated
+                * span, thus closing the write gap.  Only mandatory i/os can
+                * start aggregated spans, so make sure that the next i/o
+                * after our span is mandatory.
+                */
                dio = AVL_NEXT(t, last);
                dio->io_flags &= ~ZIO_FLAG_OPTIONAL;
        } else {
+               /* do not include the optional i/o */
                while (last != mandatory && last != first) {
                        ASSERT(last->io_flags & ZIO_FLAG_OPTIONAL);
                        last = AVL_PREV(t, last);
@@ -625,7 +635,6 @@ vdev_queue_aggregate(vdev_queue_t *vq, zio_t *zio)
                return (NULL);
 
        size = IO_SPAN(first, last);
-       ASSERT3U(size, <=, limit);
 
        abd = abd_alloc_for_io(size, B_TRUE);
        if (abd == NULL)