Improve zfs receive performance with lightweight write #11105
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ahrens
added
Component: Send/Recv
Codecov Report
@@ Coverage Diff @@
## master #11105 +/- ##
==========================================
+ Coverage 79.51% 79.65% +0.13%
==========================================
Files 400 398 -2
Lines 127585 125866 -1719
==========================================
- Hits 101449 100253 -1196
+ Misses 26136 25613 -523
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
adamdmoss
reviewed
Oct 24, 2020
|
@behlendorf It looks like some of the ZTS runs are reporting memory leaks. Do you have any tricks for debugging that? E.g. can I get the stack traces where the leaked memory was allocated? |
|
Yes in fact you can. The coverage builder's kmemleak checker will dump to the console log where the leaked memory was allocated. I believe this is the stack you're looking for (extracted and cleanup from the following log). |
ahrens
force-pushed
the
recv_directio_2
branch
from
44016c6 to
659a7a9
Compare
|
I fixed the leak (which occurs when receiving an incremental large-block stream into a dataset that previously did a non-large-block receive), and addressed code review comments that I received out-of-band. |
behlendorf
reviewed
Nov 16, 2020
ahrens
force-pushed
the
recv_directio_2
branch
from
659a7a9 to
9d216a4
Compare
behlendorf
approved these changes
Nov 17, 2020
|
@grwilson @pcd1193182 if possible would you mind wrapping up your reviews of this PR. Thanks. |
behlendorf
added
Status: Accepted
ahrens
force-pushed
the
recv_directio_2
branch
from
9d216a4 to
a60f0b2
Compare
|
Rebased. @grwilson @pcd1193182 could you take a look at this? |
grwilson
approved these changes
Dec 3, 2020
ahrens
force-pushed
the
recv_directio_2
branch
from
a60f0b2 to
7b734f8
Compare
pcd1193182
approved these changes
Dec 10, 2020
The performance of `zfs receive` can be bottlenecked on the CPU consumed by the `receive_writer` thread, especially when receiving streams with small compressed block sizes. Much of the CPU is spent creating and destroying dbuf's and arc buf's, one for each `WRITE` record in the send stream. This commit introduces the concept of "lightweight writes", which allows `zfs receive` to write to the DMU by providing an ABD, and instantiating only a new type of `dbuf_dirty_record_t`. The dbuf and arc buf for this "dirty leaf block" are not instantiated. Because there is no dbuf with the dirty data, this mechanism doesn't support reading from "lightweight-dirty" blocks (they would see the on-disk state rather than the dirty data). Since the dedup-receive code has been removed, `zfs receive` is write-only, so this works fine. Because there are no arc bufs for the received data, the received data is no longer cached in the ARC. Testing a receive of a stream with average compressed block size of 4KB, this commit improves performance by 50%, while also reducing CPU usage by 50% of a CPU. On a per-block basis, CPU consumed by receive_writer() and dbuf_evict() is now 1/7th (14%) of what it was. Baseline: 450MB/s, CPU in receive_writer() 40% + dbuf_evict() 35% New: 670MB/s, CPU in receive_writer() 17% + dbuf_evict() 0% The code is also restructured in a few ways: Added a `dr_dnode` field to the dbuf_dirty_record_t. This simplifies some existing code that no longer needs `DB_DNODE_ENTER()` and related routines. The new field is needed by the lightweight-type dirty record. To ensure that the `dr_dnode` field remains valid until the dirty record is freed, we have to ensure that the `dnode_move()` doesn't relocate the dnode_t. To do this we keep a hold on the dnode until it's zio's have completed. This is already done by the user-accounting code (`userquota_updates_task()`), this commit extends that so that it always keeps the dnode hold until zio completion (see `dnode_rele_task()`). `dn_dirty_txg` was previously zeroed when the dnode was synced. This was not necessary, since its meaning can be "when was this dnode last dirtied". This change simplifies the new `dnode_rele_task()` code. Removed some dead code related to `DRR_WRITE_BYREF` (dedup receive). Signed-off-by: Matthew Ahrens <[email protected]>
ahrens
force-pushed
the
recv_directio_2
branch
from
7b734f8 to
f6ad5ae
Compare
jsai20
pushed a commit
to jsai20/zfs
that referenced
this pull request
Mar 30, 2021
The performance of `zfs receive` can be bottlenecked on the CPU consumed by the `receive_writer` thread, especially when receiving streams with small compressed block sizes. Much of the CPU is spent creating and destroying dbuf's and arc buf's, one for each `WRITE` record in the send stream. This commit introduces the concept of "lightweight writes", which allows `zfs receive` to write to the DMU by providing an ABD, and instantiating only a new type of `dbuf_dirty_record_t`. The dbuf and arc buf for this "dirty leaf block" are not instantiated. Because there is no dbuf with the dirty data, this mechanism doesn't support reading from "lightweight-dirty" blocks (they would see the on-disk state rather than the dirty data). Since the dedup-receive code has been removed, `zfs receive` is write-only, so this works fine. Because there are no arc bufs for the received data, the received data is no longer cached in the ARC. Testing a receive of a stream with average compressed block size of 4KB, this commit improves performance by 50%, while also reducing CPU usage by 50% of a CPU. On a per-block basis, CPU consumed by receive_writer() and dbuf_evict() is now 1/7th (14%) of what it was. Baseline: 450MB/s, CPU in receive_writer() 40% + dbuf_evict() 35% New: 670MB/s, CPU in receive_writer() 17% + dbuf_evict() 0% The code is also restructured in a few ways: Added a `dr_dnode` field to the dbuf_dirty_record_t. This simplifies some existing code that no longer needs `DB_DNODE_ENTER()` and related routines. The new field is needed by the lightweight-type dirty record. To ensure that the `dr_dnode` field remains valid until the dirty record is freed, we have to ensure that the `dnode_move()` doesn't relocate the dnode_t. To do this we keep a hold on the dnode until it's zio's have completed. This is already done by the user-accounting code (`userquota_updates_task()`), this commit extends that so that it always keeps the dnode hold until zio completion (see `dnode_rele_task()`). `dn_dirty_txg` was previously zeroed when the dnode was synced. This was not necessary, since its meaning can be "when was this dnode last dirtied". This change simplifies the new `dnode_rele_task()` code. Removed some dead code related to `DRR_WRITE_BYREF` (dedup receive). Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Paul Dagnelie <[email protected]> Reviewed-by: George Wilson <[email protected]> Signed-off-by: Matthew Ahrens <[email protected]> Closes openzfs#11105
sempervictus
pushed a commit
to sempervictus/zfs
that referenced
this pull request
May 31, 2021
The performance of `zfs receive` can be bottlenecked on the CPU consumed by the `receive_writer` thread, especially when receiving streams with small compressed block sizes. Much of the CPU is spent creating and destroying dbuf's and arc buf's, one for each `WRITE` record in the send stream. This commit introduces the concept of "lightweight writes", which allows `zfs receive` to write to the DMU by providing an ABD, and instantiating only a new type of `dbuf_dirty_record_t`. The dbuf and arc buf for this "dirty leaf block" are not instantiated. Because there is no dbuf with the dirty data, this mechanism doesn't support reading from "lightweight-dirty" blocks (they would see the on-disk state rather than the dirty data). Since the dedup-receive code has been removed, `zfs receive` is write-only, so this works fine. Because there are no arc bufs for the received data, the received data is no longer cached in the ARC. Testing a receive of a stream with average compressed block size of 4KB, this commit improves performance by 50%, while also reducing CPU usage by 50% of a CPU. On a per-block basis, CPU consumed by receive_writer() and dbuf_evict() is now 1/7th (14%) of what it was. Baseline: 450MB/s, CPU in receive_writer() 40% + dbuf_evict() 35% New: 670MB/s, CPU in receive_writer() 17% + dbuf_evict() 0% The code is also restructured in a few ways: Added a `dr_dnode` field to the dbuf_dirty_record_t. This simplifies some existing code that no longer needs `DB_DNODE_ENTER()` and related routines. The new field is needed by the lightweight-type dirty record. To ensure that the `dr_dnode` field remains valid until the dirty record is freed, we have to ensure that the `dnode_move()` doesn't relocate the dnode_t. To do this we keep a hold on the dnode until it's zio's have completed. This is already done by the user-accounting code (`userquota_updates_task()`), this commit extends that so that it always keeps the dnode hold until zio completion (see `dnode_rele_task()`). `dn_dirty_txg` was previously zeroed when the dnode was synced. This was not necessary, since its meaning can be "when was this dnode last dirtied". This change simplifies the new `dnode_rele_task()` code. Removed some dead code related to `DRR_WRITE_BYREF` (dedup receive). Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Paul Dagnelie <[email protected]> Reviewed-by: George Wilson <[email protected]> Signed-off-by: Matthew Ahrens <[email protected]> Closes openzfs#11105
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
The performance of
zfs receivecan be bottlenecked on the CPU consumedby the
receive_writerthread, especially when receiving streams withsmall compressed block sizes. Much of the CPU is spent creating and
destroying dbuf's and arc buf's, one for each
WRITErecord in the sendstream.
I described this work in my talk at the 2020 OpenZFS Developer Summit: slides (link should start you at slide 25) and video (link should start you at 21:52)
Description
This commit introduces the concept of "lightweight writes", which allows
zfs receiveto write to the DMU by providing an ABD, and instantiatingonly a new type of
dbuf_dirty_record_t. The dbuf and arc buf for this"dirty leaf block" are not instantiated.
Because there is no dbuf with the dirty data, this mechanism doesn't
support reading from "lightweight-dirty" blocks (they would see the
on-disk state rather than the dirty data). Since the dedup-receive code
has been removed,
zfs receiveis write-only, so this works fine.Because there are no arc bufs for the received data, the received data
is no longer cached in the ARC.
The code is also restructured in a few ways:
Added a
dr_dnodefield to the dbuf_dirty_record_t. This simplifiessome existing code that no longer needs
DB_DNODE_ENTER()and relatedroutines. The new field is needed by the lightweight-type dirty record.
To ensure that the
dr_dnodefield remains valid until the dirty recordis freed, we have to ensure that the
dnode_move()doesn't relocate thednode_t. To do this we keep a hold on the dnode until it's zio's have
completed. This is already done by the user-accounting code
(
userquota_updates_task()), this commit extends that so that it alwayskeeps the dnode hold until zio completion (see
dnode_rele_task()).dn_dirty_txgwas previously zeroed when the dnode was synced. Thiswas not necessary, since its meaning can be "when was this dnode last
dirtied". This change simplifies the new
dnode_rele_task()code.Removed some dead code related to
DRR_WRITE_BYREF(dedup receive).How Has This Been Tested?
Testing a receive of a stream with average compressed block size of 4KB,
this commit improves performance by 50%, while also reducing CPU usage
by 50% of a CPU. On a per-block basis, CPU consumed by receive_writer()
and dbuf_evict() is now 1/7th (14%) of what it was.
Baseline: 450MB/s, CPU in receive_writer() 40% + dbuf_evict() 35%
New: 670MB/s, CPU in receive_writer() 17% + dbuf_evict() 0%
Types of changes
Checklist:
Signed-off-by.