Repository navigation
Avoid posting duplicate zpool events - #10861
Merged
Merged
Conversation
Signed-off-by: Don Brady <don.brady@delphix.com>
Contributor
Author
|
Address merge conflicts after PR-10857 landed. |
behlendorf
reviewed
Sep 2, 2020
behlendorf
left a comment
Contributor
There was a problem hiding this comment.
Looks nice! This will definitely be useful, it'll be great to catch the duplicates.
Signed-off-by: Don Brady <don.brady@delphix.com>
brad-lewis
reviewed
Sep 2, 2020
brad-lewis
left a comment
Contributor
There was a problem hiding this comment.
This is a really nice addition. I had a couple questions about the scheduling the cleaner.
Signed-off-by: Don Brady <don.brady@delphix.com>
brad-lewis
approved these changes
Sep 3, 2020
behlendorf
approved these changes
Sep 3, 2020
|
The FreeBSD stable/12 failure should be fixed by a rebase. |
behlendorf
pushed a commit
that referenced
this pull request
Sep 9, 2020
Duplicate io and checksum ereport events can misrepresent that things are worse than they seem. Ideally the zpool events and the corresponding vdev stat error counts in a zpool status should be for unique errors -- not the same error being counted over and over. This can be demonstrated in a simple example. With a single bad block in a datafile and just 5 reads of the file we end up with a degraded vdev, even though there is only one unique error in the pool. The proposed solution to the above issue, is to eliminate duplicates when posting events and when updating vdev error stats. We now save recent error events of interest when posting events so that we can easily check for duplicates when posting an error. Reviewed by: Brad Lewis <brad.lewis@delphix.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Don Brady <don.brady@delphix.com> Closes #10861
jsai20
pushed a commit
to jsai20/zfs
that referenced
this pull request
Mar 30, 2021
Duplicate io and checksum ereport events can misrepresent that things are worse than they seem. Ideally the zpool events and the corresponding vdev stat error counts in a zpool status should be for unique errors -- not the same error being counted over and over. This can be demonstrated in a simple example. With a single bad block in a datafile and just 5 reads of the file we end up with a degraded vdev, even though there is only one unique error in the pool. The proposed solution to the above issue, is to eliminate duplicates when posting events and when updating vdev error stats. We now save recent error events of interest when posting events so that we can easily check for duplicates when posting an error. Reviewed by: Brad Lewis <brad.lewis@delphix.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Don Brady <don.brady@delphix.com> Closes openzfs#10861
sempervictus
pushed a commit
to sempervictus/zfs
that referenced
this pull request
May 31, 2021
Duplicate io and checksum ereport events can misrepresent that things are worse than they seem. Ideally the zpool events and the corresponding vdev stat error counts in a zpool status should be for unique errors -- not the same error being counted over and over. This can be demonstrated in a simple example. With a single bad block in a datafile and just 5 reads of the file we end up with a degraded vdev, even though there is only one unique error in the pool. The proposed solution to the above issue, is to eliminate duplicates when posting events and when updating vdev error stats. We now save recent error events of interest when posting events so that we can easily check for duplicates when posting an error. Reviewed by: Brad Lewis <brad.lewis@delphix.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Don Brady <don.brady@delphix.com> Closes openzfs#10861
This file contains hidden or 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
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
Duplicate io and checksum ereport events can misrepresent that things are worse than they seem. Ideally the zpool events and the corresponding vdev stat error counts in a zpool status should be for unique errors -- not the same error being counted over and over. This can be demonstrated in a simple example. With a single bad block in a datafile and just 5 reads of the file we end up with a degraded vdev, even though there is only one unique error in the pool.
Example
Note that the zfs diagnosis agent will diagnose a vdev as degraded if it encounters 10 errors of the same type (io | checksum) within 10 minutes. These errors do not have to be unique! Any runtime condition that is retying a failed block read can easily generate a stream of error events that will trigger a degrade diagnosis.
Ideally, a single bad block should not be enough to degrade a vdev and it should only contribute once to the vdev's stat error count.
Description
The proposed solution to the above issue, is to eliminate duplicates when posting events and when updating vdev error stats. We now save recent error events of interest when posting events so that we can easily check for duplicates when posting an error. The bulk of the implementation changes are in
module/zfs/zfs_fm.cNote: some function prototype changes (like dropping unused args) had ripple effects across a number of files. Don't let this number of files changed scare you away from offering a review!
There are two tunables introduced,
zfs_zevent_retain_maxandzfs_zevent_retain_expire_secs. The duplicate checking mechanism can be disabled by setting thezfs_zevent_retain_maxto zero.Also added a
zio_priorityto the ereport payload since that adds the context of read/write and sync/async which is useful when evaluating duplicate errors.How Has This Been Tested?
functional/cli_root/zpool_eventssuite of tests that cover zpool events.zpool_events_duplicates, that generates duplicate block errors and confirms that duplicate events are not posted.Note that the existing test,
zpool_events_errors, verifies that the event count matches the vdev error stat counts so this test confirms that we have not broken that constraint and adds adequate code coverage.Types of changes
Checklist:
Signed-off-by.