You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by dr...@apache.org on 2017/06/07 14:19:17 UTC

[2/2] kudu git commit: util: add fault injection of EIOs

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 <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <da...@gmail.com>


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 <aw...@cloudera.com>
Authored: Thu May 18 13:31:34 2017 -0700
Committer: David Ribeiro Alves <da...@gmail.com>
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 */