Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix zpool history unbounded memory usage #9516

Merged
merged 1 commit into from
Oct 28, 2019
Merged

Fix zpool history unbounded memory usage #9516

merged 1 commit into from
Oct 28, 2019

Conversation

tuxoko
Copy link
Contributor

@tuxoko tuxoko commented Oct 26, 2019 •
edited
Loading

Motivation and Context

We have a pool with about 500MB logical size of on disk history.
zpool history command end up using about 1.2GB of memory.

Description

In original implementation, zpool history will read the whole history
before printing anything, causing memory usage goes unbounded. We fix
this by breaking it into read-print iterations.

Signed-off-by: Chunwei Chen david.chen@nutanix.com

How Has This Been Tested?

Run zpool history before and after the change and check memory usage is reduced and the output is identical.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

@davidchenntnx
Fix zpool history unbounded memory usage
d6156a0 
In original implementation, zpool history will read the whole history
before printing anything, causing memory usage goes unbounded. We fix
this by breaking it into read-print iterations.

Signed-off-by: Chunwei Chen <david.chen@nutanix.com>
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Oct 26, 2019
@codecov
Copy link

codecov bot commented Oct 26, 2019 •
edited
Loading

Codecov Report

Merging #9516 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9516      +/-   ##
==========================================
+ Coverage   79.09%   79.16%   +0.06%     
==========================================
  Files         416      416              
  Lines      123640   123647       +7     
==========================================
+ Hits        97789    97879      +90     
+ Misses      25851    25768      -83
Flag Coverage Δ
#kernel 79.74% <ø> (+0.02%) ⬆️
#user 66.7% <100%> (-0.06%) ⬇️

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 5f3eceb...d6156a0. Read the comment docs.

tcaputi
tcaputi approved these changes Oct 28, 2019
View reviewed changes
Copy link
Contributor

@tcaputi tcaputi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Oct 28, 2019
@behlendorf behlendorf merged commit 7125a10 into openzfs:master Oct 28, 2019
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Dec 26, 2019
@davidchenntnx @tonyhutter
Fix zpool history unbounded memory usage
01bddaa 
In original implementation, zpool history will read the whole history
before printing anything, causing memory usage goes unbounded. We fix
this by breaking it into read-print iterations.

Reviewed-by: Tom Caputi <tcaputi@datto.com>
Reviewed-by: Matt Ahrens <matt@delphix.com>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chunwei Chen <david.chen@nutanix.com>
Closes openzfs#9516
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Dec 27, 2019
@davidchenntnx @tonyhutter
Fix zpool history unbounded memory usage
bb230d9 
In original implementation, zpool history will read the whole history
before printing anything, causing memory usage goes unbounded. We fix
this by breaking it into read-print iterations.

Reviewed-by: Tom Caputi <tcaputi@datto.com>
Reviewed-by: Matt Ahrens <matt@delphix.com>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chunwei Chen <david.chen@nutanix.com>
Closes openzfs#9516
tonyhutter pushed a commit that referenced this pull request Jan 23, 2020
@davidchenntnx @tonyhutter
Fix zpool history unbounded memory usage
c6eaa8b 
In original implementation, zpool history will read the whole history
before printing anything, causing memory usage goes unbounded. We fix
this by breaking it into read-print iterations.

Reviewed-by: Tom Caputi <tcaputi@datto.com>
Reviewed-by: Matt Ahrens <matt@delphix.com>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chunwei Chen <david.chen@nutanix.com>
Closes #9516
ahrens pushed a commit to ahrens/zfs that referenced this pull request Mar 20, 2020
@davidchenntnx @ahrens
Fix zpool history unbounded memory usage
2b6c163 
In original implementation, zpool history will read the whole history
before printing anything, causing memory usage goes unbounded. We fix
this by breaking it into read-print iterations.

Reviewed-by: Tom Caputi <tcaputi@datto.com>
Reviewed-by: Matt Ahrens <matt@delphix.com>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chunwei Chen <david.chen@nutanix.com>
Closes openzfs#9516
ahrens pushed a commit to ahrens/zfs that referenced this pull request Mar 23, 2020
@davidchenntnx @ahrens
DLPX-68992 [Backport of DLPX-68991 to 6.0.2.0] Fix zpool history unbo…
acca441 
…unded memory usage

In original implementation, zpool history will read the whole history
before printing anything, causing memory usage goes unbounded. We fix
this by breaking it into read-print iterations.

Reviewed-by: Tom Caputi <tcaputi@datto.com>
Reviewed-by: Matt Ahrens <matt@delphix.com>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chunwei Chen <david.chen@nutanix.com>
Closes openzfs#9516
ahrens pushed a commit to ahrens/zfs that referenced this pull request Apr 6, 2020
@davidchenntnx @ahrens
DLPX-68992 [Backport of DLPX-68991 to 6.0.2.0] Fix zpool history unbo…
810efeb 
…unded memory usage

In original implementation, zpool history will read the whole history
before printing anything, causing memory usage goes unbounded. We fix
this by breaking it into read-print iterations.

Reviewed-by: Tom Caputi <tcaputi@datto.com>
Reviewed-by: Matt Ahrens <matt@delphix.com>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Chunwei Chen <david.chen@nutanix.com>
Closes openzfs#9516
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tcaputi tcaputi tcaputi approved these changes

@ikozhukhov ikozhukhov ikozhukhov approved these changes

@ahrens ahrens ahrens approved these changes

@behlendorf behlendorf behlendorf approved these changes

@tonyhutter tonyhutter Awaiting requested review from tonyhutter

Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tuxoko @behlendorf @ahrens @ikozhukhov @tcaputi @davidchenntnx