You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2018/02/21 02:32:27 UTC

[1/4] kudu git commit: internal_mini_cluster: support Cluster/LogVerifier

Repository: kudu
Updated Branches:
  refs/heads/master 270dd9996 -> a66585398


http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/mini-cluster/external_mini_cluster.cc
----------------------------------------------------------------------
diff --git a/src/kudu/mini-cluster/external_mini_cluster.cc b/src/kudu/mini-cluster/external_mini_cluster.cc
index df5cb12..55a75fa 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.cc
+++ b/src/kudu/mini-cluster/external_mini_cluster.cc
@@ -120,9 +120,13 @@ ExternalMiniCluster::~ExternalMiniCluster() {
   Shutdown();
 }
 
+Env* ExternalMiniCluster::env() const {
+  return Env::Default();
+}
+
 Status ExternalMiniCluster::DeduceBinRoot(std::string* ret) {
   string exe;
-  RETURN_NOT_OK(Env::Default()->GetExecutablePath(&exe));
+  RETURN_NOT_OK(env()->GetExecutablePath(&exe));
   *ret = DirName(exe);
   return Status::OK();
 }
@@ -157,7 +161,7 @@ Status ExternalMiniCluster::Start() {
                         .Build(&messenger_),
                         "Failed to start Messenger for minicluster");
 
-  Status s = Env::Default()->CreateDir(opts_.cluster_root);
+  Status s = env()->CreateDir(opts_.cluster_root);
   if (!s.ok() && !s.IsAlreadyPresent()) {
     RETURN_NOT_OK_PREPEND(s, "Could not create root dir " + opts_.cluster_root);
   }
@@ -673,6 +677,14 @@ Status ExternalMiniCluster::SetFlag(ExternalDaemon* daemon,
   return Status::OK();
 }
 
+string ExternalMiniCluster::WalRootForTS(int ts_idx) const {
+  return tablet_server(ts_idx)->wal_dir();
+}
+
+string ExternalMiniCluster::UuidForTS(int ts_idx) const {
+  return tablet_server(ts_idx)->uuid();
+}
+
 //------------------------------------------------------------
 // ExternalDaemon
 //------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/mini-cluster/external_mini_cluster.h
----------------------------------------------------------------------
diff --git a/src/kudu/mini-cluster/external_mini_cluster.h b/src/kudu/mini-cluster/external_mini_cluster.h
index 0f0c830..a280640 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.h
+++ b/src/kudu/mini-cluster/external_mini_cluster.h
@@ -43,6 +43,7 @@
 
 namespace kudu {
 
+class Env;
 class NodeInstancePB;
 class Sockaddr;
 class Subprocess;
@@ -259,6 +260,15 @@ class ExternalMiniCluster : public MiniCluster {
     return masters_.size();
   }
 
+  // Returns the WALs root directory for the tablet server 'ts_idx'.
+  virtual std::string WalRootForTS(int ts_idx) const override;
+
+  // Returns the UUID for the tablet server 'ts_idx'.
+  virtual std::string UuidForTS(int ts_idx) const override;
+
+  // Returns the Env on which the cluster operates.
+  virtual Env* env() const override;
+
   BindMode bind_mode() const override {
     return opts_.bind_mode;
   }

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/mini-cluster/internal_mini_cluster.cc
----------------------------------------------------------------------
diff --git a/src/kudu/mini-cluster/internal_mini_cluster.cc b/src/kudu/mini-cluster/internal_mini_cluster.cc
index 7e342cc..b9b3187 100644
--- a/src/kudu/mini-cluster/internal_mini_cluster.cc
+++ b/src/kudu/mini-cluster/internal_mini_cluster.cc
@@ -24,6 +24,7 @@
 
 #include "kudu/client/client.h"
 #include "kudu/common/wire_protocol.pb.h"
+#include "kudu/fs/fs_manager.h"
 #include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/master/catalog_manager.h"
@@ -396,5 +397,13 @@ std::shared_ptr<MasterServiceProxy> InternalMiniCluster::master_proxy(int idx) c
   return std::make_shared<MasterServiceProxy>(messenger_, addr, addr.host());
 }
 
+string InternalMiniCluster::WalRootForTS(int ts_idx) const {
+  return mini_tablet_server(ts_idx)->options()->fs_opts.wal_root;
+}
+
+string InternalMiniCluster::UuidForTS(int ts_idx) const {
+  return mini_tablet_server(ts_idx)->uuid();
+}
+
 } // namespace cluster
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/mini-cluster/internal_mini_cluster.h
----------------------------------------------------------------------
diff --git a/src/kudu/mini-cluster/internal_mini_cluster.h b/src/kudu/mini-cluster/internal_mini_cluster.h
index 4f73505..eedac07 100644
--- a/src/kudu/mini-cluster/internal_mini_cluster.h
+++ b/src/kudu/mini-cluster/internal_mini_cluster.h
@@ -152,6 +152,17 @@ class InternalMiniCluster : public MiniCluster {
     return mini_tablet_servers_.size();
   }
 
