Repository: kudu Updated Branches: refs/heads/master 74dc5da86 -> 954d9f71b
Add CatalogManager::master_consensus() accessor A patch I'm working on is going to start calling Master::GetMasterHostPorts from inside of CatalogManager::Init. GetMasterHostPorts calls into the catalog manager, and checks that it is initialized. To break this circular dependency this introduces a new accessor for the master tablet RaftConsensus instance which becomes available immediately after the tablet is initialized. A few call-sites are switched over to this accessor instead of drilling into catalog manager. Change-Id: Ie6887900329e67222da129b4a1b532cfb0a364b4 Reviewed-on: http://gerrit.cloudera.org:8080/9541 Reviewed-by: Adar Dembo <a...@cloudera.com> Reviewed-by: Alexey Serbin <aser...@cloudera.com> Tested-by: Kudu Jenkins Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/954d9f71 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/954d9f71 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/954d9f71 Branch: refs/heads/master Commit: 954d9f71b50e2fae24a6e3acfe094b4d24dc5c2c Parents: 74dc5da Author: Dan Burkert <d...@danburkert.com> Authored: Wed Mar 7 15:06:11 2018 -0800 Committer: Dan Burkert <danburk...@apache.org> Committed: Thu Mar 8 00:39:23 2018 +0000 ---------------------------------------------------------------------- src/kudu/integration-tests/security-master-auth-itest.cc | 9 ++------- src/kudu/master/catalog_manager.cc | 11 +++++++++++ src/kudu/master/catalog_manager.h | 5 +++++ src/kudu/master/master.cc | 9 ++------- 4 files changed, 20 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/954d9f71/src/kudu/integration-tests/security-master-auth-itest.cc ---------------------------------------------------------------------- diff --git a/src/kudu/integration-tests/security-master-auth-itest.cc b/src/kudu/integration-tests/security-master-auth-itest.cc index c8eca38..9bdede1 100644 --- a/src/kudu/integration-tests/security-master-auth-itest.cc +++ b/src/kudu/integration-tests/security-master-auth-itest.cc @@ -22,16 +22,13 @@ #include <gtest/gtest.h> #include "kudu/consensus/raft_consensus.h" -#include "kudu/gutil/ref_counted.h" #include "kudu/master/catalog_manager.h" #include "kudu/master/master.h" #include "kudu/master/mini_master.h" -#include "kudu/master/sys_catalog.h" #include "kudu/mini-cluster/internal_mini_cluster.h" #include "kudu/rpc/messenger.h" #include "kudu/security/tls_context.h" #include "kudu/security/token_verifier.h" -#include "kudu/tablet/tablet_replica.h" #include "kudu/util/status.h" #include "kudu/util/test_macros.h" #include "kudu/util/test_util.h" @@ -84,8 +81,7 @@ TEST_F(SecurityMasterAuthTest, FollowerCertificates) { ASSERT_TRUE(tls.has_cert()); } - auto* consensus = cluster_->mini_master(0)->master()->catalog_manager()-> - sys_catalog()->tablet_replica()->consensus(); + auto consensus = cluster_->mini_master(0)->master()->catalog_manager()->master_consensus(); ASSERT_OK(consensus->StartElection( RaftConsensus::ELECT_EVEN_IF_LEADER_IS_ALIVE, RaftConsensus::EXTERNAL_REQUEST)); @@ -105,8 +101,7 @@ TEST_F(SecurityMasterAuthTest, FollowerCertificates) { // the rest have always been followers. This is a test to cover regressions of // KUDU-2319, if any. TEST_F(SecurityMasterAuthTest, FollowerTokenVerificationKeys) { - auto* consensus = cluster_->mini_master(0)->master()->catalog_manager()-> - sys_catalog()->tablet_replica()->consensus(); + auto consensus = cluster_->mini_master(0)->master()->catalog_manager()->master_consensus(); ASSERT_OK(consensus->StartElection( RaftConsensus::ELECT_EVEN_IF_LEADER_IS_ALIVE, RaftConsensus::EXTERNAL_REQUEST)); http://git-wip-us.apache.org/repos/asf/kudu/blob/954d9f71/src/kudu/master/catalog_manager.cc ---------------------------------------------------------------------- diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc index 974927e..2aa09af 100644 --- a/src/kudu/master/catalog_manager.cc +++ b/src/kudu/master/catalog_manager.cc @@ -3755,6 +3755,17 @@ Status CatalogManager::ProcessTabletReport( return Status::OK(); } +std::shared_ptr<RaftConsensus> CatalogManager::master_consensus() const { + // CatalogManager::InitSysCatalogAsync takes lock_ in exclusive mode in order + // to initialize sys_catalog_, so it's sufficient to take lock_ in shared mode + // here to protect access to sys_catalog_. + shared_lock<LockType> l(lock_); + if (!sys_catalog_) { + return nullptr; + } + return sys_catalog_->tablet_replica()->shared_consensus(); +} + void CatalogManager::SendAlterTableRequest(const scoped_refptr<TableInfo>& table) { vector<scoped_refptr<TabletInfo>> tablets; table->GetAllTablets(&tablets); http://git-wip-us.apache.org/repos/asf/kudu/blob/954d9f71/src/kudu/master/catalog_manager.h ---------------------------------------------------------------------- diff --git a/src/kudu/master/catalog_manager.h b/src/kudu/master/catalog_manager.h index 0ae32ff..4efc318 100644 --- a/src/kudu/master/catalog_manager.h +++ b/src/kudu/master/catalog_manager.h @@ -77,6 +77,7 @@ class TokenSigningPublicKeyPB; } // namespace security namespace consensus { +class RaftConsensus; class StartTabletCopyRequestPB; } @@ -586,6 +587,10 @@ class CatalogManager : public tserver::TabletReplicaLookupIf { SysCatalogTable* sys_catalog() { return sys_catalog_.get(); } + // Returns the Master tablet's RaftConsensus instance if it is initialized, or + // else a nullptr. + std::shared_ptr<consensus::RaftConsensus> master_consensus() const; + // Dump all of the current state about tables and tablets to the // given output stream. This is verbose, meant for debugging. void DumpState(std::ostream* out) const; http://git-wip-us.apache.org/repos/asf/kudu/blob/954d9f71/src/kudu/master/master.cc ---------------------------------------------------------------------- diff --git a/src/kudu/master/master.cc b/src/kudu/master/master.cc index d198777..ad09f47 100644 --- a/src/kudu/master/master.cc +++ b/src/kudu/master/master.cc @@ -37,7 +37,6 @@ #include "kudu/gutil/bind.h" #include "kudu/gutil/bind_helpers.h" #include "kudu/gutil/move.h" -#include "kudu/gutil/ref_counted.h" #include "kudu/gutil/strings/substitute.h" #include "kudu/master/catalog_manager.h" #include "kudu/master/master.pb.h" @@ -45,7 +44,6 @@ #include "kudu/master/master_cert_authority.h" #include "kudu/master/master_path_handlers.h" #include "kudu/master/master_service.h" -#include "kudu/master/sys_catalog.h" #include "kudu/master/ts_manager.h" #include "kudu/rpc/messenger.h" #include "kudu/rpc/rpc_controller.h" @@ -53,7 +51,6 @@ #include "kudu/security/token_signer.h" #include "kudu/server/rpc_server.h" #include "kudu/server/webserver.h" -#include "kudu/tablet/tablet_replica.h" #include "kudu/tserver/tablet_copy_service.h" #include "kudu/tserver/tablet_service.h" #include "kudu/util/flag_tags.h" @@ -309,8 +306,7 @@ Status Master::ListMasters(std::vector<ServerEntryPB>* masters) const { return Status::OK(); } - RETURN_NOT_OK(catalog_manager_->CheckOnline()); - auto consensus = catalog_manager_->sys_catalog()->tablet_replica()->shared_consensus(); + auto consensus = catalog_manager_->master_consensus(); if (!consensus) { return Status::IllegalState("consensus not running"); } @@ -344,8 +340,7 @@ Status Master::ListMasters(std::vector<ServerEntryPB>* masters) const { } Status Master::GetMasterHostPorts(std::vector<HostPortPB>* hostports) const { - RETURN_NOT_OK(catalog_manager_->CheckOnline()); - auto consensus = catalog_manager_->sys_catalog()->tablet_replica()->shared_consensus(); + auto consensus = catalog_manager_->master_consensus(); if (!consensus) { return Status::IllegalState("consensus not running"); }
