This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch branch-1.12.x in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 8a90cb5d3fc381123d760888a2a3d371bc54e53b Author: Alexey Serbin <[email protected]> AuthorDate: Wed Apr 15 14:48:56 2020 -0700 Revert "[catalog manager] cache masters' addresses" This reverts commit 069c00c3a490e83c26d076a9f3ac3a9c7a87631b. Change-Id: I95a65435befc00d3b697ad83323eb08d999cabb5 Reviewed-on: http://gerrit.cloudera.org:8080/15739 Reviewed-by: Hao Hao <[email protected]> Tested-by: Alexey Serbin <[email protected]> --- src/kudu/master/catalog_manager.cc | 8 +++++--- src/kudu/master/catalog_manager.h | 10 ---------- src/kudu/master/master.cc | 10 +++++----- src/kudu/master/master.h | 4 ++-- src/kudu/master/master_service.cc | 12 ++++++++---- 5 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc index 0f98119..598c5f6 100644 --- a/src/kudu/master/catalog_manager.cc +++ b/src/kudu/master/catalog_manager.cc @@ -825,11 +825,13 @@ Status CatalogManager::Init(bool is_first_run) { auto_rebalancer_ = std::move(task); } - RETURN_NOT_OK(master_->GetMasterHostPorts(&master_addresses_)); if (hms::HmsCatalog::IsEnabled()) { + vector<HostPortPB> master_addrs_pb; + RETURN_NOT_OK(master_->GetMasterHostPorts(&master_addrs_pb)); + string master_addresses = JoinMapped( - master_addresses_, - [] (const HostPort& hostport) { + master_addrs_pb, + [] (const HostPortPB& hostport) { return Substitute("$0:$1", hostport.host(), hostport.port()); }, ","); diff --git a/src/kudu/master/catalog_manager.h b/src/kudu/master/catalog_manager.h index 6650a76..d6d5ff6 100644 --- a/src/kudu/master/catalog_manager.h +++ b/src/kudu/master/catalog_manager.h @@ -48,7 +48,6 @@ #include "kudu/util/cow_object.h" #include "kudu/util/locks.h" #include "kudu/util/monotime.h" -#include "kudu/util/net/net_util.h" #include "kudu/util/oid_generator.h" #include "kudu/util/random.h" #include "kudu/util/rw_mutex.h" @@ -784,10 +783,6 @@ class CatalogManager : public tserver::TabletReplicaLookupIf { // name is returned. static std::string NormalizeTableName(const std::string& table_name); - const std::vector<HostPort>& master_addresses() const { - return master_addresses_; - } - private: // These tests call ElectedAsLeaderCb() directly. FRIEND_TEST(MasterTest, TestShutdownDuringTableVisit); @@ -1163,11 +1158,6 @@ class CatalogManager : public tserver::TabletReplicaLookupIf { // Always acquire this lock before state_lock_. RWMutex leader_lock_; - // Cached information on master addresses. It's populated in Init() since the - // membership of masters' Raft consensus is static (i.e. no new members are - // added or any existing removed). - std::vector<HostPort> master_addresses_; - // Async operations are accessing some private methods // (TODO: this stuff should be deferred and done in the background thread) friend class AsyncAlterTable; diff --git a/src/kudu/master/master.cc b/src/kudu/master/master.cc index e8f81c7..e64b179 100644 --- a/src/kudu/master/master.cc +++ b/src/kudu/master/master.cc @@ -28,6 +28,7 @@ #include <glog/logging.h> #include "kudu/cfile/block_cache.h" +#include "kudu/common/common.pb.h" #include "kudu/common/wire_protocol.h" #include "kudu/common/wire_protocol.pb.h" #include "kudu/consensus/metadata.pb.h" @@ -366,7 +367,7 @@ Status Master::ListMasters(vector<ServerEntryPB>* masters) const { return Status::OK(); } -Status Master::GetMasterHostPorts(vector<HostPort>* hostports) const { +Status Master::GetMasterHostPorts(vector<HostPortPB>* hostports) const { auto consensus = catalog_manager_->master_consensus(); if (!consensus) { return Status::IllegalState("consensus not running"); @@ -374,7 +375,7 @@ Status Master::GetMasterHostPorts(vector<HostPort>* hostports) const { hostports->clear(); consensus::RaftConfigPB config = consensus->CommittedConfig(); - for (const auto& peer : *config.mutable_peers()) { + for (auto& peer : *config.mutable_peers()) { if (peer.member_type() == consensus::RaftPeerPB::VOTER) { // In non-distributed master configurations, we don't store our own // last known address in the Raft config. So, we'll fill it in from @@ -382,10 +383,9 @@ Status Master::GetMasterHostPorts(vector<HostPort>* hostports) const { if (!peer.has_last_known_addr()) { DCHECK_EQ(config.peers_size(), 1); DCHECK(registration_initialized_.load()); - DCHECK_GT(registration_.rpc_addresses_size(), 0); - hostports->emplace_back(HostPortFromPB(registration_.rpc_addresses(0))); + hostports->emplace_back(registration_.rpc_addresses(0)); } else { - hostports->emplace_back(HostPortFromPB(peer.last_known_addr())); + hostports->emplace_back(std::move(*peer.mutable_last_known_addr())); } } } diff --git a/src/kudu/master/master.h b/src/kudu/master/master.h index 28670fe..3f77074 100644 --- a/src/kudu/master/master.h +++ b/src/kudu/master/master.h @@ -32,7 +32,7 @@ namespace kudu { -class HostPort; +class HostPortPB; class MaintenanceManager; class MonoDelta; class ThreadPool; @@ -105,7 +105,7 @@ class Master : public kserver::KuduServer { // Gets the HostPorts for all of the masters in the cluster. // This is not as complete as ListMasters() above, but operates just // based on local state. - Status GetMasterHostPorts(std::vector<HostPort>* hostports) const; + Status GetMasterHostPorts(std::vector<HostPortPB>* hostports) const; // Crash the master on disk error. static void CrashMasterOnDiskError(const std::string& uuid); diff --git a/src/kudu/master/master_service.cc b/src/kudu/master/master_service.cc index b67cd02..50ed51d 100644 --- a/src/kudu/master/master_service.cc +++ b/src/kudu/master/master_service.cc @@ -629,10 +629,14 @@ void MasterServiceImpl::ConnectToMaster(const ConnectToMasterRequestPB* /*req*/, // Set the info about the other masters, so that the client can verify // it has the full set of info. - const auto& addresses = server_->catalog_manager()->master_addresses(); - resp->mutable_master_addrs()->Reserve(addresses.size()); - for (const auto& hp : addresses) { - *resp->add_master_addrs() = HostPortToPB(hp); + { + vector<HostPortPB> hostports; + WARN_NOT_OK(server_->GetMasterHostPorts(&hostports), + "unable to get HostPorts for masters"); + resp->mutable_master_addrs()->Reserve(hostports.size()); + for (auto& hp : hostports) { + *resp->add_master_addrs() = std::move(hp); + } } const bool is_leader = l.leader_status().ok();
