This is an automated email from the ASF dual-hosted git repository. bankim pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit ed3d916e92decf7333f4200e88c93fbcc211404d Author: Bankim Bhavsar <[email protected]> AuthorDate: Thu Mar 11 15:49:28 2021 -0800 [master] Check for 'last_known_addr' field when adding master In a single master configuration, the 'last_known_addr' field may not be populated in the Raft config and Raft implementation will throw an error on adding a new master which is not actionable. Hence adding an explicit check with an actionable error message to fix the problem. Change-Id: Ic991c82acbb2a2149f647b5c1a5b323fecb958cb Reviewed-on: http://gerrit.cloudera.org:8080/17176 Reviewed-by: Andrew Wong <[email protected]> Tested-by: Kudu Jenkins --- src/kudu/master/dynamic_multi_master-test.cc | 3 ++- src/kudu/master/master.cc | 20 +++++++++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/kudu/master/dynamic_multi_master-test.cc b/src/kudu/master/dynamic_multi_master-test.cc index f1e051d..09a1d7b 100644 --- a/src/kudu/master/dynamic_multi_master-test.cc +++ b/src/kudu/master/dynamic_multi_master-test.cc @@ -1127,7 +1127,8 @@ TEST_F(DynamicMultiMasterTest, TestAddMasterWithNoLastKnownAddr) { string err; Status actual = AddMasterToClusterUsingCLITool(reserved_hp_, &err); ASSERT_TRUE(actual.IsRuntimeError()) << actual.ToString(); - ASSERT_STR_MATCHES(err, "Invalid config to set as pending: Peer:.* has no address"); + ASSERT_STR_MATCHES(err, "'last_known_addr' field in single master Raft configuration not set. " + "Please restart master with --master_addresses flag"); // Verify no change in number of masters. NO_FATALS(VerifyVoterMasters(orig_num_masters_)); diff --git a/src/kudu/master/master.cc b/src/kudu/master/master.cc index 68552da..7b9dd47 100644 --- a/src/kudu/master/master.cc +++ b/src/kudu/master/master.cc @@ -485,7 +485,7 @@ Status Master::GetMasterHostPorts(vector<HostPort>* hostports, MasterType type) hostports->clear(); consensus::RaftConfigPB config = consensus->CommittedConfig(); - for (const auto& peer : *config.mutable_peers()) { + for (const auto& peer : config.peers()) { if (type == MasterType::ALL || get_raft_member_type(type) == peer.member_type()) { // In non-distributed master configurations, we may not store our own // last known address in the Raft config. So, we'll fill it in from @@ -512,6 +512,24 @@ Status Master::AddMaster(const HostPort& hp, rpc::RpcContext* rpc) { return Status::AlreadyPresent("Master already present"); } + // If a new master is being added to a single master configuration, check that + // the current leader master has the 'last_known_addr' populated. Otherwise + // Raft will return an error which isn't very actionable. + if (masters.size() == 1) { + auto consensus = catalog_manager_->master_consensus(); + if (!consensus) { + return Status::IllegalState("consensus not running"); + } + const auto& config = consensus->CommittedConfig(); + DCHECK_EQ(1, config.peers_size()); + if (!config.peers(0).has_last_known_addr()) { + return Status::IllegalState("'last_known_addr' field in single master Raft configuration not " + "set. Please restart master with --master_addresses flag set " + "to the single master which will populate the 'last_known_addr' " + "field."); + } + } + // Check whether the master to be added is reachable and fetch its uuid. ServerEntryPB peer_entry; RETURN_NOT_OK(GetMasterEntryForHost(messenger_, hp, &peer_entry));
