You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2020/04/15 23:14:07 UTC
[kudu] 01/03: Revert "[catalog manager] cache masters' addresses"
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 <al...@apache.org>
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 <ha...@cloudera.com>
Tested-by: Alexey Serbin <as...@cloudera.com>
---
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();