kudu git commit: [mini_cluster] fix crash in GetLeaderMasterIndex()

[to...@apache.org]

Repository: kudu
Updated Branches:
  refs/heads/master 67fcfccd4 -> 8f2a8f918


[mini_cluster] fix crash in GetLeaderMasterIndex()

If at least one of the mini-masters is shutdown by explicit call
of the MiniMaster::Shutdown() method, the subsequent call of
MiniCluster::GetLeaderMasterIndex() crashes while trying to access
the underlying Master object.  That's because the shared_ptr wrapper
has been reset by prior call of the MiniMaster::Shutdown() call.

This patch adds a check for the presence of the Master object in the
wrapper prior to call of Master::Shutdown().

Besides, added a way to specify no placeholder for the
index parameter for MiniCluster::GetLeaderMasterIndex().  That's
for the use case when it's necessary to wait for the master
to be established in the mini-cluster with no intent to get its index.

Change-Id: Icbb64ee44e22b5f32a2351e0e1289826a8485c84
Reviewed-on: http://gerrit.cloudera.org:8080/5870
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Alexey Serbin <as...@cloudera.com>


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

Branch: refs/heads/master
Commit: 8f2a8f9187c50e39752b5abb231ca41a613c6791
Parents: 67fcfcc
Author: Alexey Serbin <as...@cloudera.com>
Authored: Wed Feb 1 21:23:44 2017 -0800
Committer: Alexey Serbin <as...@cloudera.com>
Committed: Thu Feb 2 18:59:32 2017 +0000

----------------------------------------------------------------------
 src/kudu/integration-tests/mini_cluster.cc | 10 ++++++----
 src/kudu/integration-tests/mini_cluster.h  | 10 ++++++++--
 src/kudu/master/mini_master.h              |  2 ++
 3 files changed, 16 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/8f2a8f91/src/kudu/integration-tests/mini_cluster.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/mini_cluster.cc b/src/kudu/integration-tests/mini_cluster.cc
index d633151..d50c339 100644
--- a/src/kudu/integration-tests/mini_cluster.cc
+++ b/src/kudu/integration-tests/mini_cluster.cc
@@ -297,11 +297,11 @@ Status MiniCluster::GetLeaderMasterIndex(int* idx) const {
   int leader_idx = -1;
   while (MonoTime::Now() < kDeadline) {
     for (int i = 0; i < num_masters(); i++) {
-      if (mini_master(i)->master()->IsShutdown()) {
+      master::MiniMaster* mm = mini_master(i);
+      if (!mm->is_running() || mm->master()->IsShutdown()) {
         continue;
       }
-      master::CatalogManager* catalog =
-          mini_master(i)->master()->catalog_manager();
+      master::CatalogManager* catalog = mm->master()->catalog_manager();
       master::CatalogManager::ScopedLeaderSharedLock l(catalog);
       if (l.first_failed_status().ok()) {
         leader_idx = i;
@@ -318,7 +318,9 @@ Status MiniCluster::GetLeaderMasterIndex(int* idx) const {
     return Status::NotFound("Leader master was not found within deadline");
   }
 
-  *idx = leader_idx;
+  if (idx) {
+    *idx = leader_idx;
+  }
   return Status::OK();
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/8f2a8f91/src/kudu/integration-tests/mini_cluster.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/mini_cluster.h b/src/kudu/integration-tests/mini_cluster.h
index a04a89c..be5928e 100644
--- a/src/kudu/integration-tests/mini_cluster.h
+++ b/src/kudu/integration-tests/mini_cluster.h
@@ -149,8 +149,14 @@ class MiniCluster : public MiniClusterBase {
   Status CreateClient(client::KuduClientBuilder* builder,
                       client::sp::shared_ptr<client::KuduClient>* client) const override;
 
-  // Determine the leader master of the cluster. Sets 'idx' to the leader
-  // master's index (for calls to to mini_master()).
+  // Determine the leader master of the cluster. Upon successful completion,
+  // sets 'idx' to the leader master's index. The result index index can be used
+  // as an argument for calls to mini_master().
+  //
+  // It's possible to use 'nullptr' instead of providing a valid placeholder
+  // for the result master index. That's for use cases when it's enough
+  // to determine if the cluster has established leader master
+  // without intent to get the actual index.
   //
   // Note: if a leader election occurs after this method is executed, the
   // last result may not be valid.

http://git-wip-us.apache.org/repos/asf/kudu/blob/8f2a8f91/src/kudu/master/mini_master.h
----------------------------------------------------------------------
diff --git a/src/kudu/master/mini_master.h b/src/kudu/master/mini_master.h
index f807a54..e50b45a 100644
--- a/src/kudu/master/mini_master.h
+++ b/src/kudu/master/mini_master.h
@@ -56,6 +56,8 @@ class MiniMaster {
 
   void Shutdown();
 
+  bool is_running() const { return running_; }
+
   // Restart the master on the same ports as it was previously bound.
   // Requires that the master is currently started.
   Status Restart();