Bug #8585

improve batching done in zil_commit()

Added by Prakash Surya about 1 month ago. Updated 21 days ago.

Status:ClosedStart date:2017-08-23
Priority:NormalDue date:
Assignee:Prakash Surya% Done:

100%

Category:zfs - Zettabyte File System
Target version:-
Difficulty:Medium Tags:needs-triage

Description

The current implementation of zil_commit() can introduce significant
latency, beyond what is inherent due to the latency of the underlying
storage. The additional latency comes from two main problems:

1. When there's outstanding ZIL blocks being written (i.e. there's
already a "writer thread" in progress), then any new calls to
zil_commit() will block waiting for the currently oustanding ZIL
blocks to complete. The blocks written for each "writer thread" is
coined a "batch", and there can only ever be a single "batch" being
written at a time. When a batch is being written, any new ZIL
transactions will have to wait for the next batch to be written,
which won't occur until the current batch finishes.
As a result, the underlying storage may not be used as efficiently
as possible. While "new" threads enter zil_commit() and are blocked
waiting for the next batch, it's possible that the underlying
storage isn't fully utilized by the current batch of ZIL blocks. In
that case, it'd be better to allow these new threads to generate
(and issue) a new ZIL block, such that it could be serviced by the
underlying storage concurrently with the other ZIL blocks that are
being serviced.
2. Any call to zil_commit() must wait for all ZIL blocks in its "batch" 
to complete, prior to zil_commit() returning. The size of any given
batch is proportional to the number of ZIL transaction in the queue
at the time that the batch starts processing the queue; which
doesn't occur until the previous batch completes. Thus, if there's a
lot of transactions in the queue, the batch could be composed of
many ZIL blocks, and each call to zil_commit() will have to wait for
all of these writes to complete (even if the thread calling
zil_commit() only cared about one of the transactions in the batch).

To further complicate the situation, these two issues result in the
following side effect:

3. If a given batch takes longer to complete than normal, this results
in larger batch sizes, which then take longer to complete and
further drive up the latency of zil_commit(). This can occur for a
number of reasons, including (but not limited to): transient changes
in the workload, and storage latency irregularites.

History

#1 Updated by Prakash Surya 29 days ago

The review contains information about the solution and testing of the
proposed change; I'm copying this information below for posterity's sake.

Solution

The solution attempted by this change has the following goals:

  1. no on-disk changes; maintain current on-disk format.
  2. modify the "batch size" to be equal to the "ZIL block size".
  3. allow new batches to be generated and issued to disk, while there's
    already batches being serviced by the disk.
  4. allow zil_commit() to wait for as few ZIL blocks as possible.
  5. use as few ZIL blocks as possible, for the same amount of ZIL
    transactions, without introducing significant latency to any
    individual ZIL transaction. i.e. use fewer, but larger, ZIL blocks.

In theory, with these goals met, the new allgorithm will allow the
following improvements:

  1. new ZIL blocks can be generated and issued, while there's already
    oustanding ZIL blocks being serviced by the storage.
  2. the latency of zil_commit() should be proportional to the underlying
    storage latency, rather than the incoming synchronous workload.

Performance Testing

I used two fio workloads to drive a synchronous write workload. In both
cases, the performance of this change was pitted against the baseline
"master" branch.

The first workload had each fio thread trying to perform synchronous
writes as fast as it could; issuing a single write at at time. The
results from this test can be seen here: https://goo.gl/jBUiH5

For the "tl;dr", use these two links that jump directly to the section
that describe the percentage difference in fio IOPs and fio write
latency when running on both HDDs and SSDs:

The second workload also had fio issuing synchronous writes, but instead
of attempting to perform as many operations as fast as each thread could,
each fio thread attempted to achieve about 64 write operations per
second. The results of this test can be seen here: https://goo.gl/xEPGm4

Additionally, the following links will jump directly to the percentage
difference information between the baseline and the project branch:

#2 Updated by Electric Monk 21 days ago

  • % Done changed from 0 to 100
  • Status changed from New to Closed

git commit 1271e4b10dfaaed576c08a812f466f6e81370e5e

commit  1271e4b10dfaaed576c08a812f466f6e81370e5e
Author: Prakash Surya <prakash.surya@delphix.com>
Date:   2017-09-01T16:15:28.000Z

    8585 improve batching done in zil_commit()
    Reviewed by: Brad Lewis <brad.lewis@delphix.com>
    Reviewed by: Matt Ahrens <mahrens@delphix.com>
    Reviewed by: George Wilson <george.wilson@delphix.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Also available in: Atom