ztest: zdb -Yoption for use by ztest(8)
#8113
Conversation
There was a problem hiding this comment.
This looks fine code-wise. I'm just not sure if this is really a good idea. To add to ztest. I know we've done some work to make the number of possibilities more reasonable, but I think that at some point this will just turn our current ECKSUM bug into a "ztest killed because zdb took too long" bug.
| @@ -5903,6 +5906,10 @@ main(int argc, char **argv) | |||
| case 'X': | |||
| dump_opt[c]++; | |||
| break; | |||
| case 'Y': | |||
| zfs_reconstruct_indirect_combinations_max = INT_MAX; | |||
There was a problem hiding this comment.
technically I think the rest of the code uses UINT64_MAX here
There was a problem hiding this comment.
It gets assigned to a uint64_t which latter gets set to UINT64_MAX but the global is an int. We could change it to an unsigned long and set it to ULONG_MAX. Or update it to take a specific value, which is basically what we do now with -o.
Yes, that's definitely a concern. If we had a hard bound on the worst case number of splits (18?) we could limit it to that to avoid the indefinite hang. |
Codecov Report
@@ Coverage Diff @@
## master #8113 +/- ##
==========================================
+ Coverage 78.45% 78.5% +0.04%
==========================================
Files 378 378
Lines 114765 114769 +4
==========================================
+ Hits 90035 90094 +59
+ Misses 24730 24675 -55
Continue to review full report at Codecov.
|
|
These changes should be reevaluated after #8161 is finalized and merged. They may no longer be needed, and might simply be nice to have optimizations. |
The new -Y flag allows `zdb` to try all possible combinations when performing split block reconstruction. Depending on the extent of the damage this may not be able to complete in a reasonable amount of time. However, it is primarily intended to be used by ztest(8) which by design should never be able to damage a pool beyond repair. The worst case observed has been blocks with 18 splits which can be recovered in a few minutes. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
a25ecd3 to
d056ddd
Compare
|
Closing. This work may be included a more comprehensive change to |
Motivation and Context
Even with all of the optimizations made to speed up split block reconstruction
it's still possible this can lead to
ztestfailures because it gives up onreconstruction to soon. Ideally, we want to completely eliminate these
failures.
For the last three failures I inspected, two were recoverable if
zdbhadbeen allowed to check all the possible combinations. Since
ztestshould never be able to damage a pool beyond repair, and maximum
number of splits has been reduced significantly, it's reasonable to allow
zdbto attempt them all. The-Yflag was added for this purpose.The one failure which was not recoverable may have been caused by
the issue PR #8105 was designed to fix. Additional testing is under
way to determine if there are still failures with both of these changes
applied.
Description
zdb -Yfor split block reconstructionAllows
ztestto request thatzdbattempt all possible combinations.Additional optimization to check zeroed splits last since they are
unlikely to be correct. See comments for details as to why this is
the case.
How Has This Been Tested?
Locally run against 3
ztestfailures which otherwise resulted in azdbfailure.With this change 2/3 pools were verified intact.
Types of changes
Checklist:
Signed-off-by.