Repository: kudu Updated Branches: refs/heads/master 5360339df -> d1c99e02f
[tools] ksck improvements [1/n]: Eliminate KsckMaster class ksck has a KsckCluster and a KsckMaster class. The KsckCluster class's methods more-or-less map one-to-one with KsckMaster class methods. The KsckMaster class represents the collective masters of a cluster, not an individual master. This patch combines the two classes into a KsckCluster class. This class is the base class for a MockKsckCluster class and a RemoteKsckCluster class, for tests and "real life", respectively. This is just refactoring. There are no functional changes in this patch. Change-Id: I21f9e244b6ba10e11327c744abc1ac72a9b2ab1c Reviewed-on: http://gerrit.cloudera.org:8080/9881 Reviewed-by: Attila Bukor <abu...@cloudera.com> Reviewed-by: Andrew Wong <aw...@cloudera.com> Tested-by: Will Berkeley <wdberke...@gmail.com> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/7e35aee9 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/7e35aee9 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/7e35aee9 Branch: refs/heads/master Commit: 7e35aee9626bf3370d9b1d89fd1f3440b47ff7a8 Parents: 5360339 Author: Will Berkeley <wdberke...@apache.org> Authored: Thu Mar 29 16:48:42 2018 -0700 Committer: Will Berkeley <wdberke...@gmail.com> Committed: Thu Apr 5 18:20:27 2018 +0000 ---------------------------------------------------------------------- src/kudu/client/schema.h | 4 +- src/kudu/integration-tests/cluster_verifier.cc | 10 ++- src/kudu/tools/ksck-test.cc | 67 ++++++++--------- src/kudu/tools/ksck.cc | 43 +++-------- src/kudu/tools/ksck.h | 80 ++++++++------------- src/kudu/tools/ksck_remote-test.cc | 23 +++--- src/kudu/tools/ksck_remote.cc | 22 +++--- src/kudu/tools/ksck_remote.h | 25 +++---- src/kudu/tools/tool_action_cluster.cc | 10 ++- src/kudu/tools/tool_action_tablet.cc | 7 +- 10 files changed, 113 insertions(+), 178 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/7e35aee9/src/kudu/client/schema.h ---------------------------------------------------------------------- diff --git a/src/kudu/client/schema.h b/src/kudu/client/schema.h index c0abbf9..6bc8241 100644 --- a/src/kudu/client/schema.h +++ b/src/kudu/client/schema.h @@ -44,7 +44,7 @@ class Schema; class Slice; namespace tools { -class RemoteKsckMaster; +class RemoteKsckCluster; class ReplicaDumper; } @@ -585,7 +585,7 @@ class KUDU_EXPORT KuduSchema { friend class internal::MetaCache; friend class internal::MetaCacheEntry; friend class internal::WriteRpc; - friend class tools::RemoteKsckMaster; + friend class tools::RemoteKsckCluster; friend class tools::ReplicaDumper; friend KuduSchema KuduSchemaFromSchema(const Schema& schema); http://git-wip-us.apache.org/repos/asf/kudu/blob/7e35aee9/src/kudu/integration-tests/cluster_verifier.cc ---------------------------------------------------------------------- diff --git a/src/kudu/integration-tests/cluster_verifier.cc b/src/kudu/integration-tests/cluster_verifier.cc index 22cba09..834ac41 100644 --- a/src/kudu/integration-tests/cluster_verifier.cc +++ b/src/kudu/integration-tests/cluster_verifier.cc @@ -47,8 +47,7 @@ using cluster::MiniCluster; using strings::Substitute; using tools::Ksck; using tools::KsckCluster; -using tools::KsckMaster; -using tools::RemoteKsckMaster; +using tools::RemoteKsckCluster; ClusterVerifier::ClusterVerifier(MiniCluster* cluster) : cluster_(cluster), @@ -102,9 +101,8 @@ Status ClusterVerifier::RunKsck() { for (const auto& hp : cluster_->master_rpc_addrs()) { hp_strs.emplace_back(hp.ToString()); } - std::shared_ptr<KsckMaster> master; - RETURN_NOT_OK(RemoteKsckMaster::Build(hp_strs, &master)); - std::shared_ptr<KsckCluster> cluster(new KsckCluster(master)); + std::shared_ptr<KsckCluster> cluster; + RETURN_NOT_OK(RemoteKsckCluster::Build(hp_strs, &cluster)); std::shared_ptr<Ksck> ksck(new Ksck(cluster)); // Some unit tests create or remove replicas of tablets, which @@ -112,7 +110,7 @@ Status ClusterVerifier::RunKsck() { ksck->set_check_replica_count(false); // This is required for everything below. - RETURN_NOT_OK(ksck->CheckMasterRunning()); + RETURN_NOT_OK(ksck->CheckClusterRunning()); RETURN_NOT_OK(ksck->FetchTableAndTabletInfo()); RETURN_NOT_OK(ksck->FetchInfoFromTabletServers()); RETURN_NOT_OK(ksck->CheckTablesConsistency()); http://git-wip-us.apache.org/repos/asf/kudu/blob/7e35aee9/src/kudu/tools/ksck-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/tools/ksck-test.cc b/src/kudu/tools/ksck-test.cc index 74157e7..c43f6dd 100644 --- a/src/kudu/tools/ksck-test.cc +++ b/src/kudu/tools/ksck-test.cc @@ -97,41 +97,38 @@ class MockKsckTabletServer : public KsckTabletServer { const string address_; }; -class MockKsckMaster : public KsckMaster { +class MockKsckCluster : public KsckCluster { public: - MockKsckMaster() + MockKsckCluster() : fetch_info_status_(Status::OK()) { } - virtual Status Connect() OVERRIDE { + virtual Status Connect() override { return fetch_info_status_; } - virtual Status RetrieveTabletServers(TSMap* tablet_servers) OVERRIDE { - *tablet_servers = tablet_servers_; + virtual Status RetrieveTabletServers() override { return Status::OK(); } - virtual Status RetrieveTablesList(vector<shared_ptr<KsckTable>>* tables) OVERRIDE { - tables->assign(tables_.begin(), tables_.end()); + virtual Status RetrieveTablesList() override { return Status::OK(); } - virtual Status RetrieveTabletsList(const shared_ptr<KsckTable>& table) OVERRIDE { + virtual Status RetrieveTabletsList(const shared_ptr<KsckTable>& table) override { return Status::OK(); } // Public because the unit tests mutate these variables directly. Status fetch_info_status_; - TSMap tablet_servers_; - vector<shared_ptr<KsckTable>> tables_; + using KsckCluster::tables_; + using KsckCluster::tablet_servers_; }; class KsckTest : public KuduTest { public: KsckTest() - : master_(new MockKsckMaster()), - cluster_(new KsckCluster(static_pointer_cast<KsckMaster>(master_))), + : cluster_(new MockKsckCluster()), ksck_(new Ksck(cluster_, &err_stream_)) { FLAGS_color = "never"; unordered_map<string, shared_ptr<KsckTabletServer>> tablet_servers; @@ -140,7 +137,7 @@ class KsckTest : public KuduTest { shared_ptr<MockKsckTabletServer> ts(new MockKsckTabletServer(name)); InsertOrDie(&tablet_servers, ts->uuid(), ts); } - master_->tablet_servers_.swap(tablet_servers); + cluster_->tablet_servers_.swap(tablet_servers); } protected: @@ -165,7 +162,7 @@ class KsckTest : public KuduTest { void CreateDefaultAssignmentPlan(int tablets_count) { while (tablets_count > 0) { - for (const KsckMaster::TSMap::value_type& entry : master_->tablet_servers_) { + for (const KsckCluster::TSMap::value_type& entry : cluster_->tablet_servers_) { if (tablets_count-- == 0) return; assignment_plan_.push_back(entry.second->uuid()); } @@ -227,7 +224,7 @@ class KsckTest : public KuduTest { shared_ptr<KsckTable> CreateAndAddTable(const string& name, int num_replicas) { shared_ptr<KsckTable> table(new KsckTable(name, Schema(), num_replicas)); vector<shared_ptr<KsckTable>> tables = { table }; - master_->tables_.assign(tables.begin(), tables.end()); + cluster_->tables_.assign(tables.begin(), tables.end()); return table; } @@ -258,7 +255,7 @@ class KsckTest : public KuduTest { } for (const auto& replica : tablet->replicas()) { shared_ptr<MockKsckTabletServer> ts = - static_pointer_cast<MockKsckTabletServer>(master_->tablet_servers_.at(replica->ts_uuid())); + static_pointer_cast<MockKsckTabletServer>(cluster_->tablet_servers_.at(replica->ts_uuid())); InsertOrDieNoPrint(&ts->tablet_consensus_state_map_, std::make_pair(replica->ts_uuid(), tablet->id()), cstate); @@ -272,7 +269,7 @@ class KsckTest : public KuduTest { shared_ptr<KsckTabletReplica> replica( new KsckTabletReplica(assignment_plan_.back(), is_leader, true)); shared_ptr<MockKsckTabletServer> ts = static_pointer_cast<MockKsckTabletServer>( - master_->tablet_servers_.at(assignment_plan_.back())); + cluster_->tablet_servers_.at(assignment_plan_.back())); assignment_plan_.pop_back(); replicas->push_back(replica); @@ -289,16 +286,14 @@ class KsckTest : public KuduTest { auto c = MakeScopedCleanup([this]() { LOG(INFO) << "Ksck output:\n" << err_stream_.str(); }); - RETURN_NOT_OK(ksck_->CheckMasterRunning()); + RETURN_NOT_OK(ksck_->CheckClusterRunning()); RETURN_NOT_OK(ksck_->FetchTableAndTabletInfo()); RETURN_NOT_OK(ksck_->FetchInfoFromTabletServers()); RETURN_NOT_OK(ksck_->CheckTablesConsistency()); return Status::OK(); } - - shared_ptr<MockKsckMaster> master_; - shared_ptr<KsckCluster> cluster_; + shared_ptr<MockKsckCluster> cluster_; shared_ptr<Ksck> ksck_; // This is used as a stack. First the unit test is responsible to create a plan to follow, that // is the order in which each replica of each tablet will be assigned, starting from the end. @@ -311,13 +306,13 @@ class KsckTest : public KuduTest { }; TEST_F(KsckTest, TestMasterOk) { - ASSERT_OK(ksck_->CheckMasterRunning()); + ASSERT_OK(ksck_->CheckClusterRunning()); } TEST_F(KsckTest, TestMasterUnavailable) { Status error = Status::NetworkError("Network failure"); - master_->fetch_info_status_ = error; - ASSERT_TRUE(ksck_->CheckMasterRunning().IsNetworkError()); + cluster_->fetch_info_status_ = error; + ASSERT_TRUE(ksck_->CheckClusterRunning().IsNetworkError()); } TEST_F(KsckTest, TestTabletServersOk) { @@ -329,10 +324,10 @@ TEST_F(KsckTest, TestWrongUUIDTabletServer) { Status error = Status::RemoteError("ID reported by tablet server " "doesn't match the expected ID"); - static_pointer_cast<MockKsckTabletServer>(master_->tablet_servers_["ts-id-1"]) + static_pointer_cast<MockKsckTabletServer>(cluster_->tablet_servers_["ts-id-1"]) ->fetch_info_status_ = error; - ASSERT_OK(ksck_->CheckMasterRunning()); + ASSERT_OK(ksck_->CheckClusterRunning()); ASSERT_OK(ksck_->FetchTableAndTabletInfo()); ASSERT_TRUE(ksck_->FetchInfoFromTabletServers().IsNetworkError()); ASSERT_STR_CONTAINS(err_stream_.str(), @@ -350,10 +345,10 @@ TEST_F(KsckTest, TestBadTabletServer) { // Mock a failure to connect to one of the tablet servers. Status error = Status::NetworkError("Network failure"); - static_pointer_cast<MockKsckTabletServer>(master_->tablet_servers_["ts-id-1"]) + static_pointer_cast<MockKsckTabletServer>(cluster_->tablet_servers_["ts-id-1"]) ->fetch_info_status_ = error; - ASSERT_OK(ksck_->CheckMasterRunning()); + ASSERT_OK(ksck_->CheckClusterRunning()); ASSERT_OK(ksck_->FetchTableAndTabletInfo()); Status s = ksck_->FetchInfoFromTabletServers(); ASSERT_TRUE(s.IsNetworkError()) << "Status returned: " << s.ToString(); @@ -450,7 +445,7 @@ TEST_F(KsckTest, TestConsensusConflictExtraPeer) { FLAGS_consensus = true; CreateOneSmallReplicatedTable(); - shared_ptr<KsckTabletServer> ts = FindOrDie(master_->tablet_servers_, "ts-id-0"); + shared_ptr<KsckTabletServer> ts = FindOrDie(cluster_->tablet_servers_, "ts-id-0"); auto& cstate = FindOrDieNoPrint(ts->tablet_consensus_state_map_, std::make_pair("ts-id-0", "tablet-id-0")); cstate.mutable_committed_config()->add_peers()->set_permanent_uuid("ts-id-fake"); @@ -477,7 +472,7 @@ TEST_F(KsckTest, TestConsensusConflictMissingPeer) { FLAGS_consensus = true; CreateOneSmallReplicatedTable(); - shared_ptr<KsckTabletServer> ts = FindOrDie(master_->tablet_servers_, "ts-id-0"); + shared_ptr<KsckTabletServer> ts = FindOrDie(cluster_->tablet_servers_, "ts-id-0"); auto& cstate = FindOrDieNoPrint(ts->tablet_consensus_state_map_, std::make_pair("ts-id-0", "tablet-id-0")); cstate.mutable_committed_config()->mutable_peers()->RemoveLast(); @@ -504,7 +499,7 @@ TEST_F(KsckTest, TestConsensusConflictDifferentLeader) { FLAGS_consensus = true; CreateOneSmallReplicatedTable(); - const shared_ptr<KsckTabletServer>& ts = FindOrDie(master_->tablet_servers_, "ts-id-0"); + const shared_ptr<KsckTabletServer>& ts = FindOrDie(cluster_->tablet_servers_, "ts-id-0"); auto& cstate = FindOrDieNoPrint(ts->tablet_consensus_state_map_, std::make_pair("ts-id-0", "tablet-id-0")); cstate.set_leader_uuid("ts-id-1"); @@ -545,7 +540,7 @@ TEST_F(KsckTest, TestOneOneTabletBrokenTable) { TEST_F(KsckTest, TestMismatchedAssignments) { CreateOneSmallReplicatedTable(); shared_ptr<MockKsckTabletServer> ts = static_pointer_cast<MockKsckTabletServer>( - master_->tablet_servers_.at(Substitute("ts-id-$0", 0))); + cluster_->tablet_servers_.at(Substitute("ts-id-$0", 0))); ASSERT_EQ(1, ts->tablet_status_map_.erase("tablet-id-2")); Status s = RunKsck(); @@ -598,7 +593,7 @@ TEST_F(KsckTest, TestTabletCopying) { // Mark one of the tablet replicas as copying. auto not_running_ts = static_pointer_cast<MockKsckTabletServer>( - master_->tablet_servers_.at(assignment_plan_.back())); + cluster_->tablet_servers_.at(assignment_plan_.back())); auto& pb = FindOrDie(not_running_ts->tablet_status_map_, "tablet-id-0"); pb.set_tablet_data_state(TabletDataState::TABLET_DATA_COPYING); Status s = RunKsck(); @@ -619,7 +614,7 @@ TEST_F(KsckTest, TestMasterNotReportingTabletServer) { // Delete a tablet server from the master's list. This simulates a situation // where the master is starting and doesn't list all tablet servers yet, but // tablets from other tablet servers are listing a missing tablet server as a peer. - EraseKeyReturnValuePtr(&master_->tablet_servers_, "ts-id-0"); + EraseKeyReturnValuePtr(&cluster_->tablet_servers_, "ts-id-0"); Status s = RunKsck(); ASSERT_EQ("Corruption: 1 out of 1 table(s) are not healthy", s.ToString()); ASSERT_STR_CONTAINS(err_stream_.str(), "Table test has 3 under-replicated tablet(s)"); @@ -637,10 +632,10 @@ TEST_F(KsckTest, TestMasterNotReportingTabletServerWithConsensusConflict) { CreateOneSmallReplicatedTable(); // Delete a tablet server from the cluster's list as in TestMasterNotReportingTabletServer. - EraseKeyReturnValuePtr(&master_->tablet_servers_, "ts-id-0"); + EraseKeyReturnValuePtr(&cluster_->tablet_servers_, "ts-id-0"); // Now engineer a consensus conflict. - const shared_ptr<KsckTabletServer>& ts = FindOrDie(master_->tablet_servers_, "ts-id-1"); + const shared_ptr<KsckTabletServer>& ts = FindOrDie(cluster_->tablet_servers_, "ts-id-1"); auto& cstate = FindOrDieNoPrint(ts->tablet_consensus_state_map_, std::make_pair("ts-id-1", "tablet-id-1")); cstate.set_leader_uuid("ts-id-1"); http://git-wip-us.apache.org/repos/asf/kudu/blob/7e35aee9/src/kudu/tools/ksck.cc ---------------------------------------------------------------------- diff --git a/src/kudu/tools/ksck.cc b/src/kudu/tools/ksck.cc index 3a14f3e..764fd1d 100644 --- a/src/kudu/tools/ksck.cc +++ b/src/kudu/tools/ksck.cc @@ -123,43 +123,17 @@ tablet::TabletStatePB KsckTabletServer::ReplicaState(const std::string& tablet_i return tablet_status_map_.at(tablet_id).state(); } -KsckCluster::~KsckCluster() { -} - -Status KsckCluster::FetchTableAndTabletInfo() { - RETURN_NOT_OK(master_->Connect()); - RETURN_NOT_OK(RetrieveTablesList()); - RETURN_NOT_OK(RetrieveTabletServers()); - for (const shared_ptr<KsckTable>& table : tables()) { - RETURN_NOT_OK(RetrieveTabletsList(table)); - } - return Status::OK(); -} - -// Gets the list of tablet servers from the Master. -Status KsckCluster::RetrieveTabletServers() { - return master_->RetrieveTabletServers(&tablet_servers_); -} - -// Gets the list of tables from the Master. -Status KsckCluster::RetrieveTablesList() { - return master_->RetrieveTablesList(&tables_); -} - -Status KsckCluster::RetrieveTabletsList(const shared_ptr<KsckTable>& table) { - return master_->RetrieveTabletsList(table); -} - Ksck::Ksck(shared_ptr<KsckCluster> cluster, ostream* out) : cluster_(std::move(cluster)), out_(out == nullptr ? &std::cout : out) { } -Status Ksck::CheckMasterRunning() { - VLOG(1) << "Connecting to the Master"; - Status s = cluster_->master()->Connect(); +Status Ksck::CheckClusterRunning() { + DCHECK_NOTNULL(cluster_); + VLOG(1) << "Connecting to the leader master"; + Status s = cluster_->Connect(); if (s.ok()) { - Out() << "Connected to the Master" << endl; + Out() << "Connected to the leader master" << endl; } return s; } @@ -169,9 +143,9 @@ Status Ksck::FetchTableAndTabletInfo() { } Status Ksck::FetchInfoFromTabletServers() { - VLOG(1) << "Getting the Tablet Servers list"; + VLOG(1) << "Fetching the list of tablet servers"; int servers_count = cluster_->tablet_servers().size(); - VLOG(1) << Substitute("List of $0 Tablet Servers retrieved", servers_count); + VLOG(1) << Substitute("List of $0 tablet servers retrieved", servers_count); if (servers_count == 0) { return Status::NotFound("No tablet servers found"); @@ -188,8 +162,7 @@ Status Ksck::FetchInfoFromTabletServers() { vector<TabletServerSummary> tablet_server_summaries; simple_spinlock tablet_server_summaries_lock; - for (const KsckMaster::TSMap::value_type& entry : cluster_->tablet_servers()) { - + for (const KsckCluster::TSMap::value_type& entry : cluster_->tablet_servers()) { CHECK_OK(pool->SubmitFunc([&]() { Status s = ConnectToTabletServer(entry.second); TabletServerSummary summary; http://git-wip-us.apache.org/repos/asf/kudu/blob/7e35aee9/src/kudu/tools/ksck.h ---------------------------------------------------------------------- diff --git a/src/kudu/tools/ksck.h b/src/kudu/tools/ksck.h index c55b758..f0dcf5e 100644 --- a/src/kudu/tools/ksck.h +++ b/src/kudu/tools/ksck.h @@ -325,51 +325,37 @@ class KsckTabletServer { DISALLOW_COPY_AND_ASSIGN(KsckTabletServer); }; -// Class that must be extended to represent a master. -class KsckMaster { +// Class used to communicate with a cluster. +class KsckCluster { public: - // Map of KsckTabletServer objects keyed by tablet server permanent_uuid. + // Map of KsckTabletServer objects keyed by tablet server uuid. typedef std::unordered_map<std::string, std::shared_ptr<KsckTabletServer>> TSMap; - KsckMaster() { } - virtual ~KsckMaster() { } + // Fetches the lists of tables, tablets, and tablet servers from the master. + Status FetchTableAndTabletInfo() { + RETURN_NOT_OK(Connect()); + RETURN_NOT_OK(RetrieveTablesList()); + RETURN_NOT_OK(RetrieveTabletServers()); + for (const std::shared_ptr<KsckTable>& table : tables()) { + RETURN_NOT_OK(RetrieveTabletsList(table)); + } + return Status::OK(); + } - // Connects to the configured Master. + // Connects to the cluster (i.e. to the leader master). virtual Status Connect() = 0; - // Gets the list of Tablet Servers from the Master and stores it in the passed - // map, which is keyed on server permanent_uuid. - // 'tablet_servers' is only modified if this method returns OK. - virtual Status RetrieveTabletServers(TSMap* tablet_servers) = 0; + // Fetches the list of tablet servers. + virtual Status RetrieveTabletServers() = 0; - // Gets the list of tables from the Master and stores it in the passed vector. - // tables is only modified if this method returns OK. - virtual Status RetrieveTablesList(std::vector<std::shared_ptr<KsckTable>>* tables) = 0; + // Fetches the list of tables. + virtual Status RetrieveTablesList() = 0; - // Gets the list of tablets for the specified table and stores the list in it. - // The table's tablet list is only modified if this method returns OK. + // Fetches the list of tablets for the given table. + // The table's tablet list is modified only if this method returns OK. virtual Status RetrieveTabletsList(const std::shared_ptr<KsckTable>& table) = 0; - private: - DISALLOW_COPY_AND_ASSIGN(KsckMaster); -}; - -// Class used to communicate with the cluster. It bootstraps this by using the provided master. -class KsckCluster { - public: - explicit KsckCluster(std::shared_ptr<KsckMaster> master) - : master_(std::move(master)) {} - ~KsckCluster(); - - // Fetches list of tables, tablets, and tablet servers from the master and - // populates the full list in cluster_->tables(). - Status FetchTableAndTabletInfo(); - - const std::shared_ptr<KsckMaster>& master() { - return master_; - } - - const KsckMaster::TSMap& tablet_servers() { + const TSMap& tablet_servers() { return tablet_servers_; } @@ -377,21 +363,12 @@ class KsckCluster { return tables_; } - private: - friend class KsckTest; - - // Gets the list of tablet servers from the Master. - Status RetrieveTabletServers(); - - // Gets the list of tables from the Master. - Status RetrieveTablesList(); - - // Fetch the list of tablets for the given table from the Master. - Status RetrieveTabletsList(const std::shared_ptr<KsckTable>& table); - - const std::shared_ptr<KsckMaster> master_; - KsckMaster::TSMap tablet_servers_; + protected: + KsckCluster() = default; + TSMap tablet_servers_; std::vector<std::shared_ptr<KsckTable>> tables_; + + private: DISALLOW_COPY_AND_ASSIGN(KsckCluster); }; @@ -429,8 +406,9 @@ class Ksck { tablet_id_filters_ = std::move(tablet_ids); } - // Verifies that it can connect to the master. - Status CheckMasterRunning(); + // Verifies that it can connect to the cluster, i.e. that it can contact a + // leader master. + Status CheckClusterRunning(); // Populates all the cluster table and tablet info from the master. Status FetchTableAndTabletInfo(); http://git-wip-us.apache.org/repos/asf/kudu/blob/7e35aee9/src/kudu/tools/ksck_remote-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/tools/ksck_remote-test.cc b/src/kudu/tools/ksck_remote-test.cc index 370d0e2..f44aede 100644 --- a/src/kudu/tools/ksck_remote-test.cc +++ b/src/kudu/tools/ksck_remote-test.cc @@ -120,9 +120,8 @@ class RemoteKsckTest : public KuduTest { master_addresses.push_back( mini_cluster_->mini_master(i)->bound_rpc_addr_str()); } - std::shared_ptr<KsckMaster> master; - ASSERT_OK(RemoteKsckMaster::Build(master_addresses, &master)); - std::shared_ptr<KsckCluster> cluster(new KsckCluster(master)); + std::shared_ptr<KsckCluster> cluster; + ASSERT_OK(RemoteKsckCluster::Build(master_addresses, &cluster)); ksck_.reset(new Ksck(cluster, &err_stream_)); } @@ -216,17 +215,17 @@ class RemoteKsckTest : public KuduTest { }; TEST_F(RemoteKsckTest, TestMasterOk) { - ASSERT_OK(ksck_->CheckMasterRunning()); + ASSERT_OK(ksck_->CheckClusterRunning()); } TEST_F(RemoteKsckTest, TestTabletServersOk) { - ASSERT_OK(ksck_->CheckMasterRunning()); + ASSERT_OK(ksck_->CheckClusterRunning()); ASSERT_OK(ksck_->FetchTableAndTabletInfo()); ASSERT_OK(ksck_->FetchInfoFromTabletServers()); } TEST_F(RemoteKsckTest, TestTabletServerMismatchUUID) { - ASSERT_OK(ksck_->CheckMasterRunning()); + ASSERT_OK(ksck_->CheckClusterRunning()); ASSERT_OK(ksck_->FetchTableAndTabletInfo()); tserver::MiniTabletServer* tablet_server = mini_cluster_->mini_tablet_server(0); @@ -253,7 +252,7 @@ TEST_F(RemoteKsckTest, TestTableConsistency) { MonoTime deadline = MonoTime::Now() + MonoDelta::FromSeconds(30); Status s; while (MonoTime::Now() < deadline) { - ASSERT_OK(ksck_->CheckMasterRunning()); + ASSERT_OK(ksck_->CheckClusterRunning()); ASSERT_OK(ksck_->FetchTableAndTabletInfo()); ASSERT_OK(ksck_->FetchInfoFromTabletServers()); s = ksck_->CheckTablesConsistency(); @@ -273,7 +272,7 @@ TEST_F(RemoteKsckTest, TestChecksum) { MonoTime deadline = MonoTime::Now() + MonoDelta::FromSeconds(30); Status s; while (MonoTime::Now() < deadline) { - ASSERT_OK(ksck_->CheckMasterRunning()); + ASSERT_OK(ksck_->CheckClusterRunning()); ASSERT_OK(ksck_->FetchTableAndTabletInfo()); ASSERT_OK(ksck_->FetchInfoFromTabletServers()); @@ -298,7 +297,7 @@ TEST_F(RemoteKsckTest, TestChecksumTimeout) { uint64_t num_writes = 10000; LOG(INFO) << "Generating row writes..."; ASSERT_OK(GenerateRowWrites(num_writes)); - ASSERT_OK(ksck_->CheckMasterRunning()); + ASSERT_OK(ksck_->CheckClusterRunning()); ASSERT_OK(ksck_->FetchTableAndTabletInfo()); ASSERT_OK(ksck_->FetchInfoFromTabletServers()); // Use an impossibly low timeout value of zero! @@ -319,7 +318,7 @@ TEST_F(RemoteKsckTest, TestChecksumSnapshot) { CHECK(started_writing.WaitFor(MonoDelta::FromSeconds(30))); uint64_t ts = client_->GetLatestObservedTimestamp(); - ASSERT_OK(ksck_->CheckMasterRunning()); + ASSERT_OK(ksck_->CheckClusterRunning()); ASSERT_OK(ksck_->FetchTableAndTabletInfo()); ASSERT_OK(ksck_->FetchInfoFromTabletServers()); ASSERT_OK(ksck_->ChecksumData(ChecksumOptions(MonoDelta::FromSeconds(10), 16, true, ts))); @@ -342,7 +341,7 @@ TEST_F(RemoteKsckTest, TestChecksumSnapshotCurrentTimestamp) { &writer_thread); CHECK(started_writing.WaitFor(MonoDelta::FromSeconds(30))); - ASSERT_OK(ksck_->CheckMasterRunning()); + ASSERT_OK(ksck_->CheckClusterRunning()); ASSERT_OK(ksck_->FetchTableAndTabletInfo()); ASSERT_OK(ksck_->FetchInfoFromTabletServers()); ASSERT_OK(ksck_->ChecksumData(ChecksumOptions(MonoDelta::FromSeconds(10), 16, true, @@ -354,7 +353,7 @@ TEST_F(RemoteKsckTest, TestChecksumSnapshotCurrentTimestamp) { TEST_F(RemoteKsckTest, TestLeaderMasterDown) { // Make sure ksck's client is created with the current leader master. - ASSERT_OK(ksck_->CheckMasterRunning()); + ASSERT_OK(ksck_->CheckClusterRunning()); // Shut down the leader master. int leader_idx; http://git-wip-us.apache.org/repos/asf/kudu/blob/7e35aee9/src/kudu/tools/ksck_remote.cc ---------------------------------------------------------------------- diff --git a/src/kudu/tools/ksck_remote.cc b/src/kudu/tools/ksck_remote.cc index d571809..bbd015a 100644 --- a/src/kudu/tools/ksck_remote.cc +++ b/src/kudu/tools/ksck_remote.cc @@ -322,43 +322,43 @@ void RemoteKsckTabletServer::RunTabletChecksumScanAsync( ignore_result(stepper.release()); // Deletes self on callback. } -Status RemoteKsckMaster::Connect() { +Status RemoteKsckCluster::Connect() { KuduClientBuilder builder; builder.default_rpc_timeout(GetDefaultTimeout()); builder.master_server_addrs(master_addresses_); ReplicaController::SetVisibility(&builder, ReplicaController::Visibility::ALL); - client::sp::shared_ptr<KuduClient> client; return builder.Build(&client_); } -Status RemoteKsckMaster::Build(const vector<string>& master_addresses, - shared_ptr<KsckMaster>* master) { +Status RemoteKsckCluster::Build(const vector<string>& master_addresses, + shared_ptr<KsckCluster>* cluster) { CHECK(!master_addresses.empty()); shared_ptr<Messenger> messenger; MessengerBuilder builder(kMessengerName); RETURN_NOT_OK(builder.Build(&messenger)); - master->reset(new RemoteKsckMaster(master_addresses, messenger)); + cluster->reset(new RemoteKsckCluster(master_addresses, messenger)); return Status::OK(); } -Status RemoteKsckMaster::RetrieveTabletServers(TSMap* tablet_servers) { +Status RemoteKsckCluster::RetrieveTabletServers() { vector<KuduTabletServer*> servers; ElementDeleter deleter(&servers); RETURN_NOT_OK(client_->ListTabletServers(&servers)); - tablet_servers->clear(); + TSMap tablet_servers; for (const auto* s : servers) { shared_ptr<RemoteKsckTabletServer> ts( new RemoteKsckTabletServer(s->uuid(), HostPort(s->hostname(), s->port()), messenger_)); RETURN_NOT_OK(ts->Init()); - InsertOrDie(tablet_servers, ts->uuid(), ts); + InsertOrDie(&tablet_servers, ts->uuid(), ts); } + tablet_servers_.swap(tablet_servers); return Status::OK(); } -Status RemoteKsckMaster::RetrieveTablesList(vector<shared_ptr<KsckTable>>* tables) { +Status RemoteKsckCluster::RetrieveTablesList() { vector<string> table_names; RETURN_NOT_OK(client_->ListTables(&table_names)); @@ -372,11 +372,11 @@ Status RemoteKsckMaster::RetrieveTablesList(vector<shared_ptr<KsckTable>>* table t->num_replicas())); tables_temp.push_back(table); } - tables->assign(tables_temp.begin(), tables_temp.end()); + tables_.swap(tables_temp); return Status::OK(); } -Status RemoteKsckMaster::RetrieveTabletsList(const shared_ptr<KsckTable>& table) { +Status RemoteKsckCluster::RetrieveTabletsList(const shared_ptr<KsckTable>& table) { vector<shared_ptr<KsckTablet>> tablets; client::sp::shared_ptr<KuduTable> client_table; http://git-wip-us.apache.org/repos/asf/kudu/blob/7e35aee9/src/kudu/tools/ksck_remote.h ---------------------------------------------------------------------- diff --git a/src/kudu/tools/ksck_remote.h b/src/kudu/tools/ksck_remote.h index 37bac80..e265a27 100644 --- a/src/kudu/tools/ksck_remote.h +++ b/src/kudu/tools/ksck_remote.h @@ -24,7 +24,6 @@ #include <vector> #include "kudu/client/shared_ptr.h" -#include "kudu/gutil/port.h" #include "kudu/tools/ksck.h" #include "kudu/util/net/net_util.h" #include "kudu/util/status.h" @@ -80,7 +79,7 @@ class RemoteKsckTabletServer : public KsckTabletServer { const ChecksumOptions& options, ChecksumProgressCallbacks* callbacks) override; - virtual std::string address() const OVERRIDE { + virtual std::string address() const override { return host_port_.ToString(); } @@ -92,34 +91,30 @@ class RemoteKsckTabletServer : public KsckTabletServer { std::shared_ptr<consensus::ConsensusServiceProxy> consensus_proxy_; }; -// This implementation connects to a Master via RPC. -class RemoteKsckMaster : public KsckMaster { +// A KsckCluster that connects to a cluster via RPC. +class RemoteKsckCluster : public KsckCluster { public: static Status Build(const std::vector<std::string>& master_addresses, - std::shared_ptr<KsckMaster>* master); + std::shared_ptr<KsckCluster>* cluster); - virtual ~RemoteKsckMaster() { } + virtual Status Connect() override; - virtual Status Connect() OVERRIDE; + virtual Status RetrieveTabletServers() override; - virtual Status RetrieveTabletServers(TSMap* tablet_servers) OVERRIDE; + virtual Status RetrieveTablesList() override; - virtual Status RetrieveTablesList(std::vector<std::shared_ptr<KsckTable> >* tables) OVERRIDE; - - virtual Status RetrieveTabletsList(const std::shared_ptr<KsckTable>& table) OVERRIDE; + virtual Status RetrieveTabletsList(const std::shared_ptr<KsckTable>& table) override; private: - - RemoteKsckMaster(std::vector<std::string> master_addresses, - std::shared_ptr<rpc::Messenger> messenger) + RemoteKsckCluster(std::vector<std::string> master_addresses, + std::shared_ptr<rpc::Messenger> messenger) : master_addresses_(std::move(master_addresses)), messenger_(std::move(messenger)) { } const std::vector<std::string> master_addresses_; const std::shared_ptr<rpc::Messenger> messenger_; - client::sp::shared_ptr<client::KuduClient> client_; }; http://git-wip-us.apache.org/repos/asf/kudu/blob/7e35aee9/src/kudu/tools/tool_action_cluster.cc ---------------------------------------------------------------------- diff --git a/src/kudu/tools/tool_action_cluster.cc b/src/kudu/tools/tool_action_cluster.cc index d853c3c..dabb9cb 100644 --- a/src/kudu/tools/tool_action_cluster.cc +++ b/src/kudu/tools/tool_action_cluster.cc @@ -71,11 +71,9 @@ Status RunKsck(const RunnerContext& context) { const string& master_addresses_str = FindOrDie(context.required_args, kMasterAddressesArg); vector<string> master_addresses = strings::Split(master_addresses_str, ","); - shared_ptr<KsckMaster> master; - RETURN_NOT_OK_PREPEND(RemoteKsckMaster::Build(master_addresses, &master), - "unable to build KsckMaster"); - - shared_ptr<KsckCluster> cluster(new KsckCluster(master)); + shared_ptr<KsckCluster> cluster; + RETURN_NOT_OK_PREPEND(RemoteKsckCluster::Build(master_addresses, &cluster), + "unable to build KsckCluster"); shared_ptr<Ksck> ksck(new Ksck(cluster)); ksck->set_table_filters(strings::Split( @@ -83,7 +81,7 @@ Status RunKsck(const RunnerContext& context) { ksck->set_tablet_id_filters(strings::Split( FLAGS_tablets, ",", strings::SkipEmpty())); - RETURN_NOT_OK_PREPEND(ksck->CheckMasterRunning(), + RETURN_NOT_OK_PREPEND(ksck->CheckClusterRunning(), "master liveness check error"); RETURN_NOT_OK_PREPEND(ksck->FetchTableAndTabletInfo(), "error fetching the cluster metadata from the Master server"); http://git-wip-us.apache.org/repos/asf/kudu/blob/7e35aee9/src/kudu/tools/tool_action_tablet.cc ---------------------------------------------------------------------- diff --git a/src/kudu/tools/tool_action_tablet.cc b/src/kudu/tools/tool_action_tablet.cc index 1d197fb..17b0d27 100644 --- a/src/kudu/tools/tool_action_tablet.cc +++ b/src/kudu/tools/tool_action_tablet.cc @@ -136,16 +136,15 @@ Status GetTabletLeader(const client::sp::shared_ptr<KuduClient>& client, } Status DoKsckForTablet(const vector<string>& master_addresses, const string& tablet_id) { - shared_ptr<KsckMaster> master; - RETURN_NOT_OK(RemoteKsckMaster::Build(master_addresses, &master)); - shared_ptr<KsckCluster> cluster(new KsckCluster(master)); + shared_ptr<KsckCluster> cluster; + RETURN_NOT_OK(RemoteKsckCluster::Build(master_addresses, &cluster)); // Print to an unopened ofstream to discard ksck output. // See https://stackoverflow.com/questions/8243743. std::ofstream null_stream; Ksck ksck(cluster, &null_stream); ksck.set_tablet_id_filters({ tablet_id }); - RETURN_NOT_OK(ksck.CheckMasterRunning()); + RETURN_NOT_OK(ksck.CheckClusterRunning()); RETURN_NOT_OK(ksck.FetchTableAndTabletInfo()); // The return Status is ignored since a tserver that is not the destination // nor a host of a replica might be down, and in that case the move should
