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.

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

Fixes for spurious failures of resilver_restart_001 test #9703

Merged
merged 1 commit into from
Dec 10, 2019

Conversation

jwpoduska
Copy link
Contributor

@jwpoduska jwpoduska commented Dec 9, 2019

Signed-off-by: John Poduska jpoduska@datto.com
Closes #9677

Motivation and Context

This PR fixes spurious failures to a new test introduced by zfsonlinux#9588, as reported in zfsonlinux#9677.

Description

The resilver restart test was reported as failing about 2% of the time.

Issues found:

  • The event log wasn't large enough, so resilver events were missing
  • One 'zpool sync' wasn't enough for resilver to start after zinject
  • Comment capitalization inconsistencies were fixed as well

How Has This Been Tested?

The test has been run by itself over one thousand times without failure.

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:

@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Dec 9, 2019
@behlendorf behlendorf added Status: Code Review Needed Ready for review and testing and removed Status: Accepted Ready to integrate (reviewed, tested) labels Dec 9, 2019
The resilver restart test was reported as failing about 2% of the
time. Two issues were found:
- The event log wasn't large enough, so resilver events were missing
- One 'zpool sync' wasn't enough for resilver to start after zinject

Signed-off-by: John Poduska <jpoduska@datto.com>
Closes openzfs#9677
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Thanks!

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 10, 2019
@codecov
Copy link

codecov bot commented Dec 10, 2019

Codecov Report

Merging #9703 into master will decrease coverage by <1%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #9703    +/-   ##
========================================
- Coverage      79%      79%   -<1%     
========================================
  Files         418      418            
  Lines      123577   123577            
========================================
- Hits        97951    97880    -71     
- Misses      25626    25697    +71
Flag Coverage Δ
#kernel 80% <ø> (ø) ⬆️
#user 67% <ø> (ø) ⬇️

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 3c502d3...327a92b. Read the comment docs.

@behlendorf behlendorf merged commit 7bda69a into openzfs:master Dec 10, 2019
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Jan 22, 2020
The resilver restart test was reported as failing about 2% of the
time. Two issues were found:

- The event log wasn't large enough, so resilver events were missing
- One 'zpool sync' wasn't enough for resilver to start after zinject

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: John Kennedy <john.kennedy@delphix.com>
Reviewed-by: Kjeld Schouten <kjeld@schouten-lebbing.nl>
Signed-off-by: John Poduska <jpoduska@datto.com>
Issue openzfs#9588
Closes openzfs#9677
Closes openzfs#9703
tonyhutter pushed a commit that referenced this pull request Jan 23, 2020
The resilver restart test was reported as failing about 2% of the
time. Two issues were found:

- The event log wasn't large enough, so resilver events were missing
- One 'zpool sync' wasn't enough for resilver to start after zinject

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: John Kennedy <john.kennedy@delphix.com>
Reviewed-by: Kjeld Schouten <kjeld@schouten-lebbing.nl>
Signed-off-by: John Poduska <jpoduska@datto.com>
Issue #9588
Closes #9677
Closes #9703
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

resilver_restart_001 intermittently fails
4 participants