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 <[email protected]> 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 <[email protected]> Authored: Mon Jun 12 01:56:10 2017 -0700 Committer: Adar Dembo <[email protected]> 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() {
