diff --git a/db_stress_tool/db_stress_test_base.cc b/db_stress_tool/db_stress_test_base.cc index 1e87051f8d..35b74ce5b2 100644 --- a/db_stress_tool/db_stress_test_base.cc +++ b/db_stress_tool/db_stress_test_base.cc @@ -454,15 +454,29 @@ Status VerifyCheckpointColumnFamilies( } // namespace +bool StressTest::IsTolerableNonInjectedRemoteIOError(const Status& error_s) { + assert(!error_s.ok()); + return FLAGS_tolerate_non_injected_io_errors_for_remote_dbs && + (!FLAGS_env_uri.empty() || !FLAGS_fs_uri.empty()) && + error_s.IsIOError() && + !status_to_io_status(Status(error_s)).GetDataLoss(); +} + bool StressTest::IsErrorInjectedAndRetryable(const Status& error_s) { assert(!error_s.ok()); - const IOStatus io_s = status_to_io_status(Status(error_s)); - return !io_s.GetDataLoss() && - ((error_s.getState() && - FaultInjectionTestFS::IsInjectedError(error_s)) || - (FLAGS_tolerate_non_injected_io_errors_for_remote_dbs && - (!FLAGS_env_uri.empty() || !FLAGS_fs_uri.empty()) && - error_s.IsIOError())); + const bool injected_and_retryable = + error_s.getState() && FaultInjectionTestFS::IsInjectedError(error_s) && + !status_to_io_status(Status(error_s)).GetDataLoss(); + return injected_and_retryable || IsTolerableNonInjectedRemoteIOError(error_s); +} + +bool StressTest::IsRetryableOperationError(int injected_error_count, + const Status& error_s) { + assert(!error_s.ok()); + if (IsTolerableNonInjectedRemoteIOError(error_s)) { + return true; + } + return injected_error_count > 0 && IsErrorInjectedAndRetryable(error_s); } bool StressTest::LazyEntityReadEnabled() const { diff --git a/db_stress_tool/db_stress_test_base.h b/db_stress_tool/db_stress_test_base.h index 58e1c8cfdc..856564b1b7 100644 --- a/db_stress_tool/db_stress_test_base.h +++ b/db_stress_tool/db_stress_test_base.h @@ -43,6 +43,28 @@ class StressTest { // header, so they are not visible here. static bool IsErrorInjectedAndRetryable(const Status& error_s); + // Returns true if `error_s` is a non-injected, non-data-loss IO error that + // --tolerate_non_injected_io_errors_for_remote_dbs asks us to tolerate while + // a remote backend (--env_uri / --fs_uri) is in use. Infrastructure behind a + // remote backend can fail a read transiently with no RocksDB bug involved. + // + // Defined out-of-line in db_stress_test_base.cc for the same reason as + // IsErrorInjectedAndRetryable. + static bool IsTolerableNonInjectedRemoteIOError(const Status& error_s); + + // Returns true if `error_s`, surfaced by an operation during which + // `injected_error_count` faults were injected, is retryable rather than a + // verification failure. + // + // An injected-error status is only trusted when this operation actually + // injected a fault, because otherwise the status did not come from fault + // injection. A tolerated non-injected remote IO error has no injected fault + // to correlate with, so `injected_error_count` must not gate it -- gating it + // would make --tolerate_non_injected_io_errors_for_remote_dbs a no-op + // whenever fault injection is disabled. + static bool IsRetryableOperationError(int injected_error_count, + const Status& error_s); + static bool IsTolerableCompactionFailure(const Status& s) { // TOOD (hx235): allow an exact list of tolerable failures under stress // test diff --git a/db_stress_tool/no_batched_ops_stress.cc b/db_stress_tool/no_batched_ops_stress.cc index a03f79ae3e..0ad6471276 100644 --- a/db_stress_tool/no_batched_ops_stress.cc +++ b/db_stress_tool/no_batched_ops_stress.cc @@ -852,7 +852,7 @@ class NonBatchedOpsStressTest : public StressTest { key.ToString(true).c_str(), rand_keys[0]); } } - } else if (injected_error_count == 0 || !IsErrorInjectedAndRetryable(s)) { + } else if (!IsRetryableOperationError(injected_error_count, s)) { thread->shared->SetVerificationFailure(); fprintf(stderr, "error : Get() returns %s for key: %s (%" PRIi64 ").\n", s.ToString().c_str(), key.ToString(true).c_str(), rand_keys[0]); @@ -1069,8 +1069,10 @@ class NonBatchedOpsStressTest : public StressTest { ThreadStatus::OperationType::OP_MULTIGET); } if (!tmp_s.ok() && !tmp_s.IsNotFound()) { - fprintf(stderr, "Get error: %s\n", s.ToString().c_str()); - is_consistent = false; + if (!IsTolerableNonInjectedRemoteIOError(tmp_s)) { + fprintf(stderr, "Get error: %s\n", tmp_s.ToString().c_str()); + is_consistent = false; + } } else if (!s.ok() && tmp_s.ok()) { fprintf(stderr, "MultiGet(%d) returned different results with key %s. " @@ -1126,7 +1128,7 @@ class NonBatchedOpsStressTest : public StressTest { } else if (s.IsMergeInProgress() && use_txn) { // With txn this is sometimes expected. thread->stats.AddGets(1, 1); - } else if (injected_error_count == 0 || !IsErrorInjectedAndRetryable(s)) { + } else if (!IsRetryableOperationError(injected_error_count, s)) { fprintf(stderr, "MultiGet error: %s\n", s.ToString().c_str()); thread->stats.AddErrors(1); shared->SetVerificationFailure(); @@ -1328,7 +1330,7 @@ class NonBatchedOpsStressTest : public StressTest { StringToHex(key_str).c_str(), rand_keys[0]); } } - } else if (injected_error_count == 0 || !IsErrorInjectedAndRetryable(s)) { + } else if (!IsRetryableOperationError(injected_error_count, s)) { fprintf(stderr, "error : GetEntity() returns %s for key: %s (%" PRIi64 ").\n", s.ToString().c_str(), StringToHex(key_str).c_str(), rand_keys[0]); @@ -1495,9 +1497,11 @@ class NonBatchedOpsStressTest : public StressTest { ran_cmp_get_entity = true; if (!cmp_s.ok() && !cmp_s.IsNotFound()) { - fprintf(stderr, "GetEntity error: %s\n", - cmp_s.ToString().c_str()); - is_consistent = false; + if (!IsTolerableNonInjectedRemoteIOError(cmp_s)) { + fprintf(stderr, "GetEntity error: %s\n", + cmp_s.ToString().c_str()); + is_consistent = false; + } } else if (cmp_s.IsNotFound()) { if (s.ok()) { fprintf( @@ -1616,8 +1620,7 @@ class NonBatchedOpsStressTest : public StressTest { thread->stats.AddGets(1, 1); } else if (s.IsNotFound()) { thread->stats.AddGets(1, 0); - } else if (injected_error_count == 0 || - !IsErrorInjectedAndRetryable(s)) { + } else if (!IsRetryableOperationError(injected_error_count, s)) { fprintf(stderr, "MultiGetEntity error: %s\n", s.ToString().c_str()); thread->stats.AddErrors(1); thread->shared->SetVerificationFailure(); @@ -1933,7 +1936,7 @@ class NonBatchedOpsStressTest : public StressTest { if (s.ok()) { thread->stats.AddPrefixes(1, count); - } else if (injected_error_count == 0 || !IsErrorInjectedAndRetryable(s)) { + } else if (!IsRetryableOperationError(injected_error_count, s)) { fprintf(stderr, "TestPrefixScan error: %s with ReadOptions::iterate_upper_bound: " "%s, prefix_same_as_start: %s \n", diff --git a/tools/db_crashtest.py b/tools/db_crashtest.py index 2bc1a2a2e9..05bd7bf00b 100644 --- a/tools/db_crashtest.py +++ b/tools/db_crashtest.py @@ -274,7 +274,11 @@ default_params = { "flush_one_in": lambda: random.choice([1000, 1000000]), "manual_wal_flush_one_in": lambda: random.choice([0, 1000]), "sync_wal_one_in": 0, - "tolerate_non_injected_io_errors_for_remote_dbs": 0, + # Remote backends can surface transient infrastructure IO errors that are + # not RocksDB bugs, so default to tolerating them whenever a remote + # --env_uri / --fs_uri is in use. A caller can still pass 0 explicitly, and + # finalize_and_sanitize() force-disables it for local DBs. + "tolerate_non_injected_io_errors_for_remote_dbs": lambda: 1 if is_remote_db else 0, "file_checksum_impl": lambda: random.choice(["none", "crc32c", "xxh64", "big"]), "get_live_files_apis_one_in": lambda: random.choice([10000, 1000000]), "checkpoint_atomic_flush": lambda: random.choice([0, 1]), diff --git a/tools/db_crashtest_test.py b/tools/db_crashtest_test.py index 2a5d3d8b49..9d58b1293b 100644 --- a/tools/db_crashtest_test.py +++ b/tools/db_crashtest_test.py @@ -489,6 +489,17 @@ class DBCrashTestTest(unittest.TestCase): self.assertEqual(10, finalized["clear_column_family_one_in"]) self.assertEqual(0, finalized["tolerate_non_injected_io_errors_for_remote_dbs"]) + def test_finalize_defaults_tolerate_non_injected_io_errors_for_remote_db(self): + db_crashtest = self.load_db_crashtest() + db_crashtest.is_remote_db = True + params = self.build_params(db_crashtest.default_params) + + finalized = db_crashtest.finalize_and_sanitize(params) + + # Remote backends can return transient infrastructure IO errors that + # are not RocksDB bugs, so remote runs tolerate them by default. + self.assertEqual(1, finalized["tolerate_non_injected_io_errors_for_remote_dbs"]) + def test_finalize_tolerates_non_injected_io_errors_for_remote_db(self): db_crashtest = self.load_db_crashtest() db_crashtest.is_remote_db = True