Allow empty ds_props_obj to be destroyed #9704
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
Currently, 'zfs list' and 'zfs get' commands can be slow when working with snapshots that have a ds_props_obj. This is because the code that discovers all of the properties for these snapshots needs to read this object for each snapshot, which almost always ends up causing an extra random synchronous read for each snapshot. This performance penalty exists even if the properties on that snapshot have been unset because the object is normally only freed when the snapshot is freed, even though it is only created when it is needed. This patch allows the user to regain 'zfs list' performance on these snapshots by destroying the ds_props_obj when it no longer has any entries left. In practice on a production machine, this optimization seems to make 'zfs list' about 55% faster. Signed-off-by: Tom Caputi <tcaputi@datto.com>
behlendorf
approved these changes
Dec 9, 2019
Codecov Report
@@ Coverage Diff @@
## master #9704 +/- ##
========================================
+ Coverage 79% 79% +<1%
========================================
Files 418 418
Lines 123572 123565 -7
========================================
+ Hits 98121 98136 +15
+ Misses 25451 25429 -22
Continue to review full report at Codecov.
|
|
Looks good. |
behlendorf
added
Status: Accepted
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Dec 26, 2019
Currently, 'zfs list' and 'zfs get' commands can be slow when working with snapshots that have a ds_props_obj. This is because the code that discovers all of the properties for these snapshots needs to read this object for each snapshot, which almost always ends up causing an extra random synchronous read for each snapshot. This performance penalty exists even if the properties on that snapshot have been unset because the object is normally only freed when the snapshot is freed, even though it is only created when it is needed. This patch allows the user to regain 'zfs list' performance on these snapshots by destroying the ds_props_obj when it no longer has any entries left. In practice on a production machine, this optimization seems to make 'zfs list' about 55% faster. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Paul Zuchowski <pzuchowski@datto.com> Signed-off-by: Tom Caputi <tcaputi@datto.com> Closes openzfs#9704
tonyhutter
pushed a commit
to tonyhutter/zfs
that referenced
this pull request
Dec 27, 2019
Currently, 'zfs list' and 'zfs get' commands can be slow when working with snapshots that have a ds_props_obj. This is because the code that discovers all of the properties for these snapshots needs to read this object for each snapshot, which almost always ends up causing an extra random synchronous read for each snapshot. This performance penalty exists even if the properties on that snapshot have been unset because the object is normally only freed when the snapshot is freed, even though it is only created when it is needed. This patch allows the user to regain 'zfs list' performance on these snapshots by destroying the ds_props_obj when it no longer has any entries left. In practice on a production machine, this optimization seems to make 'zfs list' about 55% faster. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Paul Zuchowski <pzuchowski@datto.com> Signed-off-by: Tom Caputi <tcaputi@datto.com> Closes openzfs#9704
tonyhutter
pushed a commit
that referenced
this pull request
Jan 23, 2020
Currently, 'zfs list' and 'zfs get' commands can be slow when working with snapshots that have a ds_props_obj. This is because the code that discovers all of the properties for these snapshots needs to read this object for each snapshot, which almost always ends up causing an extra random synchronous read for each snapshot. This performance penalty exists even if the properties on that snapshot have been unset because the object is normally only freed when the snapshot is freed, even though it is only created when it is needed. This patch allows the user to regain 'zfs list' performance on these snapshots by destroying the ds_props_obj when it no longer has any entries left. In practice on a production machine, this optimization seems to make 'zfs list' about 55% faster. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Paul Zuchowski <pzuchowski@datto.com> Signed-off-by: Tom Caputi <tcaputi@datto.com> Closes #9704
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.
Currently, 'zfs list' and 'zfs get' commands can be slow when
working with snapshots that have a ds_props_obj. This is
because the code that discovers all of the properties for these
snapshots needs to read this object for each snapshot, which
almost always ends up causing an extra random synchronous read
for each snapshot. This performance penalty exists even if the
properties on that snapshot have been unset because the object
is normally only freed when the snapshot is freed, even though
it is only created when it is needed.
This patch allows the user to regain 'zfs list' performance on
these snapshots by destroying the ds_props_obj when it no longer
has any entries left. In practice on a production machine, this
optimization seems to make 'zfs list' about 55% faster.
Signed-off-by: Tom Caputi tcaputi@datto.com
Motivation and Context
The following off-cpu flame-graph shows where the
zfs listcommand is spending most of its IO time when runningzfs list -rt snapshot <leaf dataset>. This process spends >97% of its time in IO wait. Each snapshot has a single user property set. The presence of this user property causes the highlighted stack trace to be hit on each iteration. Eliminating the user property with this patch prevents zfs from needing to make this extraneous read, saving ~55% of the time cost of running the command.Types of changes
Checklist:
Signed-off-by.