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

spa_load_verify() may consume too much memory #9146

Merged
merged 1 commit into from
Aug 13, 2019
Merged

spa_load_verify() may consume too much memory #9146

merged 1 commit into from
Aug 13, 2019

Conversation

grwilson
Copy link
Member

@grwilson grwilson commented Aug 9, 2019

When a pool is imported it will scan the pool to verify the integrity of the
data and metadata. The amount it scans will depend on the import flags
provided. On systems with small amounts of memory or when importing a
pool from the crash kernel, it's possible for spa_load_verify to issue
too many I/Os that it consumes all the memory of the system resulting
in an OOM message or a hang.

To prevent this, we limit the amount of memory that the initial pool
scan can consume. This change will, by default, use 1/16th of the ARC
for scan I/Os to prevent running the system out of memory during import.

Signed-off-by: George Wilson george.wilson@delphix.com
External-issue: DLPX-65237
External-issue: DLPX-65238

ahrens
ahrens approved these changes Aug 9, 2019
View reviewed changes
cmd/zdb/zdb.c Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Aug 10, 2019 •
edited
Loading

Codecov Report

Merging #9146 into master will increase coverage by 12.38%.
The diff coverage is 81.81%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #9146       +/-   ##
===========================================
+ Coverage   66.68%   79.06%   +12.38%     
===========================================
  Files         325      400       +75     
  Lines      104496   121695    +17199     
===========================================
+ Hits        69681    96221    +26540     
+ Misses      34815    25474     -9341
Flag Coverage Δ
#kernel 79.68% <100%> (?)
#user 66.65% <81.81%> (-0.03%) ⬇️

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 3b9edd7...b8443bb. Read the comment docs.

sdimitro
sdimitro approved these changes Aug 10, 2019
View reviewed changes
Copy link
Contributor

@sdimitro sdimitro left a comment

Choose a reason for hiding this comment

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

Code LGTM.
I just pointed out a man page change and a nit.

module/zfs/spa.c Show resolved Hide resolved
module/zfs/spa.c Show resolved Hide resolved
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Aug 12, 2019
@grwilson
spa_load_verify() may consume too much memory
b8443bb 
When a pool is imported it will scan the pool to verify the integrity of the
data and metadata. The amount it scans will depend on the import flags
provided. On systems with small amounts of memory or when importing a
pool from the crash kernel, it's possible for spa_load_verify to issue
too many I/Os that it consumes all the memory of the system resulting
in an OOM message or a hang.

To prevent this, we limit the amount of memory that the initial pool
scan can consume. This change will, by default, use 1/16th of the ARC
for scan I/Os to prevent running the system out of memory during import.

Signed-off-by: George Wilson george.wilson@delphix.com
External-issue: DLPX-65237
External-issue: DLPX-65238
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Aug 13, 2019
@behlendorf behlendorf merged commit c8242a9 into openzfs:master Aug 13, 2019
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Dec 24, 2019
@grwilson @tonyhutter
spa_load_verify() may consume too much memory
ce84bf3 
When a pool is imported it will scan the pool to verify the integrity
of the data and metadata. The amount it scans will depend on the
import flags provided. On systems with small amounts of memory or
when importing a pool from the crash kernel, it's possible for
spa_load_verify to issue too many I/Os that it consumes all the memory
of the system resulting in an OOM message or a hang.

To prevent this, we limit the amount of memory that the initial pool
scan can consume. This change will, by default, use 1/16th of the ARC
for scan I/Os to prevent running the system out of memory during import.

Reviewed-by: Matt Ahrens <matt@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Serapheim Dimitropoulos <serapheim@delphix.com>
Signed-off-by: George Wilson george.wilson@delphix.com
External-issue: DLPX-65237
External-issue: DLPX-65238
Closes openzfs#9146
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Dec 27, 2019
@grwilson @tonyhutter
spa_load_verify() may consume too much memory
dde7231 
When a pool is imported it will scan the pool to verify the integrity
of the data and metadata. The amount it scans will depend on the
import flags provided. On systems with small amounts of memory or
when importing a pool from the crash kernel, it's possible for
spa_load_verify to issue too many I/Os that it consumes all the memory
of the system resulting in an OOM message or a hang.

To prevent this, we limit the amount of memory that the initial pool
scan can consume. This change will, by default, use 1/16th of the ARC
for scan I/Os to prevent running the system out of memory during import.

Reviewed-by: Matt Ahrens <matt@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Serapheim Dimitropoulos <serapheim@delphix.com>
Signed-off-by: George Wilson george.wilson@delphix.com
External-issue: DLPX-65237
External-issue: DLPX-65238
Closes openzfs#9146
tonyhutter pushed a commit that referenced this pull request Jan 23, 2020
@grwilson @tonyhutter
spa_load_verify() may consume too much memory
628fd31 
When a pool is imported it will scan the pool to verify the integrity
of the data and metadata. The amount it scans will depend on the
import flags provided. On systems with small amounts of memory or
when importing a pool from the crash kernel, it's possible for
spa_load_verify to issue too many I/Os that it consumes all the memory
of the system resulting in an OOM message or a hang.

To prevent this, we limit the amount of memory that the initial pool
scan can consume. This change will, by default, use 1/16th of the ARC
for scan I/Os to prevent running the system out of memory during import.

Reviewed-by: Matt Ahrens <matt@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Serapheim Dimitropoulos <serapheim@delphix.com>
Signed-off-by: George Wilson george.wilson@delphix.com
External-issue: DLPX-65237
External-issue: DLPX-65238
Closes #9146
allanjude pushed a commit to KlaraSystems/zfs that referenced this pull request Apr 28, 2020
@grwilson
spa_load_verify() may consume too much memory
e98a885 
When a pool is imported it will scan the pool to verify the integrity
of the data and metadata. The amount it scans will depend on the
import flags provided. On systems with small amounts of memory or
when importing a pool from the crash kernel, it's possible for
spa_load_verify to issue too many I/Os that it consumes all the memory
of the system resulting in an OOM message or a hang.

To prevent this, we limit the amount of memory that the initial pool
scan can consume. This change will, by default, use 1/16th of the ARC
for scan I/Os to prevent running the system out of memory during import.

Reviewed-by: Matt Ahrens <matt@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Serapheim Dimitropoulos <serapheim@delphix.com>
Signed-off-by: George Wilson george.wilson@delphix.com
External-issue: DLPX-65237
External-issue: DLPX-65238
Closes openzfs#9146
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sdimitro sdimitro sdimitro approved these changes

@ahrens ahrens ahrens approved these changes

@behlendorf behlendorf behlendorf approved these changes

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.
4 participants
@grwilson @behlendorf @ahrens @sdimitro