summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBrian Behlendorf <[email protected]>2019-01-16 14:10:02 -0800
committerGitHub <[email protected]>2019-01-16 14:10:02 -0800
commit64bdf63f5cceb6ea6b7e199c79a8ac3e37cdbf13 (patch)
tree4a81773f5a691ffc3a2a969efca001b99baf5f18
parentdb587941c5ff6dea01932bb78f70db63cf7f38ba (diff)
ztest: split block reconstruction
Increase the default allowed number of reconstruction attempts. There's not an exact right number for this setting. It needs to be set large enough to cover any realistic failure scenarios and small enough to avoid stalling the IO pipeline and invoking the dead man detection. The current value of 256 was empirically determined to be too low based on multi-day runs of ztest. The fault injection code would inject more damage than could be reconstructed given the relatively small number of attempts. However, in all observed cases the block could be reconstructed using a slightly higher limit. Based on local testing increasing the default value to 4096 was determined to strike the best balance. Checking all combinations takes less than 10s in the worst case, and has so far eliminated the vast majority of false positives detected by ztest. This delay is roughly on par with how long retries may be performed to a misbehaving HDD and was deemed to be reasonable. Better to err on the side of a brief delay rather than fail to reconstruct the data. Lastly, the -Y flag has been added to zdb to make it easy to try all possible combinations when performing split block reconstruction. For badly damaged blocks with 18 splits, they can be fully enumerated within a few minutes. This has been done to ensure permanent errors are never incorrectly reported when ztest verifies the pool with zdb. Reviewed by: Tom Caputi <[email protected]> Reviewed by: Matt Ahrens <[email protected]> Reviewed by: Serapheim Dimitropoulos <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #8271
-rw-r--r--cmd/zdb/zdb.c9
-rw-r--r--cmd/ztest/ztest.c3
-rw-r--r--man/man5/zfs-module-parameters.52
-rw-r--r--man/man8/zdb.88
-rw-r--r--module/zfs/vdev_indirect.c3
5 files changed, 17 insertions, 8 deletions
diff --git a/cmd/zdb/zdb.c b/cmd/zdb/zdb.c
index 1a9303d12..c436139a2 100644
--- a/cmd/zdb/zdb.c
+++ b/cmd/zdb/zdb.c
@@ -100,6 +100,7 @@ extern int zfs_recover;
extern uint64_t zfs_arc_max, zfs_arc_meta_limit;
extern int zfs_vdev_async_read_max_active;
extern boolean_t spa_load_verify_dryrun;
+extern int zfs_reconstruct_indirect_combinations_max;
static const char cmdname[] = "zdb";
uint8_t dump_opt[256];
@@ -215,6 +216,8 @@ usage(void)
"dump all read blocks into specified directory\n");
(void) fprintf(stderr, " -X attempt extreme rewind (does not "
"work with dataset)\n");
+ (void) fprintf(stderr, " -Y attempt all reconstruction "
+ "combinations for split blocks\n");
(void) fprintf(stderr, "Specify an option more than once (e.g. -bb) "
"to make only that option verbose\n");
(void) fprintf(stderr, "Default is to dump everything non-verbosely\n");
@@ -5871,7 +5874,7 @@ main(int argc, char **argv)
spa_config_path = spa_config_path_env;
while ((c = getopt(argc, argv,
- "AbcCdDeEFGhiI:klLmMo:Op:PqRsSt:uU:vVx:X")) != -1) {
+ "AbcCdDeEFGhiI:klLmMo:Op:PqRsSt:uU:vVx:XY")) != -1) {
switch (c) {
case 'b':
case 'c':
@@ -5903,6 +5906,10 @@ main(int argc, char **argv)
case 'X':
dump_opt[c]++;
break;
+ case 'Y':
+ zfs_reconstruct_indirect_combinations_max = INT_MAX;
+ zfs_deadman_enabled = 0;
+ break;
/* NB: Sort single match options below. */
case 'I':
max_inflight = strtoull(optarg, NULL, 0);
diff --git a/cmd/ztest/ztest.c b/cmd/ztest/ztest.c
index 678e29ff6..3686fab76 100644
--- a/cmd/ztest/ztest.c
+++ b/cmd/ztest/ztest.c
@@ -6447,8 +6447,7 @@ ztest_run_zdb(char *pool)
ztest_get_zdb_bin(bin, len);
(void) sprintf(zdb,
- "%s -bcc%s%s -G -d -U %s "
- "-o zfs_reconstruct_indirect_combinations_max=65536 %s",
+ "%s -bcc%s%s -G -d -Y -U %s %s",
bin,
ztest_opts.zo_verbose >= 3 ? "s" : "",
ztest_opts.zo_verbose >= 4 ? "v" : "",
diff --git a/man/man5/zfs-module-parameters.5 b/man/man5/zfs-module-parameters.5
index 1dbf865f4..c9dfceb7e 100644
--- a/man/man5/zfs-module-parameters.5
+++ b/man/man5/zfs-module-parameters.5
@@ -2025,7 +2025,7 @@ combinations each time the block is accessed. This allows all segment
copies to participate fairly in the reconstruction when all combinations
cannot be checked and prevents repeated use of one bad copy.
.sp
-Default value: \fB256\fR.
+Default value: \fB4096\fR.
.RE
.sp
diff --git a/man/man8/zdb.8 b/man/man8/zdb.8
index f00d9c0cf..79d6f8af7 100644
--- a/man/man8/zdb.8
+++ b/man/man8/zdb.8
@@ -23,7 +23,7 @@
.Nd display zpool debugging and consistency information
.Sh SYNOPSIS
.Nm
-.Op Fl AbcdDFGhikLMPsvX
+.Op Fl AbcdDFGhikLMPsvXY
.Op Fl e Oo Fl V Oc Op Fl p Ar path ...
.Op Fl I Ar inflight I/Os
.Oo Fl o Ar var Ns = Ns Ar value Oc Ns ...
@@ -50,7 +50,7 @@
.Ar device
.Nm
.Fl m
-.Op Fl AFLPX
+.Op Fl AFLPXY
.Op Fl e Oo Fl V Oc Op Fl p Ar path ...
.Op Fl t Ar txg
.Op Fl U Ar cache
@@ -349,6 +349,10 @@ Attempt
transaction rewind, that is attempt the same recovery as
.Fl F
but read transactions otherwise deemed too old.
+.It Fl Y
+Attempt all possible combinations when reconstructing indirect split blocks.
+This flag disables the individual I/O deadman timer in order to allow as
+much time as required for the attempted reconstruction.
.El
.Pp
Specifying a display option more than once enables verbosity for only that
diff --git a/module/zfs/vdev_indirect.c b/module/zfs/vdev_indirect.c
index d0725add8..2f8268f0f 100644
--- a/module/zfs/vdev_indirect.c
+++ b/module/zfs/vdev_indirect.c
@@ -213,8 +213,7 @@ int zfs_condense_indirect_commit_entry_delay_ms = 0;
* copies to participate fairly in the reconstruction when all combinations
* cannot be checked and prevents repeated use of one bad copy.
*/
-int zfs_reconstruct_indirect_combinations_max = 256;
-
+int zfs_reconstruct_indirect_combinations_max = 4096;
/*
* Enable to simulate damaged segments and validate reconstruction. This