From b938e6042b7b6c1c639da921b0fc5f8bd670fd55 Mon Sep 17 00:00:00 2001 From: anand76 Date: Wed, 29 Apr 2020 19:25:56 -0700 Subject: [PATCH] Fix a couple of bugs in FaultInjectionTestFS (#6777) Summary: Fix the following cases that can cause false alarms in db_stress when read fault injection is enabled - 1. Turn off corruption/truncation when direct IO is enabled. Since the actual IO size is larger than block size due to alignment requirements, the corruption may not result in a detectable error. 2. Handle the case when the randomly generated string to overwrite the original block is identical to the original. Tests: Run db_stress w/ and wo/ direct IO and fault injection turned on Pull Request resolved: https://github.com/facebook/rocksdb/pull/6777 Reviewed By: zhichao-cao Differential Revision: D21316734 Pulled By: anand1976 fbshipit-source-id: bf0e6468043063ca81ff877d4bf71d3f296c77aa --- test_util/fault_injection_test_fs.cc | 26 ++++++++++++++++++++------ test_util/fault_injection_test_fs.h | 3 ++- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/test_util/fault_injection_test_fs.cc b/test_util/fault_injection_test_fs.cc index 0381e5020..f08ab5ef9 100644 --- a/test_util/fault_injection_test_fs.cc +++ b/test_util/fault_injection_test_fs.cc @@ -213,7 +213,7 @@ IOStatus TestFSRandomAccessFile::Read(uint64_t offset, size_t n, IOStatus s = target_->Read(offset, n, options, result, scratch, dbg); if (s.ok()) { s = fs_->InjectError(FaultInjectionTestFS::ErrorOperation::kRead, result, - scratch); + use_direct_io(), scratch); } return s; } @@ -311,7 +311,7 @@ IOStatus FaultInjectionTestFS::NewRandomAccessFile( if (!IsFilesystemActive()) { return GetError(); } - IOStatus io_s = InjectError(ErrorOperation::kOpen, nullptr, nullptr); + IOStatus io_s = InjectError(ErrorOperation::kOpen, nullptr, false, nullptr); if (io_s.ok()) { io_s = target()->NewRandomAccessFile(fname, file_opts, result, dbg); } @@ -457,6 +457,7 @@ void FaultInjectionTestFS::UntrackFile(const std::string& f) { IOStatus FaultInjectionTestFS::InjectError(ErrorOperation op, Slice* result, + bool direct_io, char* scratch) { ErrorContext* ctx = static_cast(thread_local_error_->Get()); @@ -473,9 +474,17 @@ IOStatus FaultInjectionTestFS::InjectError(ErrorOperation op, switch (op) { case kRead: { - ErrorType type = - static_cast(ctx->rand.Uniform(ErrorType::kErrorTypeMax)); - switch (type) { + if (!direct_io) { + ctx->type = + static_cast(ctx->rand.Uniform(ErrorType::kErrorTypeMax)); + } else { + // In Direct IO mode, the actual read will read extra data due to + // alignment restrictions. So don't inject corruption or + // truncated reads as we don't know if it will actually cause a + // detectable error + ctx->type = ErrorType::kErrorTypeStatus; + } + switch (ctx->type) { // Inject IO error case ErrorType::kErrorTypeStatus: return IOStatus::IOError(); @@ -488,8 +497,13 @@ IOStatus FaultInjectionTestFS::InjectError(ErrorOperation op, std::min(result->size() - offset, 64UL); assert(offset < result->size()); assert(offset + len <= result->size()); - std::string str = DBTestBase::RandomString(&ctx->rand, + std::string str; + // The randomly generated string could be identical to the + // original one, so retry + do { + str = DBTestBase::RandomString(&ctx->rand, static_cast(len)); + } while (str == std::string(scratch + offset, len)); memcpy(scratch + offset, str.data(), len); break; } else { diff --git a/test_util/fault_injection_test_fs.h b/test_util/fault_injection_test_fs.h index 7a7b1fc36..1ac0d6269 100644 --- a/test_util/fault_injection_test_fs.h +++ b/test_util/fault_injection_test_fs.h @@ -309,7 +309,8 @@ class FaultInjectionTestFS : public FileSystemWrapper { // corruption in the contents of scratch, or truncation of slice // are the types of error with equal probability. For OPEN, // its always an IOError. - IOStatus InjectError(ErrorOperation op, Slice* slice, char* scratch); + IOStatus InjectError(ErrorOperation op, Slice* slice, + bool direct_io, char* scratch); // Get the count of how many times we injected since the previous call int GetAndResetErrorCount() {