util: add fault injection of EIOs This patch adds the functionality to inject EIOs in env_posix based on flag-specified glob patterns indicating which paths should fail and a flag-specified double indicating how often these failures should occur.
A similar flag inject_io_error is replaced by the latter flag, and wrapper macros are updated to ensure the returned error expression is only executed when necessary. A test is added to env-test.cc to verify that EIOs are successfully triggered by specifying the patterns. Change-Id: I5190d5e7f296cf27b65f1ecd2c16f3315cc46a39 Reviewed-on: http://gerrit.cloudera.org:8080/6881 Reviewed-by: Adar Dembo <[email protected]> Tested-by: Kudu Jenkins Reviewed-by: David Ribeiro Alves <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/10aeb287 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/10aeb287 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/10aeb287 Branch: refs/heads/master Commit: 10aeb28776214f0e36f1d647ed466b6f95b142ee Parents: 851e847 Author: Andrew Wong <[email protected]> Authored: Thu May 18 13:31:34 2017 -0700 Committer: David Ribeiro Alves <[email protected]> Committed: Wed Jun 7 14:17:39 2017 +0000 ---------------------------------------------------------------------- src/kudu/fs/block_manager-test.cc | 25 ++++--- src/kudu/rpc/server_negotiation.cc | 6 +- src/kudu/util/env-test.cc | 74 +++++++++++++++++++++ src/kudu/util/env_posix.cc | 111 +++++++++++++++++++++++--------- src/kudu/util/fault_injection.cc | 8 +-- src/kudu/util/fault_injection.h | 26 ++++---- 6 files changed, 186 insertions(+), 64 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/10aeb287/src/kudu/fs/block_manager-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/fs/block_manager-test.cc b/src/kudu/fs/block_manager-test.cc index 0040cdf..a3331f7 100644 --- a/src/kudu/fs/block_manager-test.cc +++ b/src/kudu/fs/block_manager-test.cc @@ -56,7 +56,8 @@ DECLARE_int64(disk_reserved_bytes_free_for_testing); DECLARE_int32(fs_data_dirs_full_disk_cache_seconds); DECLARE_uint32(fs_target_data_dirs_per_tablet); DECLARE_string(block_manager); -DECLARE_double(env_inject_io_error); +DECLARE_double(env_inject_eio); +DECLARE_bool(suicide_on_eio); // Generic block manager metrics. METRIC_DECLARE_gauge_uint64(block_manager_blocks_open_reading); @@ -851,7 +852,8 @@ TYPED_TEST(BlockManagerTest, TestMetadataOkayDespiteFailedWrites) { FLAGS_log_container_preallocate_bytes = 8 * 1024; // Force some file operations to fail. - FLAGS_env_inject_io_error = 0.1; + FLAGS_suicide_on_eio = false; + FLAGS_env_inject_eio = 0.1; // Compact log block manager metadata aggressively at startup; injected // errors may also crop up here. @@ -926,14 +928,19 @@ TYPED_TEST(BlockManagerTest, TestMetadataOkayDespiteFailedWrites) { LOG(INFO) << Substitute("Successfully deleted $0 blocks on $1 attempts", num_deleted, num_deleted_attempts); - for (const auto& id : ids) { - ASSERT_OK(read_a_block(id)); + { + // Since this test is for failed writes, don't inject faults during reads + // or while reopening the block manager. + google::FlagSaver saver; + FLAGS_env_inject_eio = false; + for (const auto& id : ids) { + ASSERT_OK(read_a_block(id)); + } + ASSERT_OK(this->ReopenBlockManager(scoped_refptr<MetricEntity>(), + shared_ptr<MemTracker>(), + { GetTestDataDirectory() }, + false /* create */)); } - - ASSERT_OK(this->ReopenBlockManager(scoped_refptr<MetricEntity>(), - shared_ptr<MemTracker>(), - { GetTestDataDirectory() }, - false /* create */)); } } http://git-wip-us.apache.org/repos/asf/kudu/blob/10aeb287/src/kudu/rpc/server_negotiation.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/server_negotiation.cc b/src/kudu/rpc/server_negotiation.cc index 5e6d070..a9fd4db 100644 --- a/src/kudu/rpc/server_negotiation.cc +++ b/src/kudu/rpc/server_negotiation.cc @@ -701,10 +701,8 @@ Status ServerNegotiation::AuthenticateByToken(faststring* recv_buf) { res = security::VerificationResult::EXPIRED_SIGNING_KEY; break; } - const Status s = kudu::fault_injection::MaybeReturnFailure( - FLAGS_rpc_inject_invalid_authn_token_ratio, - Status::NotAuthorized(VerificationResultToString(res))); - if (!s.ok()) { + if (kudu::fault_injection::MaybeTrue(FLAGS_rpc_inject_invalid_authn_token_ratio)) { + Status s = Status::NotAuthorized(VerificationResultToString(res)); RETURN_NOT_OK(SendError(ErrorStatusPB::FATAL_INVALID_AUTHENTICATION_TOKEN, s)); return s; } http://git-wip-us.apache.org/repos/asf/kudu/blob/10aeb287/src/kudu/util/env-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/env-test.cc b/src/kudu/util/env-test.cc index fb79a69..bb8095c 100644 --- a/src/kudu/util/env-test.cc +++ b/src/kudu/util/env-test.cc @@ -50,8 +50,11 @@ #endif DECLARE_bool(never_fsync); +DECLARE_bool(suicide_on_eio); +DECLARE_double(env_inject_eio); DECLARE_int32(env_inject_short_read_bytes); DECLARE_int32(env_inject_short_write_bytes); +DECLARE_string(env_inject_eio_globs); namespace kudu { @@ -978,4 +981,75 @@ TEST_F(TestEnv, TestGetExtentMap) { "Punching a hole should have increased the number of extents by one"; } +TEST_F(TestEnv, TestInjectEIO) { + // Use two files to fail with. + FLAGS_suicide_on_eio = false; + const string kTestRWPath1 = GetTestPath("test_env_rw_file1"); + unique_ptr<RWFile> rw1; + ASSERT_OK(env_->NewRWFile(kTestRWPath1, &rw1)); + + const string kTestRWPath2 = GetTestPath("test_env_rw_file2"); + unique_ptr<RWFile> rw2; + ASSERT_OK(env_->NewRWFile(kTestRWPath2, &rw2)); + + // Inject EIOs to all operations that might result in an EIO, without + // specifying a glob pattern (not specifying the glob pattern will inject + // EIOs wherever possible by default). + FLAGS_env_inject_eio = 1.0; + uint64_t size; + Status s = rw1->Size(&size); + ASSERT_TRUE(s.IsIOError()); + ASSERT_STR_CONTAINS(s.ToString(), "INJECTED FAILURE"); + s = rw2->Size(&size); + ASSERT_TRUE(s.IsIOError()); + ASSERT_STR_CONTAINS(s.ToString(), "INJECTED FAILURE"); + + // Specify and verify that both files should fail by matching glob patterns + // to of each's literal paths. + FLAGS_env_inject_eio_globs = Substitute("$0,$1", kTestRWPath1, kTestRWPath2); + Slice result; + s = rw1->Read(0, &result); + ASSERT_TRUE(s.IsIOError()); + ASSERT_STR_CONTAINS(s.ToString(), "INJECTED FAILURE"); + s = rw2->Size(&size); + ASSERT_TRUE(s.IsIOError()); + ASSERT_STR_CONTAINS(s.ToString(), "INJECTED FAILURE"); + + // Inject EIOs to all operations that might result in an EIO across paths, + // specified with a glob pattern. + FLAGS_env_inject_eio_globs = "*"; + Slice data("data"); + s = rw1->Write(0, data); + ASSERT_TRUE(s.IsIOError()); + ASSERT_STR_CONTAINS(s.ToString(), "INJECTED FAILURE"); + s = rw2->Size(&size); + ASSERT_TRUE(s.IsIOError()); + ASSERT_STR_CONTAINS(s.ToString(), "INJECTED FAILURE"); + + // Specify and verify that one of the files should fail by matching a glob + // pattern of one of the literal paths. + FLAGS_env_inject_eio_globs = kTestRWPath1; + s = rw1->Size(&size); + ASSERT_TRUE(s.IsIOError()); + ASSERT_STR_CONTAINS(s.ToString(), "INJECTED FAILURE"); + ASSERT_OK(rw2->Write(0, data)); + + // Specify the directory of one of the files and ensure that fails. + FLAGS_env_inject_eio_globs = JoinPathSegments(DirName(kTestRWPath2), "**"); + s = rw2->Sync(); + ASSERT_TRUE(s.IsIOError()); + ASSERT_STR_CONTAINS(s.ToString(), "INJECTED FAILURE"); + + // Specify a directory and check that failed directory operations are caught. + FLAGS_env_inject_eio_globs = DirName(kTestRWPath2); + s = env_->SyncDir(DirName(kTestRWPath2)); + ASSERT_TRUE(s.IsIOError()); + ASSERT_STR_CONTAINS(s.ToString(), "INJECTED FAILURE"); + + // Specify that neither file fails. + FLAGS_env_inject_eio_globs = "neither_path"; + ASSERT_OK(rw1->Close()); + ASSERT_OK(rw2->Close()); +} + } // namespace kudu http://git-wip-us.apache.org/repos/asf/kudu/blob/10aeb287/src/kudu/util/env_posix.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/env_posix.cc b/src/kudu/util/env_posix.cc index a3998fc..9146454 100644 --- a/src/kudu/util/env_posix.cc +++ b/src/kudu/util/env_posix.cc @@ -5,6 +5,7 @@ #include <dirent.h> #include <errno.h> #include <fcntl.h> +#include <fnmatch.h> #include <fts.h> #include <glob.h> #include <limits.h> @@ -35,6 +36,7 @@ #include "kudu/gutil/bind.h" #include "kudu/gutil/callback.h" #include "kudu/gutil/map-util.h" +#include "kudu/gutil/strings/split.h" #include "kudu/gutil/strings/substitute.h" #include "kudu/util/atomic.h" #include "kudu/util/debug/trace_event.h" @@ -99,6 +101,24 @@ (nread) = (expr); \ } while ((nread) == 0 && ferror(stream) == EINTR) +// With some probability, if 'filename_expr' matches the glob pattern specified +// by the 'env_inject_eio_globs' flag, calls RETURN_NOT_OK on 'error_expr'. +#define MAYBE_RETURN_EIO(filename_expr, error_expr) do { \ + const string& f_ = (filename_expr); \ + MAYBE_RETURN_FAILURE(FLAGS_env_inject_eio, \ + StringMatchesGlob(f_, FLAGS_env_inject_eio_globs) ? (error_expr) : Status::OK()) \ +} while (0); + +bool StringMatchesGlob(const string& candidate, const string& glob_patterns) { + vector<string> globs = strings::Split(glob_patterns, ",", strings::SkipEmpty()); + for (const auto& glob : globs) { + if (fnmatch(glob.c_str(), candidate.c_str(), 0) == 0) { + return true; + } + } + return false; +} + // See KUDU-588 for details. DEFINE_bool(env_use_fsync, false, "Use fsync(2) instead of fdatasync(2) for synchronizing dirty " @@ -107,8 +127,11 @@ TAG_FLAG(env_use_fsync, advanced); TAG_FLAG(env_use_fsync, evolving); DEFINE_bool(suicide_on_eio, true, - "Kill the process if an I/O operation results in EIO"); + "Kill the process if an I/O operation results in EIO. If false, " + "I/O resulting in EIOs will return the status IOError and leave " + "error-handling up to the caller."); TAG_FLAG(suicide_on_eio, advanced); +TAG_FLAG(suicide_on_eio, experimental); DEFINE_bool(never_fsync, false, "Never fsync() anything to disk. This is used by certain test cases to " @@ -116,10 +139,6 @@ DEFINE_bool(never_fsync, false, TAG_FLAG(never_fsync, advanced); TAG_FLAG(never_fsync, unsafe); -DEFINE_double(env_inject_io_error, 0.0, - "Fraction of the time that certain I/O operations will fail"); -TAG_FLAG(env_inject_io_error, hidden); - DEFINE_int32(env_inject_short_read_bytes, 0, "The number of bytes less than the requested bytes to read"); TAG_FLAG(env_inject_short_read_bytes, hidden); @@ -127,6 +146,15 @@ DEFINE_int32(env_inject_short_write_bytes, 0, "The number of bytes less than the requested bytes to write"); TAG_FLAG(env_inject_short_write_bytes, hidden); +DEFINE_double(env_inject_eio, 0.0, + "Fraction of the time that operations on certain files will fail " + "with the posix code EIO."); +TAG_FLAG(env_inject_eio, hidden); +DEFINE_string(env_inject_eio_globs, "*", + "Comma-separated list of glob patterns specifying files on which " + "I/O will fail. By default, all files may cause a failure."); +TAG_FLAG(env_inject_eio_globs, hidden); + using base::subtle::Atomic64; using base::subtle::Barrier_AtomicIncrement; using std::accumulate; @@ -247,17 +275,19 @@ Status IOError(const std::string& context, int err_number) { return Status::NotSupported(context, ErrnoToString(err_number), err_number); case EIO: if (FLAGS_suicide_on_eio) { - // TODO: This is very, very coarse-grained. A more comprehensive + // TODO(awong): This is very, very coarse-grained. A more comprehensive // approach is described in KUDU-616. LOG(FATAL) << "Fatal I/O error, context: " << context; + } else { + LOG(ERROR) << "I/O error, context: " << context; } } return Status::IOError(context, ErrnoToString(err_number), err_number); } Status DoSync(int fd, const string& filename) { - MAYBE_RETURN_FAILURE(FLAGS_env_inject_io_error, - Status::IOError(Env::kInjectedFailureStatusMsg)); + MAYBE_RETURN_EIO(filename, IOError(Env::kInjectedFailureStatusMsg, EIO)); + ThreadRestrictions::AssertIOAllowed(); if (FLAGS_never_fsync) return Status::OK(); if (FLAGS_env_use_fsync) { @@ -277,6 +307,7 @@ Status DoSync(int fd, const string& filename) { } Status DoOpen(const string& filename, Env::CreateMode mode, int* fd) { + MAYBE_RETURN_EIO(filename, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); int flags = O_RDWR; switch (mode) { @@ -300,6 +331,7 @@ Status DoOpen(const string& filename, Env::CreateMode mode, int* fd) { } Status DoReadV(int fd, const string& filename, uint64_t offset, vector<Slice>* results) { + MAYBE_RETURN_EIO(filename, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); // Convert the results into the iovec vector to request @@ -366,8 +398,7 @@ Status DoReadV(int fd, const string& filename, uint64_t offset, vector<Slice>* r Status DoWriteV(int fd, const string& filename, uint64_t offset, const vector<Slice>& data) { - MAYBE_RETURN_FAILURE(FLAGS_env_inject_io_error, - Status::IOError(Env::kInjectedFailureStatusMsg)); + MAYBE_RETURN_EIO(filename, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); // Convert the results into the iovec vector to request @@ -440,6 +471,7 @@ class PosixSequentialFile: public SequentialFile { virtual ~PosixSequentialFile() { fclose(file_); } virtual Status Read(Slice* result) OVERRIDE { + MAYBE_RETURN_EIO(filename_, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); size_t r; STREAM_RETRY_ON_EINTR(r, file_, fread_unlocked(result->mutable_data(), 1, @@ -458,6 +490,7 @@ class PosixSequentialFile: public SequentialFile { } virtual Status Skip(uint64_t n) OVERRIDE { + MAYBE_RETURN_EIO(filename_, IOError(Env::kInjectedFailureStatusMsg, EIO)); TRACE_EVENT1("io", "PosixSequentialFile::Skip", "path", filename_); ThreadRestrictions::AssertIOAllowed(); if (fseek(file_, n, SEEK_CUR)) { @@ -490,6 +523,7 @@ class PosixRandomAccessFile: public RandomAccessFile { } virtual Status Size(uint64_t *size) const OVERRIDE { + MAYBE_RETURN_EIO(filename_, IOError(Env::kInjectedFailureStatusMsg, EIO)); TRACE_EVENT1("io", "PosixRandomAccessFile::Size", "path", filename_); ThreadRestrictions::AssertIOAllowed(); struct stat st; @@ -546,8 +580,8 @@ class PosixWritableFile : public WritableFile { } virtual Status PreAllocate(uint64_t size) OVERRIDE { - MAYBE_RETURN_FAILURE(FLAGS_env_inject_io_error, - Status::IOError(Env::kInjectedFailureStatusMsg)); + MAYBE_RETURN_EIO(filename_, IOError(Env::kInjectedFailureStatusMsg, EIO)); + TRACE_EVENT1("io", "PosixWritableFile::PreAllocate", "path", filename_); ThreadRestrictions::AssertIOAllowed(); uint64_t offset = std::max(filesize_, pre_allocated_size_); @@ -565,10 +599,9 @@ class PosixWritableFile : public WritableFile { } virtual Status Close() OVERRIDE { - MAYBE_RETURN_FAILURE(FLAGS_env_inject_io_error, - Status::IOError(Env::kInjectedFailureStatusMsg)); TRACE_EVENT1("io", "PosixWritableFile::Close", "path", filename_); ThreadRestrictions::AssertIOAllowed(); + MAYBE_RETURN_EIO(filename_, IOError(Env::kInjectedFailureStatusMsg, EIO)); Status s; // If we've allocated more space than we used, truncate to the @@ -603,9 +636,8 @@ class PosixWritableFile : public WritableFile { } virtual Status Flush(FlushMode mode) OVERRIDE { - MAYBE_RETURN_FAILURE(FLAGS_env_inject_io_error, - Status::IOError(Env::kInjectedFailureStatusMsg)); TRACE_EVENT1("io", "PosixWritableFile::Flush", "path", filename_); + MAYBE_RETURN_EIO(filename_, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); #if defined(__linux__) int flags = SYNC_FILE_RANGE_WRITE; @@ -687,8 +719,8 @@ class PosixRWFile : public RWFile { virtual Status PreAllocate(uint64_t offset, size_t length, PreAllocateMode mode) OVERRIDE { - MAYBE_RETURN_FAILURE(FLAGS_env_inject_io_error, - Status::IOError(Env::kInjectedFailureStatusMsg)); + MAYBE_RETURN_EIO(filename_, IOError(Env::kInjectedFailureStatusMsg, EIO)); + TRACE_EVENT1("io", "PosixRWFile::PreAllocate", "path", filename_); ThreadRestrictions::AssertIOAllowed(); int falloc_mode = 0; @@ -708,9 +740,8 @@ class PosixRWFile : public RWFile { } virtual Status Truncate(uint64_t length) OVERRIDE { - MAYBE_RETURN_FAILURE(FLAGS_env_inject_io_error, - Status::IOError(Env::kInjectedFailureStatusMsg)); TRACE_EVENT2("io", "PosixRWFile::Truncate", "path", filename_, "length", length); + MAYBE_RETURN_EIO(filename_, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); int ret; RETRY_ON_EINTR(ret, ftruncate(fd_, length)); @@ -725,9 +756,8 @@ class PosixRWFile : public RWFile { virtual Status PunchHole(uint64_t offset, size_t length) OVERRIDE { #if defined(__linux__) - MAYBE_RETURN_FAILURE(FLAGS_env_inject_io_error, - Status::IOError(Env::kInjectedFailureStatusMsg)); TRACE_EVENT1("io", "PosixRWFile::PunchHole", "path", filename_); + MAYBE_RETURN_EIO(filename_, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); if (fallocate(fd_, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, offset, length) < 0) { return IOError(filename_, errno); @@ -739,9 +769,8 @@ class PosixRWFile : public RWFile { } virtual Status Flush(FlushMode mode, uint64_t offset, size_t length) OVERRIDE { - MAYBE_RETURN_FAILURE(FLAGS_env_inject_io_error, - Status::IOError(Env::kInjectedFailureStatusMsg)); TRACE_EVENT1("io", "PosixRWFile::Flush", "path", filename_); + MAYBE_RETURN_EIO(filename_, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); #if defined(__linux__) int flags = SYNC_FILE_RANGE_WRITE; @@ -776,9 +805,8 @@ class PosixRWFile : public RWFile { if (closed_) { return Status::OK(); } - MAYBE_RETURN_FAILURE(FLAGS_env_inject_io_error, - Status::IOError(Env::kInjectedFailureStatusMsg)); TRACE_EVENT1("io", "PosixRWFile::Close", "path", filename_); + MAYBE_RETURN_EIO(filename_, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); Status s; @@ -801,6 +829,7 @@ class PosixRWFile : public RWFile { virtual Status Size(uint64_t* size) const OVERRIDE { TRACE_EVENT1("io", "PosixRWFile::Size", "path", filename_); + MAYBE_RETURN_EIO(filename_, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); struct stat st; if (fstat(fd_, &st) == -1) { @@ -815,6 +844,7 @@ class PosixRWFile : public RWFile { return Status::NotSupported("GetExtentMap not supported on this platform"); #else TRACE_EVENT1("io", "PosixRWFile::GetExtentMap", "path", filename_); + MAYBE_RETURN_EIO(filename_, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); // This allocation size is arbitrary. @@ -908,6 +938,7 @@ class PosixEnv : public Env { virtual Status NewSequentialFile(const std::string& fname, unique_ptr<SequentialFile>* result) OVERRIDE { TRACE_EVENT1("io", "PosixEnv::NewSequentialFile", "path", fname); + MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); FILE* f = fopen(fname.c_str(), "r"); if (f == nullptr) { @@ -927,6 +958,7 @@ class PosixEnv : public Env { const std::string& fname, unique_ptr<RandomAccessFile>* result) OVERRIDE { TRACE_EVENT1("io", "PosixEnv::NewRandomAccessFile", "path", fname); + MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); int fd = open(fname.c_str(), O_RDONLY); if (fd < 0) { @@ -997,6 +1029,7 @@ class PosixEnv : public Env { virtual Status GetChildren(const std::string& dir, std::vector<std::string>* result) OVERRIDE { TRACE_EVENT1("io", "PosixEnv::GetChildren", "path", dir); + MAYBE_RETURN_EIO(dir, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); result->clear(); DIR* d = opendir(dir.c_str()); @@ -1014,6 +1047,7 @@ class PosixEnv : public Env { virtual Status DeleteFile(const std::string& fname) OVERRIDE { TRACE_EVENT1("io", "PosixEnv::DeleteFile", "path", fname); + MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); Status result; if (unlink(fname.c_str()) != 0) { @@ -1024,6 +1058,7 @@ class PosixEnv : public Env { virtual Status CreateDir(const std::string& name) OVERRIDE { TRACE_EVENT1("io", "PosixEnv::CreateDir", "path", name); + MAYBE_RETURN_EIO(name, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); Status result; if (mkdir(name.c_str(), 0777) != 0) { @@ -1034,6 +1069,7 @@ class PosixEnv : public Env { virtual Status DeleteDir(const std::string& name) OVERRIDE { TRACE_EVENT1("io", "PosixEnv::DeleteDir", "path", name); + MAYBE_RETURN_EIO(name, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); Status result; if (rmdir(name.c_str()) != 0) { @@ -1050,11 +1086,14 @@ class PosixEnv : public Env { return IOError("getcwd()", errno); } cwd->assign(wd.get()); + + MAYBE_RETURN_EIO(*cwd, IOError(Env::kInjectedFailureStatusMsg, EIO)); return Status::OK(); } Status ChangeDir(const string& dest) override { TRACE_EVENT1("io", "PosixEnv::ChangeDir", "dest", dest); + MAYBE_RETURN_EIO(dest, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); Status result; if (chdir(dest.c_str()) != 0) { @@ -1065,6 +1104,7 @@ class PosixEnv : public Env { virtual Status SyncDir(const std::string& dirname) OVERRIDE { TRACE_EVENT1("io", "SyncDir", "path", dirname); + MAYBE_RETURN_EIO(dirname, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); if (FLAGS_never_fsync) return Status::OK(); int dir_fd; @@ -1085,6 +1125,7 @@ class PosixEnv : public Env { virtual Status GetFileSize(const std::string& fname, uint64_t* size) OVERRIDE { TRACE_EVENT1("io", "PosixEnv::GetFileSize", "path", fname); + MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); Status s; struct stat sbuf; @@ -1098,6 +1139,7 @@ class PosixEnv : public Env { virtual Status GetFileSizeOnDisk(const std::string& fname, uint64_t* size) OVERRIDE { TRACE_EVENT1("io", "PosixEnv::GetFileSizeOnDisk", "path", fname); + MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); Status s; struct stat sbuf; @@ -1127,6 +1169,7 @@ class PosixEnv : public Env { virtual Status GetBlockSize(const string& fname, uint64_t* block_size) OVERRIDE { TRACE_EVENT1("io", "PosixEnv::GetBlockSize", "path", fname); + MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); Status s; struct stat sbuf; @@ -1140,6 +1183,7 @@ class PosixEnv : public Env { virtual Status GetFileModifiedTime(const string& fname, int64_t* timestamp) override { TRACE_EVENT1("io", "PosixEnv::GetFileModifiedTime", "fname", fname); + MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); struct stat s; @@ -1156,6 +1200,7 @@ class PosixEnv : public Env { // Local convenience function for safely running statvfs(). static Status StatVfs(const string& path, struct statvfs* buf) { + MAYBE_RETURN_EIO(path, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); int ret; RETRY_ON_EINTR(ret, statvfs(path.c_str(), buf)); @@ -1175,9 +1220,9 @@ class PosixEnv : public Env { } virtual Status RenameFile(const std::string& src, const std::string& target) OVERRIDE { - MAYBE_RETURN_FAILURE(FLAGS_env_inject_io_error, - Status::IOError(Env::kInjectedFailureStatusMsg)); TRACE_EVENT2("io", "PosixEnv::RenameFile", "src", src, "dst", target); + MAYBE_RETURN_EIO(src, IOError(Env::kInjectedFailureStatusMsg, EIO)); + MAYBE_RETURN_EIO(target, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); Status result; if (rename(src.c_str(), target.c_str()) != 0) { @@ -1188,6 +1233,7 @@ class PosixEnv : public Env { virtual Status LockFile(const std::string& fname, FileLock** lock) OVERRIDE { TRACE_EVENT1("io", "PosixEnv::LockFile", "path", fname); + MAYBE_RETURN_EIO(fname, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); *lock = nullptr; Status result; @@ -1256,6 +1302,7 @@ class PosixEnv : public Env { } virtual Status GetExecutablePath(string* path) OVERRIDE { + MAYBE_RETURN_EIO("/proc/self/exe", IOError(Env::kInjectedFailureStatusMsg, EIO)); uint32_t size = 64; uint32_t len = 0; while (true) { @@ -1288,6 +1335,7 @@ class PosixEnv : public Env { virtual Status IsDirectory(const string& path, bool* is_dir) OVERRIDE { TRACE_EVENT1("io", "PosixEnv::IsDirectory", "path", path); + MAYBE_RETURN_EIO(path, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); Status s; struct stat sbuf; @@ -1301,6 +1349,7 @@ class PosixEnv : public Env { virtual Status Walk(const string& root, DirectoryOrder order, const WalkCallback& cb) OVERRIDE { TRACE_EVENT1("io", "PosixEnv::Walk", "path", root); + MAYBE_RETURN_EIO(root, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); // Some sanity checks CHECK_NE(root, "/"); @@ -1391,6 +1440,7 @@ class PosixEnv : public Env { virtual Status Canonicalize(const string& path, string* result) OVERRIDE { TRACE_EVENT1("io", "PosixEnv::Canonicalize", "path", path); + MAYBE_RETURN_EIO(path, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); unique_ptr<char[], FreeDeleter> r(realpath(path.c_str(), nullptr)); if (!r) { @@ -1464,6 +1514,7 @@ class PosixEnv : public Env { virtual Status IsOnExtFilesystem(const string& path, bool* result) OVERRIDE { TRACE_EVENT0("io", "PosixEnv::IsOnExtFilesystem"); + MAYBE_RETURN_EIO(path, IOError(Env::kInjectedFailureStatusMsg, EIO)); ThreadRestrictions::AssertIOAllowed(); #ifdef __APPLE__ @@ -1488,6 +1539,7 @@ class PosixEnv : public Env { } Status EnsureFileModeAdheresToUmask(const string& path) override { + MAYBE_RETURN_EIO(path, IOError(Env::kInjectedFailureStatusMsg, EIO)); struct stat s; if (stat(path.c_str(), &s) != 0) { return IOError("stat", errno); @@ -1518,11 +1570,10 @@ class PosixEnv : public Env { }; Status MkTmpFile(const string& name_template, int* fd, string* created_filename) { - MAYBE_RETURN_FAILURE(FLAGS_env_inject_io_error, - Status::IOError(Env::kInjectedFailureStatusMsg)); ThreadRestrictions::AssertIOAllowed(); unique_ptr<char[]> fname(new char[name_template.size() + 1]); ::snprintf(fname.get(), name_template.size() + 1, "%s", name_template.c_str()); + MAYBE_RETURN_EIO(fname.get(), IOError(Env::kInjectedFailureStatusMsg, EIO)); int created_fd = mkstemp(fname.get()); if (created_fd < 0) { return IOError(Substitute("Call to mkstemp() failed on name template $0", name_template), http://git-wip-us.apache.org/repos/asf/kudu/blob/10aeb287/src/kudu/util/fault_injection.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/fault_injection.cc b/src/kudu/util/fault_injection.cc index a14c8f3..20d24e7 100644 --- a/src/kudu/util/fault_injection.cc +++ b/src/kudu/util/fault_injection.cc @@ -62,13 +62,9 @@ void DoInjectRandomLatency(double max_latency_ms) { SleepFor(MonoDelta::FromMilliseconds(g_random->NextDoubleFraction() * max_latency_ms)); } -Status DoMaybeReturnFailure(double fraction, - const Status& bad_status_to_return) { +bool DoMaybeTrue(double fraction) { GoogleOnceInit(&g_random_once, InitRandom); - if (PREDICT_TRUE(g_random->NextDoubleFraction() >= fraction)) { - return Status::OK(); - } - return bad_status_to_return; + return PREDICT_FALSE(g_random->NextDoubleFraction() <= fraction); } } // namespace fault_injection http://git-wip-us.apache.org/repos/asf/kudu/blob/10aeb287/src/kudu/util/fault_injection.h ---------------------------------------------------------------------- diff --git a/src/kudu/util/fault_injection.h b/src/kudu/util/fault_injection.h index cc22bab..43548e6 100644 --- a/src/kudu/util/fault_injection.h +++ b/src/kudu/util/fault_injection.h @@ -42,14 +42,12 @@ #define MAYBE_INJECT_RANDOM_LATENCY(max_ms_flag) \ kudu::fault_injection::MaybeInjectRandomLatency(max_ms_flag) -// With some probability, return the failure described by 'status_expr'. -// -// Unlike the other MAYBE_ macros, this one does not chain to an inline -// function so that 'status_expr' isn't evaluated unless 'fraction_flag' -// really is non-zero. +// With some probability, return the status described by 'status_expr'. +// This will not evaluate 'status_expr' if 'fraction_flag' is zero. #define MAYBE_RETURN_FAILURE(fraction_flag, status_expr) \ - static const Status status_eval = (status_expr); \ - RETURN_NOT_OK(kudu::fault_injection::MaybeReturnFailure(fraction_flag, status_eval)); + if (kudu::fault_injection::MaybeTrue(fraction_flag)) { \ + RETURN_NOT_OK((status_expr)); \ + } // Implementation details below. // Use the MAYBE_FAULT macro instead. @@ -64,8 +62,12 @@ constexpr int kExitStatus = 85; // Out-of-line implementation. void DoMaybeFault(const char* fault_str, double fraction); void DoInjectRandomLatency(double max_latency_ms); -Status DoMaybeReturnFailure(double fraction, - const Status& bad_status_to_return); +bool DoMaybeTrue(double fraction); + +inline bool MaybeTrue(double fraction) { + if (PREDICT_TRUE(fraction <= 0)) return false; + return DoMaybeTrue(fraction); +} inline void MaybeFault(const char* fault_str, double fraction) { if (PREDICT_TRUE(fraction <= 0)) return; @@ -77,12 +79,6 @@ inline void MaybeInjectRandomLatency(double max_latency) { DoInjectRandomLatency(max_latency); } -inline Status MaybeReturnFailure(double fraction, - const Status& bad_status_to_return) { - if (PREDICT_TRUE(fraction <= 0)) return Status::OK(); - return DoMaybeReturnFailure(fraction, bad_status_to_return); -} - } // namespace fault_injection } // namespace kudu #endif /* KUDU_UTIL_FAULT_INJECTION_H */
