Detect IO errors during device removal #8161
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
While device removal cannot verify the checksums of individual
blocks during device removal, it can reasonably detect hard IO
errors from the leaf vdevs. Failure to perform this error
checking can result in device removal completing successfully,
but moving no data which will permanently corrupt the pool.
Situation 1: faulted/degraded vdevs
In the configuration shown below, the removal of mirror-0 will
permanently corrupt the pool. Device removal will preferentially
copy data from 'vdev1 -> vdev3' and from 'vdev2 -> vdev4'. Which
in this case will result in nothing being copied since one vdev
in each of those groups in unavailable. However, device removal
will complete successfully since all IO errors are ignored.
tank DEGRADED 0 0 0
mirror-0 DEGRADED 0 0 0
/var/tmp/vdev1 FAULTED 0 0 0 external fault
/var/tmp/vdev2 ONLINE 0 0 0
mirror-1 DEGRADED 0 0 0
/var/tmp/vdev3 ONLINE 0 0 0
/var/tmp/vdev4 FAULTED 0 0 0 external fault
This issue is resolved by updating the source child selection
logic to exclude unreadable leaf vdevs. Additionally, unwritable
destination child vdevs which can never succeed are skipped to
prevent generating a large number of write IO errors.
Situation 2: individual hard IO errors
During removal if an unexpected hard IO error is encountered when
either reading or writing the child vdev the entire removal
operation is cancelled. While it may be possible to reconstruct
the data after removal that cannot be guaranteed. The only
strictly safe thing to do is to cancel the removal.
As a future improvement we may want to instead suspend the removal
process and allow the damaged region to be retried. But that work
is left for another time, hard IO errors during the removal process
are expected to be exceptionally rare.
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
12 tasks
tonyhutter
reviewed
Nov 29, 2018
tonyhutter
reviewed
Nov 30, 2018
tonyhutter
reviewed
Nov 30, 2018
| WORDS_FILE2="/usr/share/dict/words" | ||
| FILE_CONTENTS="Leeloo Dallas mul-ti-pass." | ||
|
|
||
| if [[ -f $WORDS_FILE1 ]]; then |
There was a problem hiding this comment.
The $WORDS_FILE* cases look to be optional. Can they be removed?
There was a problem hiding this comment.
This was an Illumos thing, I can drop it and do something generic.
Codecov Report
@@ Coverage Diff @@
## master #8161 +/- ##
==========================================
+ Coverage 78.45% 78.62% +0.17%
==========================================
Files 378 378
Lines 114765 114793 +28
==========================================
+ Hits 90035 90260 +225
+ Misses 24730 24533 -197
Continue to review full report at Codecov.
|
sdimitro
reviewed
Nov 30, 2018
| log_onexit cleanup | ||
|
|
||
| # | ||
| # Fault the first side of mirror-0 and the second side of mirror-1. |
There was a problem hiding this comment.
Similar to my comment on that other test file you can explain this in a high-level comment close to the top of the file.
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
sdimitro
approved these changes
Dec 3, 2018
tcaputi
approved these changes
Dec 3, 2018
* Added comment to vdevs_in_pool() helper function. * Moved lock in to conditional in spa_vdev_copy_segment_read_done(). We don't need to take it unconditionally. * Updated comments. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
|
Update: after several days of running |
tonyhutter
approved these changes
Dec 3, 2018
GregorKopka
pushed a commit
to GregorKopka/zfs
that referenced
this pull request
Jan 7, 2019
* Detect IO errors during device removal
While device removal cannot verify the checksums of individual
blocks during device removal, it can reasonably detect hard IO
errors from the leaf vdevs. Failure to perform this error
checking can result in device removal completing successfully,
but moving no data which will permanently corrupt the pool.
Situation 1: faulted/degraded vdevs
In the configuration shown below, the removal of mirror-0 will
permanently corrupt the pool. Device removal will preferentially
copy data from 'vdev1 -> vdev3' and from 'vdev2 -> vdev4'. Which
in this case will result in nothing being copied since one vdev
in each of those groups in unavailable. However, device removal
will complete successfully since all IO errors are ignored.
tank DEGRADED 0 0 0
mirror-0 DEGRADED 0 0 0
/var/tmp/vdev1 FAULTED 0 0 0 external fault
/var/tmp/vdev2 ONLINE 0 0 0
mirror-1 DEGRADED 0 0 0
/var/tmp/vdev3 ONLINE 0 0 0
/var/tmp/vdev4 FAULTED 0 0 0 external fault
This issue is resolved by updating the source child selection
logic to exclude unreadable leaf vdevs. Additionally, unwritable
destination child vdevs which can never succeed are skipped to
prevent generating a large number of write IO errors.
Situation 2: individual hard IO errors
During removal if an unexpected hard IO error is encountered when
either reading or writing the child vdev the entire removal
operation is cancelled. While it may be possible to reconstruct
the data after removal that cannot be guaranteed. The only
strictly safe thing to do is to cancel the removal.
As a future improvement we may want to instead suspend the removal
process and allow the damaged region to be retried. But that work
is left for another time, hard IO errors during the removal process
are expected to be exceptionally rare.
Reviewed-by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Tom Caputi <tcaputi@datto.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue openzfs#6900
Closes openzfs#8161
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
zpool removecommand should never be able to damage a poolduring device removal for situations where it's detectable and avoidable.
This particular issue was uncovered by long runs of
ztestwhich wouldoccasionally produce pools with a handful of non-reconstructable blocks.
@tcaputi identified the root cause as scenario 1 described below.
Description
While device removal cannot verify the checksums of individual
blocks during device removal, it can reasonably detect hard IO
errors from the leaf vdevs. Failure to perform this error
checking can result in device removal completing successfully,
but moving no data which will permanently corrupt the pool.
In the configuration shown below, the removal of mirror-0 will
permanently corrupt the pool. Device removal will preferentially
copy data from 'vdev1 -> vdev3' and from 'vdev2 -> vdev4'. Which
in this case will result in nothing being copied since one vdev
in each of those groups in unavailable. However, device removal
will complete successfully since all IO errors are ignored.
This issue is resolved by updating the source child selection
logic to exclude unreadable leaf vdevs. Additionally, unwritable
destination child vdevs which can never succeed are skipped to
prevent generating a large number of write IO errors.
During removal if an unexpected hard IO error is encountered when
either reading or writing the child vdev the entire removal
operation is cancelled. While it may be possible to reconstruct
the data after removal that cannot be guaranteed. The only
strictly safe thing to do is to cancel the removal.
As a future improvement we may want to instead suspend the removal
process and allow the damaged region to be retried. But that work
is left for another time, hard IO errors during the removal process
are expected to be exceptionally rare.
How Has This Been Tested?
A test case for each scenario described above was added. Prior to
this change the pool would be corrupted, afterwards it is not.
A 14 hour run of
ztestresulted in no failures due to a pool whichcould not be reconstructed. Normally, I'd see one of these failures
after about ~10 hours. I'll do a longer run of
ztestfor additonalverification.
Types of changes
Checklist:
Signed-off-by.