You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2017/06/14 00:17:56 UTC
[3/3] kudu git commit: mini_cluster: shutdown in destructors
mini_cluster: shutdown in destructors
The CHECK in the MiniMaster destructor isn't safe; it can fire during test
setup before the master has been shut down. For example, consider this
(edited) code snippet from MiniCluster::StartSingleMaster():
gscoped_ptr<MiniMaster> mini_master(new MiniMaster(...));
RETURN_NOT_OK(mini_master->Start());
RETURN_NOT_OK(mini_master->master()->WaitForSomething(...));
mini_masters_.push_back(shared_ptr<MiniMaster>(mini_master.release()));
MiniCluster's destructor guarantees that the masters are shut down before
being destroyed, but if WaitForSomething() fails, the new master won't be
added to mini_masters_ and thus will be destroyed without first being shut
down at the end of StartSingleMaster(). To make this safe, let's remove the
CHECK and shut down the master in the destructor. I rolled this out to
MiniTabletServer too, for consistency.
The CHECK (and timeout in StartSingleMaster()) is the cause of one source of
flakiness in delete_tablet-itest.
Change-Id: I742e190f751535d2a59354c3b35cdaf0a340d4ea
Reviewed-on: http://gerrit.cloudera.org:8080/7151
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/b4de65a8
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/b4de65a8
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/b4de65a8
Branch: refs/heads/master
Commit: b4de65a8a7e398157cfbc64d4f4f5ee08ab953ed
Parents: 01deeab
Author: Adar Dembo <ad...@cloudera.com>
Authored: Mon Jun 12 01:56:10 2017 -0700
Committer: Adar Dembo <ad...@cloudera.com>
Committed: Wed Jun 14 00:17:26 2017 +0000
----------------------------------------------------------------------
src/kudu/integration-tests/mini_cluster.cc | 4 ++--
src/kudu/master/mini_master.cc | 2 +-
src/kudu/tserver/mini_tablet_server.cc | 1 +
3 files changed, 4 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/b4de65a8/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 3139dc5..da57fb0 100644
--- a/src/kudu/integration-tests/mini_cluster.cc
+++ b/src/kudu/integration-tests/mini_cluster.cc
@@ -155,8 +155,8 @@ Status MiniCluster::StartSingleMaster() {
gscoped_ptr<MiniMaster> mini_master(
new MiniMaster(env_, GetMasterFsRoot(0), master_rpc_port));
RETURN_NOT_OK_PREPEND(mini_master->Start(), "Couldn't start master");
- RETURN_NOT_OK(mini_master->master()->
- WaitUntilCatalogManagerIsLeaderAndReadyForTests(MonoDelta::FromSeconds(5)));
+ RETURN_NOT_OK(mini_master->master()->WaitUntilCatalogManagerIsLeaderAndReadyForTests(
+ MonoDelta::FromSeconds(kMasterStartupWaitTimeSeconds)));
mini_masters_.push_back(shared_ptr<MiniMaster>(mini_master.release()));
return Status::OK();
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/b4de65a8/src/kudu/master/mini_master.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/mini_master.cc b/src/kudu/master/mini_master.cc
index f872dff..63188b5 100644
--- a/src/kudu/master/mini_master.cc
+++ b/src/kudu/master/mini_master.cc
@@ -48,7 +48,7 @@ MiniMaster::MiniMaster(Env* env, string fs_root, uint16_t rpc_port)
}
MiniMaster::~MiniMaster() {
- CHECK(!running_);
+ Shutdown();
}
Status MiniMaster::Start() {
http://git-wip-us.apache.org/repos/asf/kudu/blob/b4de65a8/src/kudu/tserver/mini_tablet_server.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/mini_tablet_server.cc b/src/kudu/tserver/mini_tablet_server.cc
index aabb9df..67a2734 100644
--- a/src/kudu/tserver/mini_tablet_server.cc
+++ b/src/kudu/tserver/mini_tablet_server.cc
@@ -54,6 +54,7 @@ MiniTabletServer::MiniTabletServer(const string& fs_root,
}
MiniTabletServer::~MiniTabletServer() {
+ Shutdown();
}
Status MiniTabletServer::Start() {