kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From danburk...@apache.org
Subject kudu git commit: Add CatalogManager::master_consensus() accessor
Date Thu, 08 Mar 2018 00:40:01 GMT
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 <adar@cloudera.com>
Reviewed-by: Alexey Serbin <aserbin@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 <dan@danburkert.com>
Authored: Wed Mar 7 15:06:11 2018 -0800
Committer: Dan Burkert <danburkert@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");
   }


Mime
View raw message