+  // Returns the WALs root directory for the tablet server 'ts_idx'.
+  std::string WalRootForTS(int ts_idx) const override;
+
+  // Returns the UUID for the tablet server 'ts_idx'.
+  std::string UuidForTS(int ts_idx) const override;
+
+  // Returns the Env on which the cluster operates.
+  Env* env() const override {
+    return env_;
+  }
+
   BindMode bind_mode() const override {
     return opts_.bind_mode;
   }

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/mini-cluster/mini_cluster.h
----------------------------------------------------------------------
diff --git a/src/kudu/mini-cluster/mini_cluster.h b/src/kudu/mini-cluster/mini_cluster.h
index 8a1440c..845b79c 100644
--- a/src/kudu/mini-cluster/mini_cluster.h
+++ b/src/kudu/mini-cluster/mini_cluster.h
@@ -26,6 +26,7 @@
 
 namespace kudu {
 
+class Env;
 class HostPort;
 
 namespace client {
@@ -150,6 +151,15 @@ class MiniCluster {
   // master at 'idx' is running.
   virtual std::shared_ptr<master::MasterServiceProxy> master_proxy(int idx) const = 0;
 
+  // Returns the UUID for the tablet server 'ts_idx'
+  virtual std::string UuidForTS(int ts_idx) const = 0;
+
+  // Returns the WALs root directory for the tablet server 'ts_idx'.
+  virtual std::string WalRootForTS(int ts_idx) const = 0;
+
+  // Returns the Env on which the cluster operates.
+  virtual Env* env() const = 0;
+
  protected:
   // Return the IP address that the daemon with the given index will bind to.
   // If bind_mode is LOOPBACK, this will be 127.0.0.1 and if it is WILDCARD it

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/tablet/tablet_bootstrap.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet_bootstrap.cc b/src/kudu/tablet/tablet_bootstrap.cc
index 759b1b3..04516b4 100644
--- a/src/kudu/tablet/tablet_bootstrap.cc
+++ b/src/kudu/tablet/tablet_bootstrap.cc
@@ -43,6 +43,7 @@
 #include "kudu/consensus/log.h"
 #include "kudu/consensus/log.pb.h"
 #include "kudu/consensus/log_anchor_registry.h"
+#include "kudu/consensus/log_index.h"
 #include "kudu/consensus/log_reader.h"
 #include "kudu/consensus/log_util.h"
 #include "kudu/consensus/metadata.pb.h"
@@ -127,6 +128,7 @@ using consensus::WRITE_OP;
 using log::Log;
 using log::LogAnchorRegistry;
 using log::LogEntryPB;
+using log::LogIndex;
 using log::LogOptions;
 using log::LogReader;
 using log::ReadableLogSegment;
@@ -646,7 +648,7 @@ Status TabletBootstrap::PrepareRecoveryDir(bool* needs_recovery) {
     // Since we have a recovery directory, clear out the log_dir by recursively
     // deleting it and creating a new one so that we don't end up with remnants
     // of old WAL segments or indexes after replay.
-    if (fs_manager->env()->FileExists(log_dir)) {
+    if (fs_manager->Exists(log_dir)) {
       LOG_WITH_PREFIX(INFO) << "Deleting old log files from previous recovery attempt in "
                             << log_dir;
       RETURN_NOT_OK_PREPEND(fs_manager->env()->DeleteRecursively(log_dir),
@@ -696,13 +698,18 @@ Status TabletBootstrap::PrepareRecoveryDir(bool* needs_recovery) {
 }
 
 Status TabletBootstrap::OpenLogReaderInRecoveryDir() {
+  const string& tablet_id = tablet_->tablet_id();
+  FsManager* fs_manager = tablet_meta_->fs_manager();
   VLOG_WITH_PREFIX(1) << "Opening log reader in log recovery dir "
-                      << tablet_meta_->fs_manager()->GetTabletWalRecoveryDir(tablet_->tablet_id());
+                      << fs_manager->GetTabletWalRecoveryDir(tablet_id);
   // Open the reader.
-  RETURN_NOT_OK_PREPEND(LogReader::OpenFromRecoveryDir(tablet_->metadata()->fs_manager(),
-                                                       tablet_->metadata()->tablet_id(),
-                                                       tablet_->GetMetricEntity().get(),
-                                                       &log_reader_),
+  // Since we're recovering, we don't want to have any log index -- since it
+  // isn't fsynced() during writing, its contents are useless to us.
+  scoped_refptr<LogIndex> log_index(nullptr);
+  const string recovery_dir = fs_manager->GetTabletWalRecoveryDir(tablet_id);
+  RETURN_NOT_OK_PREPEND(LogReader::Open(fs_manager->env(), recovery_dir, log_index, tablet_id,
+                                        tablet_->GetMetricEntity().get(),
+                                        &log_reader_),
                         "Could not open LogReader. Reason");
   return Status::OK();
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/tools/kudu-tool-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index de8acf8..2463dfa 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -73,7 +73,7 @@
 #include "kudu/gutil/strings/strip.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
 #include "kudu/mini-cluster/internal_mini_cluster.h"
@@ -140,7 +140,7 @@ using consensus::ReplicateMsg;
 using fs::BlockDeletionTransaction;
 using fs::FsReport;
 using fs::WritableBlock;
-using itest::ExternalMiniClusterFsInspector;
+using itest::MiniClusterFsInspector;
 using itest::TServerDetails;
 using log::Log;
 using log::LogOptions;
@@ -329,7 +329,7 @@ class ToolTest : public KuduTest {
   void StartExternalMiniCluster(ExternalMiniClusterOptions opts = {});
   void StartMiniCluster(InternalMiniClusterOptions opts = {});
   unique_ptr<ExternalMiniCluster> cluster_;
-  unique_ptr<ExternalMiniClusterFsInspector> inspect_;
+  unique_ptr<MiniClusterFsInspector> inspect_;
   unordered_map<string, TServerDetails*> ts_map_;
   unique_ptr<InternalMiniCluster> mini_cluster_;
 };
@@ -337,7 +337,7 @@ class ToolTest : public KuduTest {
 void ToolTest::StartExternalMiniCluster(ExternalMiniClusterOptions opts) {
   cluster_.reset(new ExternalMiniCluster(std::move(opts)));
   ASSERT_OK(cluster_->Start());
-  inspect_.reset(new ExternalMiniClusterFsInspector(cluster_.get()));
+  inspect_.reset(new MiniClusterFsInspector(cluster_.get()));
   ASSERT_OK(CreateTabletServerMap(cluster_->master_proxy(),
                                   cluster_->messenger(), &ts_map_));
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/tools/kudu-ts-cli-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/kudu-ts-cli-test.cc b/src/kudu/tools/kudu-ts-cli-test.cc
index 3a52fdf..736afc8 100644
--- a/src/kudu/tools/kudu-ts-cli-test.cc
+++ b/src/kudu/tools/kudu-ts-cli-test.cc
@@ -26,7 +26,7 @@
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/external_mini_cluster-itest-base.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
 #include "kudu/tablet/metadata.pb.h"


[2/4] kudu git commit: internal_mini_cluster: support Cluster/LogVerifier

Posted by al...@apache.org.
internal_mini_cluster: support Cluster/LogVerifier

This patch introduces a MiniCluster-agnostic MiniClusterFsInspector.
With this, the LogVerifier, and thus, the ClusterVerifier can support
both External- and InternalMiniClusters.

The LogVerifier would originally open a read-only FsManager (which in
turn would open a BlockManager and a DataDirManager) per server and pass
it to the LogReader to inspect the WALs. Instead, the LogVerifier now
creates a MiniClusterFsInspector, which is more lightweight and defined
per cluster. To the LogReader, it passes the WAL directory and an env,
which is all the LogReader needed from the FsManager in the first place.

To test, I updated a test case in ts_tablet_manager-itest to make use of
the new ClusterVerifier with an internal cluster. CheckCluster() uses a
LogVerifier, which in this case uses an MiniClusterFsInspector that
operates on an InternalMiniCluster.

Change-Id: I228a6e3ba1a42db4e243ffdc5116f0c60ee04a84
Reviewed-on: http://gerrit.cloudera.org:8080/9137
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/2165ce57
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/2165ce57
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/2165ce57

Branch: refs/heads/master
Commit: 2165ce577e208f27a18d3a98fdf97b9f3333b7d5
Parents: 270dd99
Author: Andrew Wong <aw...@cloudera.com>
Authored: Wed Jan 24 17:46:49 2018 -0800
Committer: Andrew Wong <aw...@cloudera.com>
Committed: Wed Feb 21 00:10:38 2018 +0000

----------------------------------------------------------------------
 src/kudu/consensus/log-test.cc                  |   2 +-
 src/kudu/consensus/log_reader.cc                |  48 +--
 src/kudu/consensus/log_reader.h                 |  26 +-
 src/kudu/fs/fs_manager.h                        |   4 +-
 src/kudu/integration-tests/CMakeLists.txt       |   2 +-
 .../integration-tests/client_failover-itest.cc  |   2 +-
 src/kudu/integration-tests/cluster_verifier.cc  |  21 +-
 .../integration-tests/create-table-itest.cc     |   2 +-
 .../integration-tests/delete_table-itest.cc     |   2 +-
 .../external_mini_cluster-itest-base.cc         |   4 +-
 .../external_mini_cluster-itest-base.h          |   4 +-
 .../external_mini_cluster_fs_inspector.cc       | 383 -------------------
 .../external_mini_cluster_fs_inspector.h        | 139 -------
 src/kudu/integration-tests/log_verifier.cc      |  65 ++--
 src/kudu/integration-tests/log_verifier.h       |  29 +-
 .../mini_cluster_fs_inspector.cc                | 377 ++++++++++++++++++
 .../mini_cluster_fs_inspector.h                 | 140 +++++++
 .../raft_config_change-itest.cc                 |   2 +-
 .../raft_consensus-itest-base.cc                |   2 +-
 .../integration-tests/raft_consensus-itest.cc   |   5 +-
 .../raft_consensus_election-itest.cc            |   2 +-
 .../raft_consensus_nonvoter-itest.cc            |   2 +-
 src/kudu/integration-tests/tablet_copy-itest.cc |   2 +-
 .../tablet_copy_client_session-itest.cc         |   2 +-
 .../tablet_replacement-itest.cc                 |   2 +-
 .../tombstoned_voting-itest.cc                  |   2 +-
 .../tombstoned_voting-stress-test.cc            |   2 +-
 src/kudu/integration-tests/ts_itest-base.cc     |   4 +-
 src/kudu/integration-tests/ts_itest-base.h      |   4 +-
 src/kudu/integration-tests/ts_recovery-itest.cc |   8 +-
 .../ts_tablet_manager-itest.cc                  |  28 +-
 src/kudu/mini-cluster/external_mini_cluster.cc  |  16 +-
 src/kudu/mini-cluster/external_mini_cluster.h   |  10 +
 src/kudu/mini-cluster/internal_mini_cluster.cc  |   9 +
 src/kudu/mini-cluster/internal_mini_cluster.h   |  11 +
 src/kudu/mini-cluster/mini_cluster.h            |  10 +
 src/kudu/tablet/tablet_bootstrap.cc             |  19 +-
 src/kudu/tools/kudu-tool-test.cc                |   8 +-
 src/kudu/tools/kudu-ts-cli-test.cc              |   2 +-
 39 files changed, 705 insertions(+), 697 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/consensus/log-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/log-test.cc b/src/kudu/consensus/log-test.cc
index 11aca81..3d56b96 100644
--- a/src/kudu/consensus/log-test.cc
+++ b/src/kudu/consensus/log-test.cc
@@ -778,7 +778,7 @@ TEST_P(LogTestOptionalCompression, TestWriteManyBatches) {
 // seg003: 0.20 through 0.29
 // seg004: 0.30 through 0.39
 TEST_P(LogTestOptionalCompression, TestLogReader) {
-  LogReader reader(fs_manager_.get(),
+  LogReader reader(env_,
                    scoped_refptr<LogIndex>(),
                    kTestTablet,
                    nullptr);

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/consensus/log_reader.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/log_reader.cc b/src/kudu/consensus/log_reader.cc
index 88adbb5..d357f23 100644
--- a/src/kudu/consensus/log_reader.cc
+++ b/src/kudu/consensus/log_reader.cc
@@ -74,44 +74,36 @@ struct LogSegmentSeqnoComparator {
 
 const int64_t LogReader::kNoSizeLimit = -1;
 
-Status LogReader::Open(FsManager* fs_manager,
+Status LogReader::Open(Env* env,
+                       const string& tablet_wal_dir,
                        const scoped_refptr<LogIndex>& index,
                        const string& tablet_id,
                        const scoped_refptr<MetricEntity>& metric_entity,
                        shared_ptr<LogReader>* reader) {
   auto log_reader = std::make_shared<LogReader>(
-      fs_manager, index, tablet_id, metric_entity);
-
-  string tablet_wal_path = fs_manager->GetTabletWalDir(tablet_id);
+      env, index, tablet_id, metric_entity);
 
-  RETURN_NOT_OK(log_reader->Init(tablet_wal_path))
+  RETURN_NOT_OK_PREPEND(log_reader->Init(tablet_wal_dir),
+                        "Unable to initialize log reader")
   *reader = log_reader;
   return Status::OK();
 }
 
-Status LogReader::OpenFromRecoveryDir(FsManager* fs_manager,
-                                      const string& tablet_id,
-                                      const scoped_refptr<MetricEntity>& metric_entity,
-                                      shared_ptr<LogReader>* reader) {
-  string recovery_path = fs_manager->GetTabletWalRecoveryDir(tablet_id);
-
-  // When recovering, we don't want to have any log index -- since it isn't fsynced()
-  // during writing, its contents are useless to us.
-  scoped_refptr<LogIndex> index(nullptr);
-  auto log_reader = std::make_shared<LogReader>(
-      fs_manager, index, tablet_id, metric_entity);
-  RETURN_NOT_OK_PREPEND(log_reader->Init(recovery_path),
-                        "Unable to initialize log reader");
-  *reader = log_reader;
-  return Status::OK();
+Status LogReader::Open(FsManager* fs_manager,
+                       const scoped_refptr<LogIndex>& index,
+                       const std::string& tablet_id,
+                       const scoped_refptr<MetricEntity>& metric_entity,
+                       std::shared_ptr<LogReader>* reader) {
+  return LogReader::Open(fs_manager->env(), fs_manager->GetTabletWalDir(tablet_id),
+                         index, tablet_id, metric_entity, reader);
 }
 
-LogReader::LogReader(FsManager* fs_manager,
-                     const scoped_refptr<LogIndex>& index,
+LogReader::LogReader(Env* env,
+                     scoped_refptr<LogIndex> index,
                      string tablet_id,
                      const scoped_refptr<MetricEntity>& metric_entity)
-    : fs_manager_(fs_manager),
-      log_index_(index),
+    : env_(env),
+      log_index_(std::move(index)),
       tablet_id_(std::move(tablet_id)),
       state_(kLogReaderInitialized) {
   if (metric_entity) {
@@ -131,9 +123,7 @@ Status LogReader::Init(const string& tablet_wal_path) {
   }
   VLOG(1) << "Reading wal from path:" << tablet_wal_path;
 
-  Env* env = fs_manager_->env();
-
-  if (!fs_manager_->Exists(tablet_wal_path)) {
+  if (!env_->FileExists(tablet_wal_path)) {
     return Status::IllegalState("Cannot find wal location at", tablet_wal_path);
   }
 
@@ -141,7 +131,7 @@ Status LogReader::Init(const string& tablet_wal_path) {
   // list existing segment files
   vector<string> log_files;
 
-  RETURN_NOT_OK_PREPEND(env->GetChildren(tablet_wal_path, &log_files),
+  RETURN_NOT_OK_PREPEND(env_->GetChildren(tablet_wal_path, &log_files),
                         "Unable to read children from path");
 
   SegmentSequence read_segments;
@@ -151,7 +141,7 @@ Status LogReader::Init(const string& tablet_wal_path) {
     if (HasPrefixString(log_file, FsManager::kWalFileNamePrefix)) {
       string fqp = JoinPathSegments(tablet_wal_path, log_file);
       scoped_refptr<ReadableLogSegment> segment;
-      Status s = ReadableLogSegment::Open(env, fqp, &segment);
+      Status s = ReadableLogSegment::Open(env_, fqp, &segment);
       if (s.IsUninitialized()) {
         // This indicates that the segment was created but the writer
         // crashed before the header was successfully written. In this

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/consensus/log_reader.h
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/log_reader.h b/src/kudu/consensus/log_reader.h
index 312868f..bc88cb3 100644
--- a/src/kudu/consensus/log_reader.h
+++ b/src/kudu/consensus/log_reader.h
@@ -35,6 +35,7 @@
 namespace kudu {
 
 class Counter;
+class Env;
 class FsManager;
 class Histogram;
 class MetricEntity;
@@ -57,23 +58,25 @@ class LogReader {
  public:
   ~LogReader();
 
-  // Opens a LogReader on the default tablet log directory, and sets
-  // 'reader' to the newly created LogReader.
+  // Opens a LogReader on the tablet log directory specified by
+  // 'tablet_wal_dir', and sets 'reader' to the newly created LogReader.
   //
   // 'index' may be NULL, but if it is, ReadReplicatesInRange() may not
   // be used.
-  static Status Open(FsManager* fs_manager,
+  static Status Open(Env* env,
+                     const std::string& tablet_wal_dir,
                      const scoped_refptr<LogIndex>& index,
                      const std::string& tablet_id,
                      const scoped_refptr<MetricEntity>& metric_entity,
                      std::shared_ptr<LogReader>* reader);
 
-  // Opens a LogReader on a specific tablet log recovery directory, and sets
-  // 'reader' to the newly created LogReader.
-  static Status OpenFromRecoveryDir(FsManager* fs_manager,
-                                    const std::string& tablet_id,
-                                    const scoped_refptr<MetricEntity>& metric_entity,
-                                    std::shared_ptr<LogReader>* reader);
+  // Same as above, but will use `fs_manager` to determine the default WAL dir
+  // for the tablet.
+  static Status Open(FsManager* fs_manager,
+                     const scoped_refptr<LogIndex>& index,
+                     const std::string& tablet_id,
+                     const scoped_refptr<MetricEntity>& metric_entity,
+                     std::shared_ptr<LogReader>* reader);
 
   // Return the minimum replicate index that is retained in the currently available
   // logs. May return -1 if no replicates have been logged.
@@ -165,8 +168,7 @@ class LogReader {
                                   faststring* tmp_buf,
                                   gscoped_ptr<LogEntryBatchPB>* batch) const;
 
-  LogReader(FsManager* fs_manager, const scoped_refptr<LogIndex>& index,
-            std::string tablet_id,
+  LogReader(Env* env, scoped_refptr<LogIndex> index, std::string tablet_id,
             const scoped_refptr<MetricEntity>& metric_entity);
 
   // Reads the headers of all segments in 'tablet_wal_path'.
@@ -175,7 +177,7 @@ class LogReader {
   // Initializes an 'empty' reader for tests, i.e. does not scan a path looking for segments.
   Status InitEmptyReaderForTests();
 
-  FsManager *fs_manager_;
+  Env* env_;
   const scoped_refptr<LogIndex> log_index_;
   const std::string tablet_id_;
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/fs/fs_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/fs/fs_manager.h b/src/kudu/fs/fs_manager.h
index 955d159..9a29e57 100644
--- a/src/kudu/fs/fs_manager.h
+++ b/src/kudu/fs/fs_manager.h
@@ -56,7 +56,7 @@ struct CreateBlockOptions;
 } // namespace fs
 
 namespace itest {
-class ExternalMiniClusterFsInspector;
+class MiniClusterFsInspector;
 } // namespace itest
 
 namespace tserver {
@@ -264,7 +264,7 @@ class FsManager {
   FRIEND_TEST(FsManagerTestBase, TestMetadataDirInDataRoot);
   FRIEND_TEST(FsManagerTestBase, TestIsolatedMetadataDir);
   FRIEND_TEST(tserver::MiniTabletServerTest, TestFsLayoutEndToEnd);
-  friend class itest::ExternalMiniClusterFsInspector; // for access to directory names
+  friend class itest::MiniClusterFsInspector; // for access to directory names
 
   // Initializes, sanitizes, and canonicalizes the filesystem roots.
   // Determines the correct filesystem root for tablet-specific metadata.

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/CMakeLists.txt b/src/kudu/integration-tests/CMakeLists.txt
index 17fbc68..af5dae9 100644
--- a/src/kudu/integration-tests/CMakeLists.txt
+++ b/src/kudu/integration-tests/CMakeLists.txt
@@ -24,9 +24,9 @@ set(INTEGRATION_TESTS_SRCS
   cluster_itest_util.cc
   cluster_verifier.cc
   external_mini_cluster-itest-base.cc
-  external_mini_cluster_fs_inspector.cc
   internal_mini_cluster-itest-base.cc
   log_verifier.cc
+  mini_cluster_fs_inspector.cc
   raft_consensus-itest-base.cc
   test_workload.cc
   ts_itest-base.cc

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/client_failover-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/client_failover-itest.cc b/src/kudu/integration-tests/client_failover-itest.cc
index 47cedf4..fb23592 100644
--- a/src/kudu/integration-tests/client_failover-itest.cc
+++ b/src/kudu/integration-tests/client_failover-itest.cc
@@ -36,7 +36,7 @@
 #include "kudu/gutil/map-util.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/external_mini_cluster-itest-base.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
 #include "kudu/tablet/metadata.pb.h"

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/cluster_verifier.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/cluster_verifier.cc b/src/kudu/integration-tests/cluster_verifier.cc
index 4e2147b..b903a81 100644
--- a/src/kudu/integration-tests/cluster_verifier.cc
+++ b/src/kudu/integration-tests/cluster_verifier.cc
@@ -29,7 +29,6 @@
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/integration-tests/cluster_verifier.h"
 #include "kudu/integration-tests/log_verifier.h"
-#include "kudu/mini-cluster/external_mini_cluster.h"
 #include "kudu/mini-cluster/mini_cluster.h"
 #include "kudu/tools/ksck.h"
 #include "kudu/tools/ksck_remote.h"
@@ -44,7 +43,6 @@ using std::vector;
 
 namespace kudu {
 
-using cluster::ExternalMiniCluster;
 using cluster::MiniCluster;
 using strings::Substitute;
 using tools::Ksck;
@@ -89,17 +87,14 @@ void ClusterVerifier::CheckCluster() {
   }
   ASSERT_OK(s);
 
-  // TODO(todd): we should support LogVerifier on internal clusters!
-  if (ExternalMiniCluster* emc = dynamic_cast<ExternalMiniCluster*>(cluster_)) {
-    // Verify that the committed op indexes match up across the servers.
-    // We have to use "AssertEventually" here because many tests verify clusters
-    // while they are still running, and the verification can fail spuriously in
-    // the case that
-    LogVerifier lv(emc);
-    AssertEventually([&]() {
-        ASSERT_OK(lv.VerifyCommittedOpIdsMatch());
-      });
-  }
+  LogVerifier lv(cluster_);
+  // Verify that the committed op indexes match up across the servers.  We have
+  // to use "AssertEventually" here because many tests verify clusters while
+  // they are still running, and the verification can fail spuriously in the
+  // case that
+  AssertEventually([&]() {
+    ASSERT_OK(lv.VerifyCommittedOpIdsMatch());
+  });
 }
 
 Status ClusterVerifier::DoKsck() {

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/create-table-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/create-table-itest.cc b/src/kudu/integration-tests/create-table-itest.cc
index b223156..1bed8ff 100644
--- a/src/kudu/integration-tests/create-table-itest.cc
+++ b/src/kudu/integration-tests/create-table-itest.cc
@@ -43,7 +43,7 @@
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/external_mini_cluster-itest-base.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/master/master.pb.h"
 #include "kudu/master/master.proxy.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/delete_table-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/delete_table-itest.cc b/src/kudu/integration-tests/delete_table-itest.cc
index 3424e58..12b6848 100644
--- a/src/kudu/integration-tests/delete_table-itest.cc
+++ b/src/kudu/integration-tests/delete_table-itest.cc
@@ -54,7 +54,7 @@
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/cluster_verifier.h"
 #include "kudu/integration-tests/external_mini_cluster-itest-base.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/master/master.pb.h"
 #include "kudu/master/master.proxy.h"

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/external_mini_cluster-itest-base.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster-itest-base.cc b/src/kudu/integration-tests/external_mini_cluster-itest-base.cc
index db7d83b..756a1b9 100644
--- a/src/kudu/integration-tests/external_mini_cluster-itest-base.cc
+++ b/src/kudu/integration-tests/external_mini_cluster-itest-base.cc
@@ -28,7 +28,7 @@
 
 #include "kudu/gutil/stl_util.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
 #include "kudu/util/pstack_watcher.h"
 #include "kudu/util/status.h"
@@ -64,7 +64,7 @@ void ExternalMiniClusterITestBase::StartClusterWithOpts(
     ExternalMiniClusterOptions opts) {
   cluster_.reset(new ExternalMiniCluster(std::move(opts)));
   ASSERT_OK(cluster_->Start());
-  inspect_.reset(new itest::ExternalMiniClusterFsInspector(cluster_.get()));
+  inspect_.reset(new itest::MiniClusterFsInspector(cluster_.get()));
   ASSERT_OK(itest::CreateTabletServerMap(cluster_->master_proxy(),
                                          cluster_->messenger(),
                                          &ts_map_));

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/external_mini_cluster-itest-base.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster-itest-base.h b/src/kudu/integration-tests/external_mini_cluster-itest-base.h
index eba956c..29c2c57 100644
--- a/src/kudu/integration-tests/external_mini_cluster-itest-base.h
+++ b/src/kudu/integration-tests/external_mini_cluster-itest-base.h
@@ -23,7 +23,7 @@
 #include <vector>
 
 #include "kudu/client/shared_ptr.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
 #include "kudu/util/test_util.h"
 
@@ -53,7 +53,7 @@ class ExternalMiniClusterITestBase : public KuduTest {
   void StopCluster();
 
   std::unique_ptr<cluster::ExternalMiniCluster> cluster_;
-  std::unique_ptr<itest::ExternalMiniClusterFsInspector> inspect_;
+  std::unique_ptr<itest::MiniClusterFsInspector> inspect_;
   client::sp::shared_ptr<client::KuduClient> client_;
   std::unordered_map<std::string, itest::TServerDetails*> ts_map_;
 };

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc b/src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc
deleted file mode 100644
index 911f9ea..0000000
--- a/src/kudu/integration-tests/external_mini_cluster_fs_inspector.cc
+++ /dev/null
@@ -1,383 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements.  See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership.  The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License.  You may obtain a copy of the License at
-//
-//   http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied.  See the License for the
-// specific language governing permissions and limitations
-// under the License.
-
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
-
-#include <algorithm>
-#include <set>
-
-#include <glog/logging.h>
-
-#include "kudu/consensus/metadata.pb.h"
-#include "kudu/gutil/strings/join.h"
-#include "kudu/gutil/strings/substitute.h"
-#include "kudu/gutil/strings/util.h"
-#include "kudu/fs/fs_manager.h"
-#include "kudu/mini-cluster/external_mini_cluster.h"
-#include "kudu/util/env.h"
-#include "kudu/util/env_util.h"
-#include "kudu/util/monotime.h"
-#include "kudu/util/path_util.h"
-#include "kudu/util/pb_util.h"
-#include "kudu/util/status.h"
-
-namespace kudu {
-namespace itest {
-
-using std::set;
-using std::string;
-using std::vector;
-using cluster::ExternalMiniCluster;
-using consensus::ConsensusMetadataPB;
-using env_util::ListFilesInDir;
-using strings::Substitute;
-using tablet::TabletDataState;
-using tablet::TabletSuperBlockPB;
-
-ExternalMiniClusterFsInspector::ExternalMiniClusterFsInspector(ExternalMiniCluster* cluster)
-    : env_(Env::Default()),
-      cluster_(CHECK_NOTNULL(cluster)) {
-}
-
-ExternalMiniClusterFsInspector::~ExternalMiniClusterFsInspector() {}
-
-int ExternalMiniClusterFsInspector::CountFilesInDir(const string& path,
-                                                    StringPiece pattern) {
-  vector<string> entries;
-  Status s = ListFilesInDir(env_, path, &entries);
-  if (!s.ok()) return 0;
-  return std::count_if(entries.begin(), entries.end(), [&](const string& s) {
-      return pattern.empty() || MatchPattern(s, pattern);
-    });
-}
-
-int ExternalMiniClusterFsInspector::CountWALFilesOnTS(int index) {
-  string ts_wal_dir = JoinPathSegments(cluster_->tablet_server(index)->wal_dir(),
-                                       FsManager::kWalDirName);
-  vector<string> tablets;
-  CHECK_OK(ListFilesInDir(env_, ts_wal_dir, &tablets));
-  int total_segments = 0;
-  for (const string& tablet : tablets) {
-    string tablet_wal_dir = JoinPathSegments(ts_wal_dir, tablet);
-    total_segments += CountFilesInDir(tablet_wal_dir);
-  }
-  return total_segments;
-}
-
-vector<string> ExternalMiniClusterFsInspector::ListTablets() {
-  set<string> tablets;
-  for (int i = 0; i < cluster_->num_tablet_servers(); i++) {
-    auto ts_tablets = ListTabletsOnTS(i);
-    tablets.insert(ts_tablets.begin(), ts_tablets.end());
-  }
-  return vector<string>(tablets.begin(), tablets.end());
-}
-
-vector<string> ExternalMiniClusterFsInspector::ListTabletsOnTS(int index) {
-  string wal_dir = cluster_->tablet_server(index)->wal_dir();
-  string meta_dir = JoinPathSegments(wal_dir, FsManager::kTabletMetadataDirName);
-  vector<string> tablets;
-  CHECK_OK(ListFilesInDir(env_, meta_dir, &tablets));
-  return tablets;
-}
-
-vector<string> ExternalMiniClusterFsInspector::ListTabletsWithDataOnTS(int index) {
-  string wal_dir = JoinPathSegments(cluster_->tablet_server(index)->wal_dir(),
-                                    FsManager::kWalDirName);
-  vector<string> tablets;
-  CHECK_OK(ListFilesInDir(env_, wal_dir, &tablets));
-  return tablets;
-}
-
-int ExternalMiniClusterFsInspector::CountFilesInWALDirForTS(
-    int index,
-    const string& tablet_id,
-    StringPiece pattern) {
-  string wal_dir = JoinPathSegments(cluster_->tablet_server(index)->wal_dir(),
-                                    FsManager::kWalDirName);
-  string tablet_wal_dir = JoinPathSegments(wal_dir, tablet_id);
-  if (!env_->FileExists(tablet_wal_dir)) {
-    return 0;
-  }
-  return CountFilesInDir(tablet_wal_dir, pattern);
-}
-
-bool ExternalMiniClusterFsInspector::DoesConsensusMetaExistForTabletOnTS(int index,
-                                                                         const string& tablet_id) {
-  ConsensusMetadataPB cmeta_pb;
-  Status s = ReadConsensusMetadataOnTS(index, tablet_id, &cmeta_pb);
-  return s.ok();
-}
-
-int ExternalMiniClusterFsInspector::CountReplicasInMetadataDirs() {
-  // Rather than using FsManager's functionality for listing blocks, we just manually
-  // list the contents of the metadata directory. This is because we're using an
-  // external minicluster, and initializing a new FsManager to point at the running
-  // tablet servers isn't easy.
-  int count = 0;
-  for (int i = 0; i < cluster_->num_tablet_servers(); i++) {
-    string wal_dir = cluster_->tablet_server(i)->wal_dir();
-    count += CountFilesInDir(JoinPathSegments(wal_dir, FsManager::kTabletMetadataDirName));
-  }
-  return count;
-}
-
-Status ExternalMiniClusterFsInspector::CheckNoDataOnTS(int index) {
-  const string& wal_dir = cluster_->tablet_server(index)->wal_dir();
-  if (CountFilesInDir(JoinPathSegments(wal_dir, FsManager::kTabletMetadataDirName)) > 0) {
-    return Status::IllegalState("tablet metadata blocks still exist", wal_dir);
-  }
-  if (CountWALFilesOnTS(index) > 0) {
-    return Status::IllegalState("wals still exist", wal_dir);
-  }
-  if (CountFilesInDir(JoinPathSegments(wal_dir, FsManager::kConsensusMetadataDirName)) > 0) {
-    return Status::IllegalState("consensus metadata still exists", wal_dir);
-  }
-  return Status::OK();;
-}
-
-Status ExternalMiniClusterFsInspector::CheckNoData() {
-  for (int i = 0; i < cluster_->num_tablet_servers(); i++) {
-    RETURN_NOT_OK(CheckNoDataOnTS(i));
-  }
-  return Status::OK();;
-}
-
-string ExternalMiniClusterFsInspector::GetTabletSuperBlockPathOnTS(int ts_index,
-                                                                   const string& tablet_id) const {
-  string wal_dir = cluster_->tablet_server(ts_index)->wal_dir();
-  string meta_dir = JoinPathSegments(wal_dir, FsManager::kTabletMetadataDirName);
-  return JoinPathSegments(meta_dir, tablet_id);
-}
-
-Status ExternalMiniClusterFsInspector::ReadTabletSuperBlockOnTS(int index,
-                                                                const string& tablet_id,
-                                                                TabletSuperBlockPB* sb) {
-  const auto& sb_path = GetTabletSuperBlockPathOnTS(index, tablet_id);
-  return pb_util::ReadPBContainerFromPath(env_, sb_path, sb);
-}
-
-int64_t ExternalMiniClusterFsInspector::GetTabletSuperBlockMTimeOrDie(int ts_index,
-                                                                      const string& tablet_id) {
-  int64_t timestamp;
-  CHECK_OK(env_->GetFileModifiedTime(GetTabletSuperBlockPathOnTS(ts_index, tablet_id), &timestamp));
-  return timestamp;
-}
-
-string ExternalMiniClusterFsInspector::GetConsensusMetadataPathOnTS(int index,
-                                                                    const string& tablet_id) const {
-  string wal_dir = cluster_->tablet_server(index)->wal_dir();
-  string cmeta_dir = JoinPathSegments(wal_dir, FsManager::kConsensusMetadataDirName);
-  return JoinPathSegments(cmeta_dir, tablet_id);
-}
-
-Status ExternalMiniClusterFsInspector::ReadConsensusMetadataOnTS(int index,
-                                                                 const string& tablet_id,
-                                                                 ConsensusMetadataPB* cmeta_pb) {
-  auto cmeta_path = GetConsensusMetadataPathOnTS(index, tablet_id);
-  if (!env_->FileExists(cmeta_path)) {
-    return Status::NotFound("Consensus metadata file not found", cmeta_path);
-  }
-  return pb_util::ReadPBContainerFromPath(env_, cmeta_path, cmeta_pb);
-}
-
-Status ExternalMiniClusterFsInspector::WriteConsensusMetadataOnTS(
-    int index,
-    const string& tablet_id,
-    const ConsensusMetadataPB& cmeta_pb) {
-  auto cmeta_path = GetConsensusMetadataPathOnTS(index, tablet_id);
-  return pb_util::WritePBContainerToPath(env_, cmeta_path, cmeta_pb,
-                                         pb_util::OVERWRITE, pb_util::NO_SYNC);
-}
-
-
-Status ExternalMiniClusterFsInspector::CheckTabletDataStateOnTS(
-    int index,
-    const string& tablet_id,
-    const vector<TabletDataState>& allowed_states) {
-
-  TabletSuperBlockPB sb;
-  RETURN_NOT_OK(ReadTabletSuperBlockOnTS(index, tablet_id, &sb));
-  if (std::find(allowed_states.begin(), allowed_states.end(), sb.tablet_data_state()) !=
-      allowed_states.end()) {
-    return Status::OK();
-  }
-
-  vector<string> state_names;
-  for (auto state : allowed_states) {
-    state_names.push_back(TabletDataState_Name(state));
-  }
-  string expected_str = JoinStrings(state_names, ",");
-  if (state_names.size() > 1) {
-    expected_str = "one of: " + expected_str;
-  }
-
-  return Status::IllegalState(Substitute("State $0 unexpected, expected $1",
-                                         TabletDataState_Name(sb.tablet_data_state()),
-                                         expected_str));
-}
-
-Status ExternalMiniClusterFsInspector::WaitForNoData(const MonoDelta& timeout) {
-  MonoTime deadline = MonoTime::Now() + timeout;
-  Status s;
-  while (true) {
-    s = CheckNoData();
-    if (s.ok()) return Status::OK();
-    if (deadline < MonoTime::Now()) {
-      break;
-    }
-    SleepFor(MonoDelta::FromMilliseconds(10));
-  }
-  return Status::TimedOut("Timed out waiting for no data", s.ToString());
-}
-
-Status ExternalMiniClusterFsInspector::WaitForNoDataOnTS(int index, const MonoDelta& timeout) {
-  MonoTime deadline = MonoTime::Now() + timeout;
-  Status s;
-  while (true) {
-    s = CheckNoDataOnTS(index);
-    if (s.ok()) return Status::OK();
-    if (deadline < MonoTime::Now()) {
-      break;
-    }
-    SleepFor(MonoDelta::FromMilliseconds(10));
-  }
-  return Status::TimedOut("Timed out waiting for no data", s.ToString());
-}
-
-Status ExternalMiniClusterFsInspector::WaitForMinFilesInTabletWalDirOnTS(int index,
-                                                                         const string& tablet_id,
-                                                                         int count,
-                                                                         const MonoDelta& timeout) {
-  int seen = 0;
-  MonoTime deadline = MonoTime::Now() + timeout;
-  while (true) {
-    seen = CountFilesInWALDirForTS(index, tablet_id);
-    if (seen >= count) return Status::OK();
-    if (deadline < MonoTime::Now()) {
-      break;
-    }
-    SleepFor(MonoDelta::FromMilliseconds(10));
-  }
-  return Status::TimedOut(Substitute("Timed out waiting for number of WAL segments on tablet $0 "
-                                     "on TS $1 to be $2. Found $3",
-                                     tablet_id, index, count, seen));
-}
-
-Status ExternalMiniClusterFsInspector::WaitForReplicaCount(int expected, const MonoDelta& timeout) {
-  const MonoTime deadline = MonoTime::Now() + timeout;
-  int found;
-  while (true) {
-    found = CountReplicasInMetadataDirs();
-    if (found == expected) {
-      return Status::OK();
-    }
-    if (MonoTime::Now() > deadline) {
-      break;
-    }
-    SleepFor(MonoDelta::FromMilliseconds(10));
-  }
-  return Status::TimedOut(
-      Substitute("Timed out waiting for a total replica count of $0. "
-                 "Found $1 replicas", expected, found));
-}
-
-Status ExternalMiniClusterFsInspector::WaitForTabletDataStateOnTS(
-    int index,
-    const string& tablet_id,
-    const vector<TabletDataState>& expected_states,
-    const MonoDelta& timeout) {
-  MonoTime start = MonoTime::Now();
-  MonoTime deadline = start + timeout;
-  Status s;
-  while (true) {
-    s = CheckTabletDataStateOnTS(index, tablet_id, expected_states);
-    if (s.ok()) return Status::OK();
-    if (MonoTime::Now() > deadline) break;
-    SleepFor(MonoDelta::FromMilliseconds(5));
-  }
-  return Status::TimedOut(Substitute("Timed out after $0 waiting for correct tablet state: $1",
-                                     (MonoTime::Now() - start).ToString(),
-                                     s.ToString()));
-}
-
-Status ExternalMiniClusterFsInspector::WaitForFilePatternInTabletWalDirOnTs(
-    int ts_index, const string& tablet_id,
-    const vector<string>& substrings_required,
-    const vector<string>& substrings_disallowed,
-    const MonoDelta& timeout) {
-  Status s;
-  MonoTime deadline = MonoTime::Now() + timeout;
-
-  string ts_wal_dir = JoinPathSegments(cluster_->tablet_server(ts_index)->wal_dir(),
-                                       FsManager::kWalDirName);
-  string tablet_wal_dir = JoinPathSegments(ts_wal_dir, tablet_id);
-
-  string error_msg;
-  vector<string> entries;
-  while (true) {
-    Status s = ListFilesInDir(env_, tablet_wal_dir, &entries);
-    std::sort(entries.begin(), entries.end());
-
-    error_msg = "";
-    bool any_missing_required = false;
-    for (const string& required_filter : substrings_required) {
-      bool filter_matched = false;
-      for (const string& entry : entries) {
-        if (entry.find(required_filter) != string::npos) {
-          filter_matched = true;
-          break;
-        }
-      }
-      if (!filter_matched) {
-        any_missing_required = true;
-        error_msg += "missing from substrings_required: " + required_filter + "; ";
-        break;
-      }
-    }
-
-    bool any_present_disallowed = false;
-    for (const string& entry : entries) {
-      if (any_present_disallowed) break;
-      for (const string& disallowed_filter : substrings_disallowed) {
-        if (entry.find(disallowed_filter) != string::npos) {
-          any_present_disallowed = true;
-          error_msg += "present from substrings_disallowed: " + entry +
-                       " (" + disallowed_filter + "); ";
-          break;
-        }
-      }
-    }
-
-    if (!any_missing_required && !any_present_disallowed) {
-      return Status::OK();
-    }
-    if (MonoTime::Now() > deadline) {
-      break;
-    }
-    SleepFor(MonoDelta::FromMilliseconds(10));
-  }
-
-  return Status::TimedOut(Substitute("Timed out waiting for file pattern on "
-                                     "tablet $0 on TS $1 in directory $2",
-                                     tablet_id, ts_index, tablet_wal_dir),
-                          error_msg + "entries: " + JoinStrings(entries, ", "));
-}
-
-} // namespace itest
-} // namespace kudu
-

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/external_mini_cluster_fs_inspector.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster_fs_inspector.h b/src/kudu/integration-tests/external_mini_cluster_fs_inspector.h
deleted file mode 100644
index fd82a89..0000000
--- a/src/kudu/integration-tests/external_mini_cluster_fs_inspector.h
+++ /dev/null
@@ -1,139 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements.  See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership.  The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License.  You may obtain a copy of the License at
-//
-//   http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied.  See the License for the
-// specific language governing permissions and limitations
-// under the License.
-#pragma once
-
-#include <cstdint>
-#include <string>
-#include <vector>
-
-#include "kudu/gutil/macros.h"
-#include "kudu/gutil/strings/stringpiece.h"
-#include "kudu/tablet/metadata.pb.h"
-#include "kudu/util/monotime.h"
-
-namespace kudu {
-
-class Env;
-class Status;
-
-namespace cluster {
-class ExternalMiniCluster;
-} // namespace cluster
-
-namespace consensus {
-class ConsensusMetadataPB;
-} // namespace consensus
-
-namespace itest {
-
-// Utility class that digs around in a tablet server's data directory and
-// provides methods useful for integration testing. This class must outlive
-// the Env and ExternalMiniCluster objects that are passed into it.
-class ExternalMiniClusterFsInspector {
- public:
-  // Does not take ownership of the ExternalMiniCluster pointer.
-  explicit ExternalMiniClusterFsInspector(cluster::ExternalMiniCluster* cluster);
-  ~ExternalMiniClusterFsInspector();
-
-  // If provided, files are filtered by the glob-style pattern 'pattern'.
-  int CountFilesInDir(const std::string& path, StringPiece pattern = StringPiece());
-
-  // List all of the tablets with tablet metadata in the cluster.
-  std::vector<std::string> ListTablets();
-
-  // List all of the tablets with tablet metadata on the given tablet server index.
-  // This may include tablets that are tombstoned and not running.
-  std::vector<std::string> ListTabletsOnTS(int index);
-
-  // List the tablet IDs on the given tablet which actually have data (as
-  // evidenced by their having a WAL). This excludes those that are tombstoned.
-  std::vector<std::string> ListTabletsWithDataOnTS(int index);
-
-  // Return the number of files in the WAL directory for the given 'tablet_id' on TS 'index'.
-  // If provided, files are filtered by the glob-style pattern 'pattern'.
-  int CountFilesInWALDirForTS(int index,
-                              const std::string& tablet_id,
-                              StringPiece pattern = StringPiece());
-
-  bool DoesConsensusMetaExistForTabletOnTS(int index, const std::string& tablet_id);
-
-  int CountReplicasInMetadataDirs();
-  Status CheckNoDataOnTS(int index);
-  Status CheckNoData();
-
-  Status ReadTabletSuperBlockOnTS(int index, const std::string& tablet_id,
-                                  tablet::TabletSuperBlockPB* sb);
-
-  // Get the modification time (in micros) of the tablet superblock for the given tablet
-  // server index and tablet ID.
-  int64_t GetTabletSuperBlockMTimeOrDie(int ts_index, const std::string& tablet_id);
-
-  Status ReadConsensusMetadataOnTS(int index, const std::string& tablet_id,
-                                   consensus::ConsensusMetadataPB* cmeta_pb);
-  Status WriteConsensusMetadataOnTS(int index,
-                                    const std::string& tablet_id,
-                                    const consensus::ConsensusMetadataPB& cmeta_pb);
-
-  Status CheckTabletDataStateOnTS(int index,
-                                  const std::string& tablet_id,
-                                  const std::vector<tablet::TabletDataState>& expected_states);
-
-  Status WaitForNoData(const MonoDelta& timeout = MonoDelta::FromSeconds(30));
-  Status WaitForNoDataOnTS(int index, const MonoDelta& timeout = MonoDelta::FromSeconds(30));
-  Status WaitForMinFilesInTabletWalDirOnTS(int index,
-                                           const std::string& tablet_id,
-                                           int count,
-                                           const MonoDelta& timeout = MonoDelta::FromSeconds(60));
-  Status WaitForReplicaCount(int expected, const MonoDelta& timeout = MonoDelta::FromSeconds(30));
-  Status WaitForTabletDataStateOnTS(int index,
-                                    const std::string& tablet_id,
-                                    const std::vector<tablet::TabletDataState>& expected_states,
-                                    const MonoDelta& timeout = MonoDelta::FromSeconds(30));
-
-  // Loop and check for certain filenames in the WAL directory of the specified
-  // tablet. This function returns OK if we reach a state where:
-  // * For each string in 'substrings_required', we find *at least one file*
-  //   whose name contains that string, and:
-  // * For each string in 'substrings_disallowed', we find *no files* whose name
-  //   contains that string, even if the file also matches a string in the
-  //   'substrings_required'.
-  Status WaitForFilePatternInTabletWalDirOnTs(
-      int ts_index,
-      const std::string& tablet_id,
-      const std::vector<std::string>& substrings_required,
-      const std::vector<std::string>& substrings_disallowed,
-      const MonoDelta& timeout = MonoDelta::FromSeconds(30));
-
- private:
-  // Return the number of files in WAL directories on the given tablet server.
-  // This includes log index files (not just segments).
-  int CountWALFilesOnTS(int index);
-
-  std::string GetConsensusMetadataPathOnTS(int index,
-                                           const std::string& tablet_id) const;
-
-  std::string GetTabletSuperBlockPathOnTS(int ts_index,
-                                          const std::string& tablet_id) const;
-
-  Env* const env_;
-  cluster::ExternalMiniCluster* const cluster_;
-
-  DISALLOW_COPY_AND_ASSIGN(ExternalMiniClusterFsInspector);
-};
-
-} // namespace itest
-} // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/log_verifier.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/log_verifier.cc b/src/kudu/integration-tests/log_verifier.cc
index 3a04350..beeb546 100644
--- a/src/kudu/integration-tests/log_verifier.cc
+++ b/src/kudu/integration-tests/log_verifier.cc
@@ -36,14 +36,14 @@
 #include "kudu/consensus/log_reader.h"
 #include "kudu/consensus/log_util.h"
 #include "kudu/consensus/opid.pb.h"
-#include "kudu/fs/fs_manager.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/gutil/strings/substitute.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
-#include "kudu/mini-cluster/external_mini_cluster.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
+#include "kudu/mini-cluster/mini_cluster.h"
 #include "kudu/util/env.h"
 #include "kudu/util/metrics.h"
+#include "kudu/util/path_util.h"
 #include "kudu/util/status.h"
 
 using std::map;
@@ -56,39 +56,26 @@ using strings::Substitute;
 
 namespace kudu {
 
-using cluster::ExternalMiniCluster;
-using cluster::ExternalTabletServer;
+using cluster::MiniCluster;
 using consensus::OpId;
+using itest::MiniClusterFsInspector;
 using log::LogReader;
-using itest::ExternalMiniClusterFsInspector;
 
-LogVerifier::LogVerifier(ExternalMiniCluster* cluster)
-    : cluster_(cluster) {
+LogVerifier::LogVerifier(cluster::MiniCluster* cluster)
+    : cluster_(cluster),
+      env_(cluster->env()) {
+  inspector_.reset(new MiniClusterFsInspector(cluster));
+  CHECK(inspector_);
 }
 
-LogVerifier::~LogVerifier() {
-}
-
-Status LogVerifier::OpenFsManager(ExternalTabletServer* ets,
-                                  unique_ptr<FsManager>* fs) {
-  FsManagerOpts fs_opts;
-  fs_opts.read_only = true;
-  fs_opts.wal_root = ets->wal_dir();
-  fs_opts.data_roots = ets->data_dirs();
-  fs_opts.block_manager_type = cluster_->block_manager_type();
-
-  unique_ptr<FsManager> ret(new FsManager(Env::Default(), fs_opts));
-  RETURN_NOT_OK_PREPEND(ret->Open(),
-                        Substitute("Couldn't initialize FS Manager for $0", ets->wal_dir()));
-  fs->swap(ret);
-  return Status::OK();
-}
+LogVerifier::~LogVerifier() {}
 
-Status LogVerifier::ScanForCommittedOpIds(FsManager* fs, const string& tablet_id,
+Status LogVerifier::ScanForCommittedOpIds(int ts_idx, const string& tablet_id,
                                           map<int64_t, int64_t>* index_to_term) {
 
   shared_ptr<LogReader> reader;
-  RETURN_NOT_OK(LogReader::Open(fs, scoped_refptr<log::LogIndex>(), tablet_id,
+  const string wal_dir = JoinPathSegments(inspector_->WalDirForTS(ts_idx), tablet_id);
+  RETURN_NOT_OK(LogReader::Open(env_, wal_dir, scoped_refptr<log::LogIndex>(), tablet_id,
                                 scoped_refptr<MetricEntity>(), &reader));
   log::SegmentSequence segs;
   RETURN_NOT_OK(reader->GetSegmentsSnapshot(&segs));
@@ -113,14 +100,13 @@ Status LogVerifier::ScanForCommittedOpIds(FsManager* fs, const string& tablet_id
   return Status::OK();
 }
 
-Status LogVerifier::ScanForHighestCommittedOpIdInLog(ExternalTabletServer* ets,
+Status LogVerifier::ScanForHighestCommittedOpIdInLog(int ts_idx,
                                                      const string& tablet_id,
                                                      OpId* commit_id) {
-  unique_ptr<FsManager> fs;
-  RETURN_NOT_OK(OpenFsManager(ets, &fs));
-  const string& wal_dir = fs->GetTabletWalDir(tablet_id);
+  const string& wal_dir = inspector_->WalDirForTS(ts_idx);
   map<int64_t, int64_t> index_to_term;
-  RETURN_NOT_OK_PREPEND(ScanForCommittedOpIds(fs.get(), tablet_id, &index_to_term),
+
+  RETURN_NOT_OK_PREPEND(ScanForCommittedOpIds(ts_idx, tablet_id, &index_to_term),
                         Substitute("Couldn't scan log in dir $0", wal_dir));
   if (index_to_term.empty()) {
     return Status::NotFound("no COMMITs in log");
@@ -131,10 +117,7 @@ Status LogVerifier::ScanForHighestCommittedOpIdInLog(ExternalTabletServer* ets,
 }
 
 Status LogVerifier::VerifyCommittedOpIdsMatch() {
-  ExternalMiniClusterFsInspector inspect(cluster_);
-  Env* env = Env::Default();
-
-  for (const string& tablet_id : inspect.ListTablets()) {
+  for (const string& tablet_id : inspector_->ListTablets()) {
     LOG(INFO) << "Checking tablet " << tablet_id;
 
     // Union set of the op indexes seen on any server.
@@ -145,12 +128,10 @@ Status LogVerifier::VerifyCommittedOpIdsMatch() {
     // Gather the [index->term] map for each of the tablet servers
     // hosting this tablet.
     for (int i = 0; i < cluster_->num_tablet_servers(); i++) {
-      unique_ptr<FsManager> fs;
-      RETURN_NOT_OK(OpenFsManager(cluster_->tablet_server(i), &fs));
-      const string& wal_dir = fs->GetTabletWalDir(tablet_id);
-      if (!env->FileExists(wal_dir)) continue;
+      const string& wal_dir = JoinPathSegments(inspector_->WalDirForTS(i), tablet_id);
+      if (!env_->FileExists(wal_dir)) continue;
       map<int64_t, int64_t> index_to_term;
-      RETURN_NOT_OK_PREPEND(ScanForCommittedOpIds(fs.get(), tablet_id, &index_to_term),
+      RETURN_NOT_OK_PREPEND(ScanForCommittedOpIds(i, tablet_id, &index_to_term),
                             Substitute("Couldn't scan log for TS $0", i));
       for (const auto& index_term : index_to_term) {
         all_op_indexes.insert(index_term.first);
@@ -179,7 +160,7 @@ Status LogVerifier::VerifyCommittedOpIdsMatch() {
           for (int i = 0; i < cluster_->num_tablet_servers(); i++) {
             if (i != 0) err += ", ";
             strings::SubstituteAndAppend(&err, "T $0=$1",
-                                         cluster_->tablet_server(i)->uuid(),
+                                         cluster_->UuidForTS(i),
                                          committed_terms[i]);
           }
           err += "]";

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/log_verifier.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/log_verifier.h b/src/kudu/integration-tests/log_verifier.h
index 7f3d293..d831e6a 100644
--- a/src/kudu/integration-tests/log_verifier.h
+++ b/src/kudu/integration-tests/log_verifier.h
@@ -26,21 +26,24 @@
 
 namespace kudu {
 
+class Env;
+
 namespace cluster {
-class ExternalMiniCluster;
-class ExternalTabletServer;
+class MiniCluster;
 } // namespace cluster
 
-class FsManager;
-
 namespace consensus {
 class OpId;
 } // namespace consensus
 
+namespace itest {
+class MiniClusterFsInspector;
+}
+
 // Verifies correctness of the logs in an external mini-cluster.
 class LogVerifier {
  public:
-  explicit LogVerifier(cluster::ExternalMiniCluster* cluster);
+  explicit LogVerifier(cluster::MiniCluster* cluster);
   ~LogVerifier();
 
   // Verify that, for every tablet in the cluster, the logs of each of that tablet's replicas
@@ -58,21 +61,19 @@ class LogVerifier {
 
   // Scans the WAL on the given tablet server to find the COMMIT message with the highest
   // index.
-  Status ScanForHighestCommittedOpIdInLog(cluster::ExternalTabletServer* ets,
+  Status ScanForHighestCommittedOpIdInLog(int ts_idx,
                                           const std::string& tablet_id,
                                           consensus::OpId* commit_id);
 
  private:
-  // Open an FsManager for the given tablet server.
-  Status OpenFsManager(cluster::ExternalTabletServer* ets,
-                       std::unique_ptr<FsManager>* fs);
-
-  // Scan the WALs for tablet 'tablet_id' on the given 'fs'. Sets entries
-  // in '*index_to_term' for each COMMIT entry found in the WALs.
-  Status ScanForCommittedOpIds(FsManager* fs, const std::string& tablet_id,
+  // Scan the WALs for tablet 'tablet_id' on the server specified by 'ts_idx'.
+  // Sets entries in '*index_to_term' for each COMMIT entry found in the WALs.
+  Status ScanForCommittedOpIds(int ts_idx, const std::string& tablet_id,
                                std::map<int64_t, int64_t>* index_to_term);
 
-  cluster::ExternalMiniCluster* const cluster_;
+  cluster::MiniCluster* const cluster_;
+  Env* const env_;
+  std::unique_ptr<itest::MiniClusterFsInspector> inspector_;
 
   DISALLOW_COPY_AND_ASSIGN(LogVerifier);
 };

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/mini_cluster_fs_inspector.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/mini_cluster_fs_inspector.cc b/src/kudu/integration-tests/mini_cluster_fs_inspector.cc
new file mode 100644
index 0000000..8c186a5
--- /dev/null
+++ b/src/kudu/integration-tests/mini_cluster_fs_inspector.cc
@@ -0,0 +1,377 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
+
+#include <algorithm>
+#include <set>
+
+#include <glog/logging.h>
+
+#include "kudu/consensus/metadata.pb.h"
+#include "kudu/fs/fs_manager.h"
+#include "kudu/gutil/strings/join.h"
+#include "kudu/gutil/strings/substitute.h"
+#include "kudu/gutil/strings/util.h"
+#include "kudu/mini-cluster/mini_cluster.h"
+#include "kudu/util/env.h"
+#include "kudu/util/env_util.h"
+#include "kudu/util/monotime.h"
+#include "kudu/util/path_util.h"
+#include "kudu/util/pb_util.h"
+#include "kudu/util/status.h"
+
+namespace kudu {
+namespace itest {
+
+using cluster::MiniCluster;
+using std::set;
+using std::string;
+using std::vector;
+using consensus::ConsensusMetadataPB;
+using env_util::ListFilesInDir;
+using strings::Substitute;
+using tablet::TabletDataState;
+using tablet::TabletSuperBlockPB;
+
+MiniClusterFsInspector::MiniClusterFsInspector(MiniCluster* cluster)
+    : cluster_(cluster),
+      env_(cluster->env()) {}
+
+int MiniClusterFsInspector::CountFilesInDir(const string& path,
+                                            StringPiece pattern) {
+  vector<string> entries;
+  Status s = ListFilesInDir(env_, path, &entries);
+  if (!s.ok()) return 0;
+  return std::count_if(entries.begin(), entries.end(), [&](const string& s) {
+      return pattern.empty() || MatchPattern(s, pattern);
+    });
+}
+
+string MiniClusterFsInspector::WalDirForTS(int ts_idx) const {
+  return JoinPathSegments(cluster_->WalRootForTS(ts_idx), FsManager::kWalDirName);
+}
+
+int MiniClusterFsInspector::CountWALFilesOnTS(int ts_idx) {
+  string ts_wal_dir = WalDirForTS(ts_idx);
+  vector<string> tablets;
+  CHECK_OK(ListFilesInDir(env_, ts_wal_dir, &tablets));
+  int total_segments = 0;
+  for (const string& tablet : tablets) {
+    string tablet_wal_dir = JoinPathSegments(ts_wal_dir, tablet);
+    total_segments += CountFilesInDir(tablet_wal_dir);
+  }
+  return total_segments;
+}
+
+vector<string> MiniClusterFsInspector::ListTablets() {
+  set<string> tablets;
+  for (int i = 0; i < cluster_->num_tablet_servers(); i++) {
+    auto ts_tablets = ListTabletsOnTS(i);
+    tablets.insert(ts_tablets.begin(), ts_tablets.end());
+  }
+  return vector<string>(tablets.begin(), tablets.end());
+}
+
+vector<string> MiniClusterFsInspector::ListTabletsOnTS(int ts_idx) {
+  string meta_dir = JoinPathSegments(cluster_->WalRootForTS(ts_idx),
+                                     FsManager::kTabletMetadataDirName);
+  vector<string> tablets;
+  CHECK_OK(ListFilesInDir(env_, meta_dir, &tablets));
+  return tablets;
+}
+
+vector<string> MiniClusterFsInspector::ListTabletsWithDataOnTS(int ts_idx) {
+  vector<string> tablets;
+  CHECK_OK(ListFilesInDir(env_, WalDirForTS(ts_idx), &tablets));
+  return tablets;
+}
+
+int MiniClusterFsInspector::CountFilesInWALDirForTS(
+    int ts_idx,
+    const string& tablet_id,
+    StringPiece pattern) {
+  string tablet_wal_dir = JoinPathSegments(WalDirForTS(ts_idx), tablet_id);
+  if (!env_->FileExists(tablet_wal_dir)) {
+    return 0;
+  }
+  return CountFilesInDir(tablet_wal_dir, pattern);
+}
+
+bool MiniClusterFsInspector::DoesConsensusMetaExistForTabletOnTS(int ts_idx,
+                                                                 const string& tablet_id) {
+  ConsensusMetadataPB cmeta_pb;
+  Status s = ReadConsensusMetadataOnTS(ts_idx, tablet_id, &cmeta_pb);
+  return s.ok();
+}
+
+int MiniClusterFsInspector::CountReplicasInMetadataDirs() {
+  // Rather than using FsManager's functionality for listing blocks, we just manually
+  // list the contents of the metadata directory. This is because we're using an
+  // external minicluster, and initializing a new FsManager to point at the running
+  // tablet servers isn't easy.
+  int count = 0;
+  for (int i = 0; i < cluster_->num_tablet_servers(); i++) {
+    count += CountFilesInDir(JoinPathSegments(cluster_->WalRootForTS(i),
+                                              FsManager::kTabletMetadataDirName));
+  }
+  return count;
+}
+
+Status MiniClusterFsInspector::CheckNoDataOnTS(int ts_idx) {
+  const string& wal_root = cluster_->WalRootForTS(ts_idx);
+  if (CountFilesInDir(JoinPathSegments(wal_root, FsManager::kTabletMetadataDirName)) > 0) {
+    return Status::IllegalState("tablet metadata blocks still exist", wal_root);
+  }
+  if (CountWALFilesOnTS(ts_idx) > 0) {
+    return Status::IllegalState("wals still exist", wal_root);
+  }
+  if (CountFilesInDir(JoinPathSegments(wal_root, FsManager::kConsensusMetadataDirName)) > 0) {
+    return Status::IllegalState("consensus metadata still exists", wal_root);
+  }
+  return Status::OK();;
+}
+
+Status MiniClusterFsInspector::CheckNoData() {
+  for (int i = 0; i < cluster_->num_tablet_servers(); i++) {
+    RETURN_NOT_OK(CheckNoDataOnTS(i));
+  }
+  return Status::OK();;
+}
+
+string MiniClusterFsInspector::GetTabletSuperBlockPathOnTS(int ts_idx,
+                                                           const string& tablet_id) const {
+  string meta_dir = JoinPathSegments(cluster_->WalRootForTS(ts_idx),
+                                     FsManager::kTabletMetadataDirName);
+  return JoinPathSegments(meta_dir, tablet_id);
+}
+
+Status MiniClusterFsInspector::ReadTabletSuperBlockOnTS(int ts_idx,
+                                                        const string& tablet_id,
+                                                        TabletSuperBlockPB* sb) {
+  const auto& sb_path = GetTabletSuperBlockPathOnTS(ts_idx, tablet_id);
+  return pb_util::ReadPBContainerFromPath(env_, sb_path, sb);
+}
+
+int64_t MiniClusterFsInspector::GetTabletSuperBlockMTimeOrDie(int ts_idx,
+                                                              const string& tablet_id) {
+  int64_t timestamp;
+  CHECK_OK(env_->GetFileModifiedTime(
+      GetTabletSuperBlockPathOnTS(ts_idx, tablet_id), &timestamp));
+  return timestamp;
+}
+
+string MiniClusterFsInspector::GetConsensusMetadataPathOnTS(int ts_idx,
+                                                            const string& tablet_id) const {
+  string wal_root = cluster_->WalRootForTS(ts_idx);
+  string cmeta_dir = JoinPathSegments(wal_root, FsManager::kConsensusMetadataDirName);
+  return JoinPathSegments(cmeta_dir, tablet_id);
+}
+
+Status MiniClusterFsInspector::ReadConsensusMetadataOnTS(int ts_idx,
+                                                         const string& tablet_id,
+                                                         ConsensusMetadataPB* cmeta_pb) {
+  auto cmeta_path = GetConsensusMetadataPathOnTS(ts_idx, tablet_id);
+  if (!env_->FileExists(cmeta_path)) {
+    return Status::NotFound("Consensus metadata file not found", cmeta_path);
+  }
+  return pb_util::ReadPBContainerFromPath(env_, cmeta_path, cmeta_pb);
+}
+
+Status MiniClusterFsInspector::WriteConsensusMetadataOnTS(
+    int ts_idx,
+    const string& tablet_id,
+    const ConsensusMetadataPB& cmeta_pb) {
+  auto cmeta_path = GetConsensusMetadataPathOnTS(ts_idx, tablet_id);
+  return pb_util::WritePBContainerToPath(env_, cmeta_path, cmeta_pb,
+                                         pb_util::OVERWRITE, pb_util::NO_SYNC);
+}
+
+
+Status MiniClusterFsInspector::CheckTabletDataStateOnTS(
+    int ts_idx,
+    const string& tablet_id,
+    const vector<TabletDataState>& allowed_states) {
+  TabletSuperBlockPB sb;
+  RETURN_NOT_OK(ReadTabletSuperBlockOnTS(ts_idx, tablet_id, &sb));
+  if (std::find(allowed_states.begin(), allowed_states.end(), sb.tablet_data_state()) !=
+      allowed_states.end()) {
+    return Status::OK();
+  }
+
+  vector<string> state_names;
+  for (auto state : allowed_states) {
+    state_names.push_back(TabletDataState_Name(state));
+  }
+  string expected_str = JoinStrings(state_names, ",");
+  if (state_names.size() > 1) {
+    expected_str = "one of: " + expected_str;
+  }
+
+  return Status::IllegalState(Substitute("State $0 unexpected, expected $1",
+                                         TabletDataState_Name(sb.tablet_data_state()),
+                                         expected_str));
+}
+
+Status MiniClusterFsInspector::WaitForNoData(const MonoDelta& timeout) {
+  MonoTime deadline = MonoTime::Now() + timeout;
+  Status s;
+  while (true) {
+    s = CheckNoData();
+    if (s.ok()) return Status::OK();
+    if (deadline < MonoTime::Now()) {
+      break;
+    }
+    SleepFor(MonoDelta::FromMilliseconds(10));
+  }
+  return Status::TimedOut("Timed out waiting for no data", s.ToString());
+}
+
+Status MiniClusterFsInspector::WaitForNoDataOnTS(int ts_idx, const MonoDelta& timeout) {
+  MonoTime deadline = MonoTime::Now() + timeout;
+  Status s;
+  while (true) {
+    s = CheckNoDataOnTS(ts_idx);
+    if (s.ok()) return Status::OK();
+    if (deadline < MonoTime::Now()) {
+      break;
+    }
+    SleepFor(MonoDelta::FromMilliseconds(10));
+  }
+  return Status::TimedOut("Timed out waiting for no data", s.ToString());
+}
+
+Status MiniClusterFsInspector::WaitForMinFilesInTabletWalDirOnTS(int ts_idx,
+                                                                 const string& tablet_id,
+                                                                 int count,
+                                                                 const MonoDelta& timeout) {
+  int seen = 0;
+  MonoTime deadline = MonoTime::Now() + timeout;
+  while (true) {
+    seen = CountFilesInWALDirForTS(ts_idx, tablet_id);
+    if (seen >= count) return Status::OK();
+    if (deadline < MonoTime::Now()) {
+      break;
+    }
+    SleepFor(MonoDelta::FromMilliseconds(10));
+  }
+  return Status::TimedOut(Substitute("Timed out waiting for number of WAL segments on tablet $0 "
+                                     "on TS $1 to be $2. Found $3",
+                                     tablet_id, ts_idx, count, seen));
+}
+
+Status MiniClusterFsInspector::WaitForReplicaCount(int expected, const MonoDelta& timeout) {
+  const MonoTime deadline = MonoTime::Now() + timeout;
+  int found;
+  while (true) {
+    found = CountReplicasInMetadataDirs();
+    if (found == expected) {
+      return Status::OK();
+    }
+    if (MonoTime::Now() > deadline) {
+      break;
+    }
+    SleepFor(MonoDelta::FromMilliseconds(10));
+  }
+  return Status::TimedOut(
+      Substitute("Timed out waiting for a total replica count of $0. "
+                 "Found $1 replicas", expected, found));
+}
+
+Status MiniClusterFsInspector::WaitForTabletDataStateOnTS(
+    int ts_idx,
+    const string& tablet_id,
+    const vector<TabletDataState>& expected_states,
+    const MonoDelta& timeout) {
+  MonoTime start = MonoTime::Now();
+  MonoTime deadline = start + timeout;
+  Status s;
+  while (true) {
+    s = CheckTabletDataStateOnTS(ts_idx, tablet_id, expected_states);
+    if (s.ok()) return Status::OK();
+    if (MonoTime::Now() > deadline) break;
+    SleepFor(MonoDelta::FromMilliseconds(5));
+  }
+  return Status::TimedOut(Substitute("Timed out after $0 waiting for correct tablet state: $1",
+                                     (MonoTime::Now() - start).ToString(),
+                                     s.ToString()));
+}
+
+Status MiniClusterFsInspector::WaitForFilePatternInTabletWalDirOnTs(
+    int ts_idx, const string& tablet_id,
+    const vector<string>& substrings_required,
+    const vector<string>& substrings_disallowed,
+    const MonoDelta& timeout) {
+  Status s;
+  MonoTime deadline = MonoTime::Now() + timeout;
+
+  string tablet_wal_dir = JoinPathSegments(WalDirForTS(ts_idx), tablet_id);
+
+  string error_msg;
+  vector<string> entries;
+  while (true) {
+    Status s = ListFilesInDir(env_, tablet_wal_dir, &entries);
+    std::sort(entries.begin(), entries.end());
+
+    error_msg = "";
+    bool any_missing_required = false;
+    for (const string& required_filter : substrings_required) {
+      bool filter_matched = false;
+      for (const string& entry : entries) {
+        if (entry.find(required_filter) != string::npos) {
+          filter_matched = true;
+          break;
+        }
+      }
+      if (!filter_matched) {
+        any_missing_required = true;
+        error_msg += "missing from substrings_required: " + required_filter + "; ";
+        break;
+      }
+    }
+
+    bool any_present_disallowed = false;
+    for (const string& entry : entries) {
+      if (any_present_disallowed) break;
+      for (const string& disallowed_filter : substrings_disallowed) {
+        if (entry.find(disallowed_filter) != string::npos) {
+          any_present_disallowed = true;
+          error_msg += Substitute("present from substrings_disallowed: $0 ($1)",
+                                  entry, disallowed_filter);
+          break;
+        }
+      }
+    }
+
+    if (!any_missing_required && !any_present_disallowed) {
+      return Status::OK();
+    }
+    if (MonoTime::Now() > deadline) {
+      break;
+    }
+    SleepFor(MonoDelta::FromMilliseconds(10));
+  }
+
+  return Status::TimedOut(Substitute("Timed out waiting for file pattern on "
+                                     "tablet $0 on TS $1 in directory $2",
+                                     tablet_id, ts_idx, tablet_wal_dir),
+                          error_msg + "entries: " + JoinStrings(entries, ", "));
+}
+
+} // namespace itest
+} // namespace kudu
+

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/mini_cluster_fs_inspector.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/mini_cluster_fs_inspector.h b/src/kudu/integration-tests/mini_cluster_fs_inspector.h
new file mode 100644
index 0000000..ba41220
--- /dev/null
+++ b/src/kudu/integration-tests/mini_cluster_fs_inspector.h
@@ -0,0 +1,140 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+#pragma once
+
+#include <cstdint>
+#include <string>
+#include <vector>
+
+#include "kudu/gutil/strings/stringpiece.h"
+#include "kudu/tablet/metadata.pb.h"
+#include "kudu/util/monotime.h"
+
+namespace kudu {
+
+class Env;
+class Status;
+
+namespace cluster {
+class MiniCluster;
+} // namespace cluster
+
+namespace consensus {
+class ConsensusMetadataPB;
+} // namespace consensus
+
+namespace itest {
+
+// Utility class that digs around in a tablet server's FS layout and provides
+// methods useful for integration testing. This class must outlive the Env and
+// MiniCluster objects that are passed into it.
+class MiniClusterFsInspector {
+ public:
+  explicit MiniClusterFsInspector(cluster::MiniCluster* cluster);
+
+  ~MiniClusterFsInspector() {}
+
+  // Returns the WALs FS subdirectory created for TS 'ts_idx'.
+  std::string WalDirForTS(int ts_idx) const;
+
+  // If provided, files are filtered by the glob-style pattern 'pattern'.
+  int CountFilesInDir(const std::string& path, StringPiece pattern = StringPiece());
+
+  // List all of the tablets with tablet metadata in the cluster.
+  std::vector<std::string> ListTablets();
+
+  // List all of the tablets with tablet metadata on the given tablet server
+  // 'ts_idx'.  This may include tablets that are tombstoned and not running.
+  std::vector<std::string> ListTabletsOnTS(int ts_idx);
+
+  // List the tablet IDs on the given tablet which actually have data (as
+  // evidenced by their having a WAL). This excludes those that are tombstoned.
+  std::vector<std::string> ListTabletsWithDataOnTS(int ts_idx);
+
+  // Returns the number of files in the WAL directory for 'tablet_id' on TS
+  // 'ts_idx'. If provided, files are filtered by the glob string 'pattern'.
+  int CountFilesInWALDirForTS(int ts_idx,
+                              const std::string& tablet_id,
+                              StringPiece pattern = StringPiece());
+
+  bool DoesConsensusMetaExistForTabletOnTS(int ts_idx, const std::string& tablet_id);
+
+  int CountReplicasInMetadataDirs();
+  Status CheckNoDataOnTS(int ts_idx);
+  Status CheckNoData();
+
+  Status ReadTabletSuperBlockOnTS(int ts_idx, const std::string& tablet_id,
+                                  tablet::TabletSuperBlockPB* sb);
+
+  // Get the modification time (in micros) of the tablet superblock for the
+  // given tablet server ts_idx and tablet ID.
+  int64_t GetTabletSuperBlockMTimeOrDie(int ts_idx, const std::string& tablet_id);
+
+  Status ReadConsensusMetadataOnTS(int ts_idx, const std::string& tablet_id,
+                                   consensus::ConsensusMetadataPB* cmeta_pb);
+  Status WriteConsensusMetadataOnTS(int ts_idx,
+                                    const std::string& tablet_id,
+                                    const consensus::ConsensusMetadataPB& cmeta_pb);
+
+  Status CheckTabletDataStateOnTS(int ts_idx,
+                                  const std::string& tablet_id,
+                                  const std::vector<tablet::TabletDataState>& allowed_states);
+
+  Status WaitForNoData(const MonoDelta& timeout = MonoDelta::FromSeconds(30));
+  Status WaitForNoDataOnTS(int ts_idx, const MonoDelta& timeout = MonoDelta::FromSeconds(30));
+  Status WaitForMinFilesInTabletWalDirOnTS(int ts_idx,
+                                           const std::string& tablet_id,
+                                           int count,
+                                           const MonoDelta& timeout = MonoDelta::FromSeconds(60));
+  Status WaitForReplicaCount(int expected, const MonoDelta& timeout = MonoDelta::FromSeconds(30));
+  Status WaitForTabletDataStateOnTS(int ts_idx,
+                                    const std::string& tablet_id,
+                                    const std::vector<tablet::TabletDataState>& expected_states,
+                                    const MonoDelta& timeout = MonoDelta::FromSeconds(30));
+
+  // Loop and check for certain filenames in the WAL directory of the specified
+  // tablet. This function returns OK if we reach a state where:
+  // * For each string in 'substrings_required', we find *at least one file*
+  //   whose name contains that string, and:
+  // * For each string in 'substrings_disallowed', we find *no files* whose name
+  //   contains that string, even if the file also matches a string in the
+  //   'substrings_required'.
+  Status WaitForFilePatternInTabletWalDirOnTs(
+      int ts_idx,
+      const std::string& tablet_id,
+      const std::vector<std::string>& substrings_required,
+      const std::vector<std::string>& substrings_disallowed,
+      const MonoDelta& timeout = MonoDelta::FromSeconds(30));
+
+ private:
+  const cluster::MiniCluster* const cluster_;
+  Env* const env_;
+
+  // Return the number of files in WAL directories on the given tablet server.
+  // This includes log ts_idx files (not just segments).
+  int CountWALFilesOnTS(int ts_idx);
+
+  std::string GetConsensusMetadataPathOnTS(int ts_idx,
+                                           const std::string& tablet_id) const;
+
+  std::string GetTabletSuperBlockPathOnTS(int ts_idx,
+                                          const std::string& tablet_id) const;
+
+};
+
+} // namespace itest
+} // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/raft_config_change-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/raft_config_change-itest.cc b/src/kudu/integration-tests/raft_config_change-itest.cc
index d7b1301..c17f5c6 100644
--- a/src/kudu/integration-tests/raft_config_change-itest.cc
+++ b/src/kudu/integration-tests/raft_config_change-itest.cc
@@ -37,7 +37,7 @@
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/external_mini_cluster-itest-base.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/master/master.pb.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/raft_consensus-itest-base.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/raft_consensus-itest-base.cc b/src/kudu/integration-tests/raft_consensus-itest-base.cc
index 5b5686d..f10b652 100644
--- a/src/kudu/integration-tests/raft_consensus-itest-base.cc
+++ b/src/kudu/integration-tests/raft_consensus-itest-base.cc
@@ -41,7 +41,7 @@
 #include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/stringprintf.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/integration-tests/ts_itest-base.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/raft_consensus-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/raft_consensus-itest.cc b/src/kudu/integration-tests/raft_consensus-itest.cc
index 7bd4f16..41193a8 100644
--- a/src/kudu/integration-tests/raft_consensus-itest.cc
+++ b/src/kudu/integration-tests/raft_consensus-itest.cc
@@ -54,8 +54,8 @@
 #include "kudu/gutil/strings/util.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/cluster_verifier.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/log_verifier.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/raft_consensus-itest-base.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/master/master.pb.h"
@@ -1064,10 +1064,11 @@ TEST_F(RaftConsensusITest, TestLMPMismatchOnRestartedReplica) {
 
   // The COMMIT messages end up in the WAL asynchronously, so loop reading the
   // tablet server's WAL until it shows up.
+  int replica_idx = cluster_->tablet_server_index_by_uuid(replica_ets->uuid());
   ASSERT_EVENTUALLY([&]() {
       LogVerifier lv(cluster_.get());
       OpId commit;
-      ASSERT_OK(lv.ScanForHighestCommittedOpIdInLog(replica_ets, tablet_id_, &commit));
+      ASSERT_OK(lv.ScanForHighestCommittedOpIdInLog(replica_idx, tablet_id_, &commit));
       ASSERT_EQ("2.2", OpIdToString(commit));
     });
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/raft_consensus_election-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/raft_consensus_election-itest.cc b/src/kudu/integration-tests/raft_consensus_election-itest.cc
index cb8803b..00078bf 100644
--- a/src/kudu/integration-tests/raft_consensus_election-itest.cc
+++ b/src/kudu/integration-tests/raft_consensus_election-itest.cc
@@ -32,7 +32,7 @@
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/cluster_verifier.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/raft_consensus-itest-base.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc b/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc
index 6726109..8908029 100644
--- a/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc
+++ b/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc
@@ -41,7 +41,7 @@
 #include "kudu/gutil/strings/util.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/cluster_verifier.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/raft_consensus-itest-base.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/master/master.pb.h"

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/tablet_copy-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/tablet_copy-itest.cc b/src/kudu/integration-tests/tablet_copy-itest.cc
index 3be6d0f..4b2a9cf 100644
--- a/src/kudu/integration-tests/tablet_copy-itest.cc
+++ b/src/kudu/integration-tests/tablet_copy-itest.cc
@@ -55,7 +55,7 @@
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/cluster_verifier.h"
 #include "kudu/integration-tests/external_mini_cluster-itest-base.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/master/master.pb.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/tablet_copy_client_session-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/tablet_copy_client_session-itest.cc b/src/kudu/integration-tests/tablet_copy_client_session-itest.cc
index 5279d0b..78a0e77 100644
--- a/src/kudu/integration-tests/tablet_copy_client_session-itest.cc
+++ b/src/kudu/integration-tests/tablet_copy_client_session-itest.cc
@@ -35,7 +35,7 @@
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/external_mini_cluster-itest-base.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
 #include "kudu/tablet/metadata.pb.h"

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/tablet_replacement-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/tablet_replacement-itest.cc b/src/kudu/integration-tests/tablet_replacement-itest.cc
index 7ca9c3b..5aeaed5 100644
--- a/src/kudu/integration-tests/tablet_replacement-itest.cc
+++ b/src/kudu/integration-tests/tablet_replacement-itest.cc
@@ -39,7 +39,7 @@
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/cluster_verifier.h"
 #include "kudu/integration-tests/external_mini_cluster-itest-base.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
 #include "kudu/rpc/rpc_controller.h"

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/tombstoned_voting-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/tombstoned_voting-itest.cc b/src/kudu/integration-tests/tombstoned_voting-itest.cc
index 951f28f..9c28cd9 100644
--- a/src/kudu/integration-tests/tombstoned_voting-itest.cc
+++ b/src/kudu/integration-tests/tombstoned_voting-itest.cc
@@ -29,7 +29,7 @@
 #include "kudu/gutil/map-util.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/external_mini_cluster-itest-base.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/master/master.pb.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/tombstoned_voting-stress-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/tombstoned_voting-stress-test.cc b/src/kudu/integration-tests/tombstoned_voting-stress-test.cc
index ba71f8e..e300630 100644
--- a/src/kudu/integration-tests/tombstoned_voting-stress-test.cc
+++ b/src/kudu/integration-tests/tombstoned_voting-stress-test.cc
@@ -36,7 +36,7 @@
 #include "kudu/gutil/macros.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/external_mini_cluster-itest-base.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
 #include "kudu/tablet/metadata.pb.h"

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/ts_itest-base.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/ts_itest-base.cc b/src/kudu/integration-tests/ts_itest-base.cc
index 26f4010..6ec2000 100644
--- a/src/kudu/integration-tests/ts_itest-base.cc
+++ b/src/kudu/integration-tests/ts_itest-base.cc
@@ -45,7 +45,7 @@
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/cluster_verifier.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/master/master.pb.h"
 #include "kudu/master/master.proxy.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
@@ -144,7 +144,7 @@ void TabletServerIntegrationTestBase::CreateCluster(
 
   cluster_.reset(new cluster::ExternalMiniCluster(std::move(opts)));
   ASSERT_OK(cluster_->Start());
-  inspect_.reset(new itest::ExternalMiniClusterFsInspector(cluster_.get()));
+  inspect_.reset(new itest::MiniClusterFsInspector(cluster_.get()));
   CreateTSProxies();
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/ts_itest-base.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/ts_itest-base.h b/src/kudu/integration-tests/ts_itest-base.h
index 19ee9c5..ce41df4 100644
--- a/src/kudu/integration-tests/ts_itest-base.h
+++ b/src/kudu/integration-tests/ts_itest-base.h
@@ -25,7 +25,7 @@
 #include "kudu/client/shared_ptr.h"
 #include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
 #include "kudu/tserver/tablet_server-test-base.h"
 #include "kudu/util/random.h"
@@ -151,7 +151,7 @@ class TabletServerIntegrationTestBase : public TabletServerTestBase {
 
  protected:
   gscoped_ptr<cluster::ExternalMiniCluster> cluster_;
-  gscoped_ptr<itest::ExternalMiniClusterFsInspector> inspect_;
+  gscoped_ptr<itest::MiniClusterFsInspector> inspect_;
 
   // Maps server uuid to TServerDetails
   itest::TabletServerMap tablet_servers_;

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/ts_recovery-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/ts_recovery-itest.cc b/src/kudu/integration-tests/ts_recovery-itest.cc
index c396bf8..4b78e15 100644
--- a/src/kudu/integration-tests/ts_recovery-itest.cc
+++ b/src/kudu/integration-tests/ts_recovery-itest.cc
@@ -58,7 +58,7 @@
 #include "kudu/integration-tests/cluster_itest_util.h"
 #include "kudu/integration-tests/cluster_verifier.h"
 #include "kudu/integration-tests/external_mini_cluster-itest-base.h"
-#include "kudu/integration-tests/external_mini_cluster_fs_inspector.h"
+#include "kudu/integration-tests/mini_cluster_fs_inspector.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
 #include "kudu/tablet/metadata.pb.h"
@@ -101,7 +101,7 @@ using consensus::ConsensusMetadataManager;
 using consensus::OpId;
 using consensus::RECEIVED_OPID;
 using fs::BlockManager;
-using itest::ExternalMiniClusterFsInspector;
+using itest::MiniClusterFsInspector;
 using log::AppendNoOpsToLogSync;
 using log::Log;
 using log::LogOptions;
@@ -164,8 +164,8 @@ TEST_P(TsRecoveryITest, TestNoBlockIDReuseIfMissingBlocks) {
   };
 
   unique_ptr<TestWorkload> write_workload(StartSingleTabletWorkload("foo"));
-  unique_ptr<ExternalMiniClusterFsInspector> inspect(
-      new ExternalMiniClusterFsInspector(cluster_.get()));
+  unique_ptr<MiniClusterFsInspector> inspect(
+      new MiniClusterFsInspector(cluster_.get()));
   vector<string> tablets = inspect->ListTabletsOnTS(0);
   ASSERT_EQ(1, tablets.size());
   const string tablet_id = tablets[0];

http://git-wip-us.apache.org/repos/asf/kudu/blob/2165ce57/src/kudu/integration-tests/ts_tablet_manager-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/ts_tablet_manager-itest.cc b/src/kudu/integration-tests/ts_tablet_manager-itest.cc
index e56d016..8efebe9 100644
--- a/src/kudu/integration-tests/ts_tablet_manager-itest.cc
+++ b/src/kudu/integration-tests/ts_tablet_manager-itest.cc
@@ -39,6 +39,7 @@
 #include "kudu/gutil/stl_util.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
+#include "kudu/integration-tests/cluster_verifier.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/master/master.pb.h"
 #include "kudu/master/master.proxy.h"
@@ -86,6 +87,7 @@ using kudu::rpc::Messenger;
 using kudu::rpc::MessengerBuilder;
 using kudu::tablet::TabletReplica;
 using kudu::tserver::MiniTabletServer;
+using kudu::ClusterVerifier;
 using std::map;
 using std::shared_ptr;
 using std::string;
@@ -213,28 +215,11 @@ TEST_P(FailedTabletsAreReplacedITest, OneReplica) {
     ASSERT_EQ(1, tablet_ids.size());
     tablet_id = tablet_ids[0];
   });
+  work.StopAndJoin();
 
   // Wait until all the replicas are running before failing one arbitrarily.
-  const auto wait_until_running = [&]() {
-    AssertEventually([&]{
-      auto num_replicas_running = 0;
-      for (auto idx = 0; idx < cluster_->num_tablet_servers(); ++idx) {
-        MiniTabletServer* ts = cluster_->mini_tablet_server(idx);
-        scoped_refptr<TabletReplica> replica;
-        Status s = ts->server()->tablet_manager()->GetTabletReplica(tablet_id, &replica);
-        if (s.IsNotFound()) {
-          continue;
-        }
-        ASSERT_OK(s);
-        if (tablet::RUNNING == replica->state()) {
-          ++num_replicas_running;
-        }
-      }
-      ASSERT_EQ(kNumReplicas, num_replicas_running);
-    }, MonoDelta::FromSeconds(60));
-    NO_PENDING_FATALS();
-  };
-  NO_FATALS(wait_until_running());
+  ClusterVerifier v(cluster_.get());
+  NO_FATALS(v.CheckCluster());
 
   {
     // Inject an error into one of replicas. Shutting it down will leave it in
@@ -251,8 +236,7 @@ TEST_P(FailedTabletsAreReplacedITest, OneReplica) {
   }
 
   // Ensure the tablet eventually is replicated.
-  NO_FATALS(wait_until_running());
-  work.StopAndJoin();
+  NO_FATALS(v.CheckCluster());
 }
 INSTANTIATE_TEST_CASE_P(,
                         FailedTabletsAreReplacedITest,


[3/4] kudu git commit: gutil: properly hook up ANNOTATE_HAPPENS_BEFORE/AFTER

Posted by al...@apache.org.
gutil: properly hook up ANNOTATE_HAPPENS_BEFORE/AFTER

This updates dynamic_annotations to hook up ANNOTATE_HAPPENS_BEFORE and
ANNOTATE_HAPPENS_AFTER to the appropriate annotations in the TSAN
runtime library. According to [1] (committed in 2011) the functions have
been implemented in TSAN for quite some time, and it was an error that
we weren't using them.

The previous implementation of the function called some
condition-variable annotations in the TSAN runtime library instead.
Those functions actually turn out to be implemented as no-ops. So, I
looked for existing call sites to ANNOTATE_HAPPENS_BEFORE and AFTER and
removed them. This cleaned up atomic_refcount pretty substantially.
Again I found a matching change[2] in Chromium, from which this code is
derived.

[1] https://codereview.chromium.org/6982022
[2] https://codereview.chromium.org/580813002

Change-Id: Ida27aff6b9771c0009fba5e31ec7a0c7c53caa59
Reviewed-on: http://gerrit.cloudera.org:8080/9325
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/a964b0e3
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/a964b0e3
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/a964b0e3

Branch: refs/heads/master
Commit: a964b0e36c7bb16bdbbd7f8aa98c67e1da5a99eb
Parents: 2165ce5
Author: Todd Lipcon <to...@apache.org>
Authored: Wed Feb 14 09:28:03 2018 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Wed Feb 21 01:48:58 2018 +0000

----------------------------------------------------------------------
 src/kudu/gutil/atomic_refcount.h     | 58 +++++--------------------------
 src/kudu/gutil/dynamic_annotations.c |  4 +++
 src/kudu/gutil/dynamic_annotations.h | 12 +++++--
 src/kudu/gutil/once.cc               |  3 +-
 src/kudu/gutil/once.h                |  4 ---
 5 files changed, 22 insertions(+), 59 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/a964b0e3/src/kudu/gutil/atomic_refcount.h
----------------------------------------------------------------------
diff --git a/src/kudu/gutil/atomic_refcount.h b/src/kudu/gutil/atomic_refcount.h
index 9c80921..6500d2d 100644
--- a/src/kudu/gutil/atomic_refcount.h
+++ b/src/kudu/gutil/atomic_refcount.h
@@ -32,9 +32,6 @@
 // initialization you should use the word only via the routines below; the
 // "volatile" in the signatures below is for backwards compatibility.
 //
-// The implementation includes annotations to avoid some false alarms 
-// when using Helgrind (the data race detector).
-//
 // If you need to do something very different from this, use a Mutex.
 
 #include <glog/logging.h>
@@ -42,7 +39,6 @@
 #include "kudu/gutil/atomicops.h"
 #include "kudu/gutil/integral_types.h"
 #include "kudu/gutil/logging-inl.h"
-#include "kudu/gutil/dynamic_annotations.h"
 
 namespace base {
 
@@ -65,11 +61,7 @@ inline void RefCountIncN(volatile Atomic32 *ptr, Atomic32 increment) {
 // became zero will be visible to a thread that has just made the count zero.
 inline bool RefCountDecN(volatile Atomic32 *ptr, Atomic32 decrement) {
   DCHECK_GT(decrement, 0);
-  ANNOTATE_HAPPENS_BEFORE(ptr);
   bool res = base::subtle::Barrier_AtomicIncrement(ptr, -decrement) != 0;
-  if (!res) {
-    ANNOTATE_HAPPENS_AFTER(ptr);
-  }
   return res;
 }
 
@@ -94,22 +86,14 @@ inline bool RefCountDec(volatile Atomic32 *ptr) {
 // to act on the object, knowing that it has exclusive access to the
 // object.
 inline bool RefCountIsOne(const volatile Atomic32 *ptr) {
-  bool res = base::subtle::Acquire_Load(ptr) == 1;
-  if (res) {
-    ANNOTATE_HAPPENS_AFTER(ptr);
-  }
-  return res;
+  return base::subtle::Acquire_Load(ptr) == 1;
 }
 
 // Return whether the reference count is zero.  With conventional object
 // referencing counting, the object will be destroyed, so the reference count
 // should never be zero.  Hence this is generally used for a debug check.
 inline bool RefCountIsZero(const volatile Atomic32 *ptr) {
-  bool res = (subtle::Acquire_Load(ptr) == 0);
-  if (res) {
-    ANNOTATE_HAPPENS_AFTER(ptr);
-  }
-  return res;
+  return subtle::Acquire_Load(ptr) == 0;
 }
 
 #if BASE_HAS_ATOMIC64
@@ -122,12 +106,7 @@ inline void RefCountIncN(volatile base::subtle::Atomic64 *ptr,
 inline bool RefCountDecN(volatile base::subtle::Atomic64 *ptr,
                          base::subtle::Atomic64 decrement) {
   DCHECK_GT(decrement, 0);
-  ANNOTATE_HAPPENS_BEFORE(ptr);
-  bool res = base::subtle::Barrier_AtomicIncrement(ptr, -decrement) != 0;
-  if (!res) {
-    ANNOTATE_HAPPENS_AFTER(ptr);
-  }
-  return res;
+  return base::subtle::Barrier_AtomicIncrement(ptr, -decrement) != 0;
 }
 inline void RefCountInc(volatile base::subtle::Atomic64 *ptr) {
   base::RefCountIncN(ptr, 1);
@@ -136,18 +115,10 @@ inline bool RefCountDec(volatile base::subtle::Atomic64 *ptr) {
   return base::RefCountDecN(ptr, 1);
 }
 inline bool RefCountIsOne(const volatile base::subtle::Atomic64 *ptr) {
-  bool res = base::subtle::Acquire_Load(ptr) == 1;
-  if (res) {
-    ANNOTATE_HAPPENS_AFTER(ptr);
-  }
-  return res;
+  return base::subtle::Acquire_Load(ptr) == 1;
 }
 inline bool RefCountIsZero(const volatile base::subtle::Atomic64 *ptr) {
-  bool res = (base::subtle::Acquire_Load(ptr) == 0);
-  if (res) {
-    ANNOTATE_HAPPENS_AFTER(ptr);
-  }
-  return res;
+  return base::subtle::Acquire_Load(ptr) == 0;
 }
 #endif
 
@@ -158,13 +129,8 @@ inline void RefCountIncN(volatile AtomicWord *ptr, AtomicWord increment) {
       reinterpret_cast<volatile AtomicWordCastType *>(ptr), increment);
 }
 inline bool RefCountDecN(volatile AtomicWord *ptr, AtomicWord decrement) {
-  ANNOTATE_HAPPENS_BEFORE(ptr);
-  bool res = base::RefCountDecN(
+  return base::RefCountDecN(
       reinterpret_cast<volatile AtomicWordCastType *>(ptr), decrement);
-  if (!res) {
-    ANNOTATE_HAPPENS_AFTER(ptr);
-  }
-  return res;
 }
 inline void RefCountInc(volatile AtomicWord *ptr) {
   base::RefCountIncN(ptr, 1);
@@ -173,20 +139,12 @@ inline bool RefCountDec(volatile AtomicWord *ptr) {
   return base::RefCountDecN(ptr, 1);
 }
 inline bool RefCountIsOne(const volatile AtomicWord *ptr) {
-  bool res = base::subtle::Acquire_Load(
+  return base::subtle::Acquire_Load(
       reinterpret_cast<const volatile AtomicWordCastType *>(ptr)) == 1;
-  if (res) {
-    ANNOTATE_HAPPENS_AFTER(ptr);
-  }
-  return res;
 }
 inline bool RefCountIsZero(const volatile AtomicWord *ptr) {
-  bool res = base::subtle::Acquire_Load(
+  return base::subtle::Acquire_Load(
       reinterpret_cast<const volatile AtomicWordCastType *>(ptr)) == 0;
-  if (res) {
-    ANNOTATE_HAPPENS_AFTER(ptr);
-  }
-  return res;
 }
 #endif
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/a964b0e3/src/kudu/gutil/dynamic_annotations.c
----------------------------------------------------------------------
diff --git a/src/kudu/gutil/dynamic_annotations.c b/src/kudu/gutil/dynamic_annotations.c
index 17a94da..f93b634 100644
--- a/src/kudu/gutil/dynamic_annotations.c
+++ b/src/kudu/gutil/dynamic_annotations.c
@@ -84,6 +84,10 @@ void AnnotateCondVarSignal(const char *file, int line,
                            const volatile void *cv){}
 void AnnotateCondVarSignalAll(const char *file, int line,
                               const volatile void *cv){}
+void AnnotateHappensBefore(const char *file, int line,
+                           const volatile void *obj);
+void AnnotateHappensAfter(const char *file, int line,
+                          const volatile void *obj);
 void AnnotatePublishMemoryRange(const char *file, int line,
                                 const volatile void *address,
                                 long size){}

http://git-wip-us.apache.org/repos/asf/kudu/blob/a964b0e3/src/kudu/gutil/dynamic_annotations.h
----------------------------------------------------------------------
diff --git a/src/kudu/gutil/dynamic_annotations.h b/src/kudu/gutil/dynamic_annotations.h
index 9f838f2..7e03d45 100644
--- a/src/kudu/gutil/dynamic_annotations.h
+++ b/src/kudu/gutil/dynamic_annotations.h
@@ -121,8 +121,10 @@
     AnnotateCondVarSignalAll(__FILE__, __LINE__, cv)
 
   /* Annotations for user-defined synchronization mechanisms. */
-  #define ANNOTATE_HAPPENS_BEFORE(obj) ANNOTATE_CONDVAR_SIGNAL(obj)
-  #define ANNOTATE_HAPPENS_AFTER(obj)  ANNOTATE_CONDVAR_WAIT(obj)
+  #define ANNOTATE_HAPPENS_BEFORE(obj) \
+    AnnotateHappensBefore(__FILE__, __LINE__, obj)
+  #define ANNOTATE_HAPPENS_AFTER(obj) \
+    AnnotateHappensAfter(__FILE__, __LINE__, obj)
 
   /* Report that the bytes in the range [pointer, pointer+size) are about
      to be published safely. The race checker will create a happens-before
@@ -490,9 +492,13 @@ void AnnotateCondVarSignal(const char *file, int line,
                            const volatile void *cv);
 void AnnotateCondVarSignalAll(const char *file, int line,
                               const volatile void *cv);
+void AnnotateHappensBefore(const char *file, int line,
+                           const volatile void *obj);
+void AnnotateHappensAfter(const char *file, int line,
+                          const volatile void *obj);
 void AnnotatePublishMemoryRange(const char *file, int line,
                                 const volatile void *address,
-                                long size);
+                                long size); // NOLINT
 void AnnotateUnpublishMemoryRange(const char *file, int line,
                                   const volatile void *address,
                                   long size);

http://git-wip-us.apache.org/repos/asf/kudu/blob/a964b0e3/src/kudu/gutil/once.cc
----------------------------------------------------------------------
diff --git a/src/kudu/gutil/once.cc b/src/kudu/gutil/once.cc
index 34787c6..4171d0a 100644
--- a/src/kudu/gutil/once.cc
+++ b/src/kudu/gutil/once.cc
@@ -5,7 +5,7 @@
 #include <glog/logging.h>
 
 #include "kudu/gutil/atomicops.h"
-#include "kudu/gutil/dynamic_annotations.h"
+#include "kudu/gutil/macros.h"
 #include "kudu/gutil/integral_types.h"
 #include "kudu/gutil/logging-inl.h"
 #include "kudu/gutil/once.h"
@@ -44,7 +44,6 @@ void GoogleOnceInternalInit(Atomic32 *control, void (*func)(),
     } else {
       (*func_with_arg)(arg);
     }
-    ANNOTATE_HAPPENS_BEFORE(control);
     int32 old_control = base::subtle::NoBarrier_Load(control);
     base::subtle::Release_Store(control, GOOGLE_ONCE_INTERNAL_DONE);
     if (old_control == GOOGLE_ONCE_INTERNAL_WAITER) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/a964b0e3/src/kudu/gutil/once.h
----------------------------------------------------------------------
diff --git a/src/kudu/gutil/once.h b/src/kudu/gutil/once.h
index 460a2ec..640fe02 100644
--- a/src/kudu/gutil/once.h
+++ b/src/kudu/gutil/once.h
@@ -25,7 +25,6 @@
 #define BASE_ONCE_H_
 
 #include "kudu/gutil/atomicops.h"
-#include "kudu/gutil/dynamic_annotations.h"
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/type_traits.h"
@@ -53,7 +52,6 @@ inline void GoogleOnceInit(GoogleOnceType* state, void (*func)()) {
   if (PREDICT_FALSE(s != GOOGLE_ONCE_INTERNAL_DONE)) {
     GoogleOnceInternalInit(&state->state, func, 0, 0);
   }
-  ANNOTATE_HAPPENS_AFTER(&state->state);
 }
 
 // A version of GoogleOnceInit where the function argument takes a pointer
@@ -69,7 +67,6 @@ inline void GoogleOnceInitArg(GoogleOnceType* state,
                            reinterpret_cast<void(*)(void*)>(func_with_arg),
                            const_cast<mutable_T*>(arg));
   }
-  ANNOTATE_HAPPENS_AFTER(&state->state);
 }
 
 // GoogleOnceDynamic is like GoogleOnceType, but is dynamically
@@ -108,7 +105,6 @@ class GoogleOnceDynamic {
                              reinterpret_cast<void (*)(void*)>(func_with_arg),
                              const_cast<mutable_T*>(arg));
     }
-    ANNOTATE_HAPPENS_AFTER(&this->state_);
   }
  private:
   Atomic32 state_;


[4/4] kudu git commit: KUDU-2274 [itest] stress test for replica replacement

Posted by al...@apache.org.
KUDU-2274 [itest] stress test for replica replacement

This is a stress test for the automatic replica replacement in Kudu.

Parameters of the test are configurable via run-time gflags, so it's
possible to run it in a 'standalone' mode, targeting it to be a sort
of an endurance test.

This test reproduces the race described in KUDU-2274: it triggers
DCHECK() assertions added in 194fd8b169f29aafbd78a47709ac51d2e8354a1a
before the relevant fixes for KUDU-2274 were checked in.

Change-Id: I036a882f7e9132a5c26013227c50c0699b59ed6e
Reviewed-on: http://gerrit.cloudera.org:8080/9255
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/a6658539
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/a6658539
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/a6658539

Branch: refs/heads/master
Commit: a66585398e3873a3b8a38f6a3a1becb644979a50
Parents: a964b0e
Author: Alexey Serbin <as...@cloudera.com>
Authored: Tue Feb 13 16:45:32 2018 -0800
Committer: Alexey Serbin <as...@cloudera.com>
Committed: Wed Feb 21 02:14:04 2018 +0000

----------------------------------------------------------------------
 src/kudu/integration-tests/CMakeLists.txt       |   1 +
 src/kudu/integration-tests/cluster_verifier.cc  |   4 +-
 src/kudu/integration-tests/cluster_verifier.h   |   5 +-
 .../raft_consensus_stress-itest.cc              | 299 +++++++++++++++++++
 4 files changed, 305 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/a6658539/src/kudu/integration-tests/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/CMakeLists.txt b/src/kudu/integration-tests/CMakeLists.txt
index af5dae9..c3922eb 100644
--- a/src/kudu/integration-tests/CMakeLists.txt
+++ b/src/kudu/integration-tests/CMakeLists.txt
@@ -91,6 +91,7 @@ ADD_KUDU_TEST(raft_config_change-itest)
 ADD_KUDU_TEST(raft_consensus_election-itest)
 ADD_KUDU_TEST(raft_consensus_failure_detector-imc-itest)
 ADD_KUDU_TEST(raft_consensus_nonvoter-itest)
+ADD_KUDU_TEST(raft_consensus_stress-itest)
 ADD_KUDU_TEST(raft_consensus-itest RUN_SERIAL true)
 ADD_KUDU_TEST(registration-test RESOURCE_LOCK "master-web-port")
 ADD_KUDU_TEST(security-faults-itest)

http://git-wip-us.apache.org/repos/asf/kudu/blob/a6658539/src/kudu/integration-tests/cluster_verifier.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/cluster_verifier.cc b/src/kudu/integration-tests/cluster_verifier.cc
index b903a81..22cba09 100644
--- a/src/kudu/integration-tests/cluster_verifier.cc
+++ b/src/kudu/integration-tests/cluster_verifier.cc
@@ -75,7 +75,7 @@ void ClusterVerifier::CheckCluster() {
   Status s;
   double sleep_time = 0.1;
   while (MonoTime::Now() < deadline) {
-    s = DoKsck();
+    s = RunKsck();
     if (s.ok()) {
       break;
     }
@@ -97,7 +97,7 @@ void ClusterVerifier::CheckCluster() {
   });
 }
 
-Status ClusterVerifier::DoKsck() {
+Status ClusterVerifier::RunKsck() {
   vector<string> hp_strs;
   for (const auto& hp : cluster_->master_rpc_addrs()) {
     hp_strs.emplace_back(hp.ToString());

http://git-wip-us.apache.org/repos/asf/kudu/blob/a6658539/src/kudu/integration-tests/cluster_verifier.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/cluster_verifier.h b/src/kudu/integration-tests/cluster_verifier.h
index 56eaffa..533a707 100644
--- a/src/kudu/integration-tests/cluster_verifier.h
+++ b/src/kudu/integration-tests/cluster_verifier.h
@@ -76,9 +76,10 @@ class ClusterVerifier {
                                 int expected_row_count,
                                 const MonoDelta& timeout);
 
- private:
-  Status DoKsck();
+  // Run the ksck utility against the cluster.
+  Status RunKsck();
 
+ private:
   // Implementation for CheckRowCount -- returns a Status instead of firing
   // gtest assertions.
   Status DoCheckRowCount(const std::string& table_name,

http://git-wip-us.apache.org/repos/asf/kudu/blob/a6658539/src/kudu/integration-tests/raft_consensus_stress-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/raft_consensus_stress-itest.cc b/src/kudu/integration-tests/raft_consensus_stress-itest.cc
new file mode 100644
index 0000000..7857930
--- /dev/null
+++ b/src/kudu/integration-tests/raft_consensus_stress-itest.cc
@@ -0,0 +1,299 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <atomic>
+#include <cstdint>
+#include <cstdlib>
+#include <ostream>
+#include <string>
+#include <thread>
+#include <vector>
+
+#include <gflags/gflags.h>
+#include <gflags/gflags_declare.h>
+#include <glog/logging.h>
+#include <gtest/gtest.h>
+
+#include "kudu/gutil/gscoped_ptr.h"
+#include "kudu/gutil/strings/substitute.h"
+#include "kudu/integration-tests/cluster_itest_util.h"
+#include "kudu/integration-tests/cluster_verifier.h"
+#include "kudu/integration-tests/raft_consensus-itest-base.h"
+#include "kudu/integration-tests/test_workload.h"
+#include "kudu/mini-cluster/external_mini_cluster.h"
+#include "kudu/util/monotime.h"
+#include "kudu/util/scoped_cleanup.h"
+#include "kudu/util/status.h"
+#include "kudu/util/test_macros.h"
+#include "kudu/util/test_util.h"
+
+// Binaries built in the address- and thread-sanitizer build configurations
+// run much slower than binaries built in debug and release configurations.
+// The paramters are adjusted accordingly to avoid test flakiness.
+#if defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER)
+constexpr int kBuildCfgFactor = 2;
+#else
+constexpr int kBuildCfgFactor = 1;
+#endif
+
+DEFINE_bool(test_raft_prepare_replacement_before_eviction, true,
+            "When enabled, failed replicas will only be evicted after a "
+            "replacement has been prepared for them.");
+DEFINE_double(test_tablet_copy_early_session_timeout_prob,
+              0.25 / kBuildCfgFactor,
+              "The probability that a tablet copy session will time out early, "
+              "resulting in a tablet copy failure.");
+DEFINE_int32(test_follower_unavailable_considered_failed_sec,
+             1 * kBuildCfgFactor,
+             "Seconds that a leader tablet replica is unable to successfully "
+             "heartbeat to a follower after which the follower is considered "
+             "to be failed and evicted from the config.");
+DEFINE_int32(test_heartbeat_interval_ms,
+             250 * kBuildCfgFactor,
+             "Interval at which the TS heartbeats to the master.");
+DEFINE_int32(test_max_ksck_failures, 30,
+             "Maximum number of ksck failures in a row to tolerate before "
+             "considering the test as failed.");
+// GLOG_FATAL:    3
+// GLOG_ERROR:    2
+// GLOG_WARNING:  1
+// GLOG_INFO:     0
+DEFINE_int32(test_minloglevel, google::GLOG_ERROR,
+             "Logging level for masters and tablet servers under test.");
+DEFINE_int32(test_num_iterations,
+             10 / kBuildCfgFactor,
+             "Number of iterations, repeating the test scenario.");
+DEFINE_int32(test_num_replicas_per_server,
+             20 / kBuildCfgFactor,
+             "Number of tablets per server to create.");
+DEFINE_int32(test_raft_heartbeat_interval_ms,
+             100 * kBuildCfgFactor,
+             "The Raft heartbeat interval for tablet servers under the test.");
+DEFINE_int32(test_replication_factor, 3,
+             "The replication factor of the test table.");
+
+DECLARE_int32(num_replicas);
+DECLARE_int32(num_tablet_servers);
+
+using kudu::cluster::ExternalTabletServer;
+using kudu::itest::StartElection;
+using kudu::itest::TServerDetails;
+using std::string;
+using std::vector;
+using strings::Substitute;
+
+namespace kudu {
+namespace tserver {
+
+class RaftConsensusStressITest : public RaftConsensusITestBase {
+};
+
+// Test scenario to verify the behavior of the system when tablet replicas fail
+// and are replaced again and again. With high enough number of iterations, at
+// some point all replacement replicas are placed on top of previously
+// tombstoned ones.
+TEST_F(RaftConsensusStressITest, RemoveReplaceInCycle) {
+  if (!AllowSlowTests()) {
+    LOG(WARNING) << "test is skipped; set KUDU_ALLOW_SLOW_TESTS=1 to run";
+    return;
+  }
+
+  const bool is_343_scheme = FLAGS_test_raft_prepare_replacement_before_eviction;
+  const int kReplicaUnavailableSec = FLAGS_test_follower_unavailable_considered_failed_sec;
+  const int kReplicationFactor = FLAGS_test_replication_factor;
+  const int kNumTabletServers = 2 * kReplicationFactor;
+  const int kNumReplicasPerServer = FLAGS_test_num_replicas_per_server;
+  const int kNumTablets = kNumTabletServers * kNumReplicasPerServer / kReplicationFactor;
+  const MonoDelta kTimeout = MonoDelta::FromSeconds(60 * kBuildCfgFactor);
+  const MonoDelta kShortTimeout = MonoDelta::FromSeconds(1 * kBuildCfgFactor);
+
+  // Translate the replicas-per-server parameter into the catalog manager's
+  // --max_create_tablets_per_ts flag. Current logic in the catalog manager
+  // does not take into account the replication factor, that's why the
+  // additional division by kReplicationFactor.
+  const int kMaxCreateTabletsPerTs = kNumTablets / kReplicationFactor;
+
+  // This test scenario induces a lot of faults/errors and it runs multiple
+  // iterations. Tablet servers and master are too chatty in this case, logging
+  // a lot of information. Setting --minloglevel=2 allow for logging only of
+  // error and fatal messages from tablet servers and masters.
+  const vector<string> kMasterFlags = {
+    Substitute("--minloglevel=$0", FLAGS_test_minloglevel),
+    Substitute("--raft_prepare_replacement_before_eviction=$0", is_343_scheme),
+    Substitute("--max_create_tablets_per_ts=$0", kMaxCreateTabletsPerTs),
+  };
+  const vector<string> kTserverFlags = {
+    Substitute("--minloglevel=$0", FLAGS_test_minloglevel),
+    Substitute("--tablet_copy_early_session_timeout_prob=$0",
+               FLAGS_test_tablet_copy_early_session_timeout_prob),
+    Substitute("--raft_prepare_replacement_before_eviction=$0", is_343_scheme),
+    Substitute("--follower_unavailable_considered_failed_sec=$0",
+               kReplicaUnavailableSec),
+    Substitute("--consensus_rpc_timeout_ms=$0", 1000 * kReplicaUnavailableSec),
+    Substitute("--heartbeat_interval_ms=$0", FLAGS_test_heartbeat_interval_ms),
+    Substitute("--raft_heartbeat_interval_ms=$0", FLAGS_test_raft_heartbeat_interval_ms),
+  };
+
+  FLAGS_num_replicas = kReplicationFactor;
+  FLAGS_num_tablet_servers = kNumTabletServers;
+  NO_FATALS(BuildAndStart(kTserverFlags, kMasterFlags));
+
+  TestWorkload workload(cluster_.get());
+  workload.set_table_name("RemoveReplaceInCycle");
+  // Keep half of the total avaialable 'location space' for the replacement
+  // replicas spawned by the scenario below.
+  workload.set_num_tablets(kNumTablets);
+  workload.set_num_replicas(kReplicationFactor);
+  workload.set_num_write_threads(1);
+  workload.set_write_timeout_millis(kTimeout.ToMilliseconds());
+  workload.set_timeout_allowed(true);
+  // TODO(KUDU-1188): start using at least one read thread once leader leases
+  //                  are implemented. Without leader leases, keeping a reader
+  //                  thread leads to intermittent failures due to the CHECK()
+  //                  assertion in test_workload.cc:243 with messages like:
+  //
+  // Check failed: row_count >= expected_row_count (31049 vs. 31050)
+  //
+  workload.set_num_read_threads(0);
+  workload.set_client_default_admin_operation_timeout_millis(kTimeout.ToMilliseconds());
+  workload.Setup();
+  workload.Start();
+  while (workload.rows_inserted() < 100L * kNumTablets) {
+    SleepFor(MonoDelta::FromMilliseconds(10));
+  }
+  workload.StopAndJoin();
+
+  std::atomic<bool> is_running(true);
+  std::atomic<bool> do_elections(true);
+  std::atomic<bool> do_pauses(true);
+  std::thread pause_and_resume_thread([&] {
+    // Select random tablet server and pause it for some time to make the system
+    // spawn the replacement replica elsewhere.
+    int prev_ts_idx = -1;
+    while (is_running) {
+      const int ts_idx = rand() % cluster_->num_tablet_servers();
+      if (ts_idx == prev_ts_idx) {
+        continue;
+      }
+      ExternalTabletServer* ext_ts = cluster_->tablet_server(ts_idx);
+      TServerDetails* ts = tablet_servers_[ext_ts->uuid()];
+      vector<string> tablet_ids;
+      Status s = ListRunningTabletIds(ts, kShortTimeout, &tablet_ids);
+      if (s.IsNetworkError() || tablet_ids.empty()) {
+        continue;
+      }
+      CHECK_OK(s);
+      prev_ts_idx = ts_idx;
+      if (do_pauses) {
+        CHECK_OK(ext_ts->Pause());
+        SleepFor(MonoDelta::FromSeconds(3 * kReplicaUnavailableSec));
+        CHECK_OK(ext_ts->Resume());
+      }
+      SleepFor(MonoDelta::FromMilliseconds(250));
+    }
+  });
+  std::thread election_thread([&] {
+    while (is_running) {
+      if (do_elections) {
+        const auto ts_idx = rand() % cluster_->num_tablet_servers();
+        ExternalTabletServer* ext_ts = cluster_->tablet_server(ts_idx);
+        TServerDetails* ts = tablet_servers_[ext_ts->uuid()];
+        vector<string> tablet_ids;
+        Status s = ListRunningTabletIds(ts, kShortTimeout, &tablet_ids);
+        if (s.IsNetworkError() || s.IsTimedOut() || tablet_ids.empty()) {
+          continue;
+        }
+        CHECK_OK(s);
+        const auto tablet_idx = rand() % tablet_ids.size();
+        // Best effort attempt: ignoring the result of StartElection() call.
+        StartElection(ts, tablet_ids[tablet_idx], kShortTimeout);
+      }
+      SleepFor(kShortTimeout);
+    }
+  });
+  SCOPED_CLEANUP({
+    is_running = false;
+    pause_and_resume_thread.join();
+    election_thread.join();
+  });
+
+  auto ksck_failures_in_a_row = 0;
+  int64_t rows_inserted = workload.rows_inserted();
+  for (auto iteration = 0; iteration < FLAGS_test_num_iterations; ) {
+    workload.Start();
+    while (workload.rows_inserted() < rows_inserted + 10) {
+      SleepFor(MonoDelta::FromMilliseconds(1));
+    }
+    workload.StopAndJoin();
+    rows_inserted = workload.rows_inserted();
+
+    ClusterVerifier v(cluster_.get());
+    // Set shorter timeouts for the verification to abort earlier
+    // and signal the actor threads to stop messing with the tablets.
+    v.SetOperationsTimeout(kShortTimeout);
+    v.SetVerificationTimeout(kShortTimeout);
+
+    const auto& s = v.RunKsck();
+    if (!s.ok()) {
+      do_elections = false;
+      do_pauses = false;
+      if (!s.IsTimedOut()) {
+        ++ksck_failures_in_a_row;
+      }
+      if (ksck_failures_in_a_row > FLAGS_test_max_ksck_failures) {
+        break;
+      }
+      continue;
+    }
+    ksck_failures_in_a_row = 0;
+    do_elections = true;
+    do_pauses = true;
+
+    SleepFor(MonoDelta::FromSeconds(1));
+    LOG(INFO) << "completed iteration " << iteration;
+    ++iteration;
+  }
+  is_running = false;
+
+  NO_FATALS(cluster_->AssertNoCrashes());
+
+  ClusterVerifier v(cluster_.get());
+  v.SetOperationsTimeout(kTimeout);
+  v.SetVerificationTimeout(kTimeout);
+  if (ksck_failures_in_a_row > FLAGS_test_max_ksck_failures) {
+    // Suspecting a Raft consensus failure while running ksck with shorter
+    // timeout (see above). Run an extra round of ksck with the regular timeout
+    // to verify that replicas haven't really converged and, if so, just bail
+    // right at this point.
+    const auto& s = v.RunKsck();
+    if (!s.ok()) {
+      FAIL() << Substitute("$0: tablet replicas haven't converged", s.ToString());
+    }
+  }
+
+  NO_FATALS(v.CheckCluster());
+  // Using ClusterVerifier::AT_LEAST because the TestWorkload instance was run
+  // with the 'timeout_allowed' option enabled. In that case, the actual actual
+  // number of inserted rows may be greater than workload reports via its
+  // rows_inserted() method.
+  NO_FATALS(v.CheckRowCount(workload.table_name(),
+                            ClusterVerifier::AT_LEAST,
+                            workload.rows_inserted()));
+}
+
+}  // namespace tserver
+}  // namespace kudu