kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject kudu git commit: remote_bootstrap_client: mild API changes
Date Thu, 04 Aug 2016 23:30:36 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 7ca727396 -> 993a686ee


remote_bootstrap_client: mild API changes

1. I removed the uuid argument from the constructor; it's always the uuid of
   the FsManager.
2. I made the TabletStatusListener argument to FetchAll() optional. I think
   that was the original intent (because the TabletMetadata OUT parameter in
   Start() is optional), but a CHECK_NOTNULL() got added at some point.
3. I removed the peer uuid argument from Start(). It was only used for
   logging, so I added an equivalent field to the response protocol.

Change-Id: Id7f482d1193a5a46fa51488eab4712c0c4bfceb1
Reviewed-on: http://gerrit.cloudera.org:8080/3811
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <todd@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/993a686e
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/993a686e
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/993a686e

Branch: refs/heads/master
Commit: 993a686ee04bfd4f9374c510e1084cb6fdd9e0ce
Parents: 7ca7273
Author: Adar Dembo <adar@cloudera.com>
Authored: Wed Jul 27 22:11:45 2016 -0700
Committer: Adar Dembo <adar@cloudera.com>
Committed: Thu Aug 4 23:29:53 2016 +0000

----------------------------------------------------------------------
 .../integration-tests/remote_bootstrap-itest.cc | 37 ++++++++++----------
 src/kudu/tserver/remote_bootstrap.proto         |  3 ++
 .../tserver/remote_bootstrap_client-test.cc     |  7 ++--
 src/kudu/tserver/remote_bootstrap_client.cc     | 23 ++++++------
 src/kudu/tserver/remote_bootstrap_client.h      |  9 ++---
 src/kudu/tserver/remote_bootstrap_service.cc    |  1 +
 src/kudu/tserver/ts_tablet_manager.cc           | 25 +++++++------
 7 files changed, 51 insertions(+), 54 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/993a686e/src/kudu/integration-tests/remote_bootstrap-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/remote_bootstrap-itest.cc b/src/kudu/integration-tests/remote_bootstrap-itest.cc
index 661ed21..9c9d0c7 100644
--- a/src/kudu/integration-tests/remote_bootstrap-itest.cc
+++ b/src/kudu/integration-tests/remote_bootstrap-itest.cc
@@ -290,25 +290,26 @@ TEST_F(RemoteBootstrapITest, TestDeleteTabletDuringRemoteBootstrap)
{
   ASSERT_OK(fs_manager->CreateInitialFileSystemLayout());
   ASSERT_OK(fs_manager->Open());
 
-  // Start up a RemoteBootstrapClient and open a remote bootstrap session.
-  gscoped_ptr<RemoteBootstrapClient> rb_client(
-      new RemoteBootstrapClient(tablet_id, fs_manager.get(),
-                                cluster_->messenger(), fs_manager->uuid()));
-  scoped_refptr<tablet::TabletMetadata> meta;
-  ASSERT_OK(rb_client->Start(cluster_->tablet_server(kTsIndex)->uuid(),
-                             cluster_->tablet_server(kTsIndex)->bound_rpc_hostport(),
-                             &meta));
-
-  // Tombstone the tablet on the remote!
-  ASSERT_OK(itest::DeleteTablet(ts, tablet_id, TABLET_DATA_TOMBSTONED, boost::none, timeout));
-
-  // Now finish bootstrapping!
-  tablet::TabletStatusListener listener(meta);
-  ASSERT_OK(rb_client->FetchAll(&listener));
-  ASSERT_OK(rb_client->Finish());
+  {
+    // Start up a RemoteBootstrapClient and open a remote bootstrap session.
+    RemoteBootstrapClient rb_client(tablet_id, fs_manager.get(),
+                                    cluster_->messenger());
+    scoped_refptr<tablet::TabletMetadata> meta;
+    ASSERT_OK(rb_client.Start(cluster_->tablet_server(kTsIndex)->bound_rpc_hostport(),
+                              &meta));
+
+    // Tombstone the tablet on the remote!
+    ASSERT_OK(itest::DeleteTablet(ts, tablet_id,
+                                  TABLET_DATA_TOMBSTONED, boost::none, timeout));
+
+    // Now finish bootstrapping!
+    tablet::TabletStatusListener listener(meta);
+    ASSERT_OK(rb_client.FetchAll(&listener));
+    ASSERT_OK(rb_client.Finish());
+
+    // Run destructor, which closes the remote session.
+  }
 
-  // Run destructor, which closes the remote session.
-  rb_client.reset();
   SleepFor(MonoDelta::FromMilliseconds(50));  // Give a little time for a crash (KUDU-1009).
   ASSERT_TRUE(cluster_->tablet_server(kTsIndex)->IsProcessAlive());
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/993a686e/src/kudu/tserver/remote_bootstrap.proto
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/remote_bootstrap.proto b/src/kudu/tserver/remote_bootstrap.proto
index 342765d..225f3a5 100644
--- a/src/kudu/tserver/remote_bootstrap.proto
+++ b/src/kudu/tserver/remote_bootstrap.proto
@@ -113,6 +113,9 @@ message BeginRemoteBootstrapSessionResponsePB {
   // A snapshot of the committed Consensus state at the time that the
   // remote bootstrap session was started.
   required consensus.ConsensusStatePB initial_committed_cstate = 5;
+
+  // permanent_uuid of the responding peer.
+  optional bytes responder_uuid = 6;
 }
 
 message CheckRemoteBootstrapSessionActiveRequestPB {

http://git-wip-us.apache.org/repos/asf/kudu/blob/993a686e/src/kudu/tserver/remote_bootstrap_client-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/remote_bootstrap_client-test.cc b/src/kudu/tserver/remote_bootstrap_client-test.cc
index 1c35ba1..101faa7 100644
--- a/src/kudu/tserver/remote_bootstrap_client-test.cc
+++ b/src/kudu/tserver/remote_bootstrap_client-test.cc
@@ -45,14 +45,13 @@ class RemoteBootstrapClientTest : public RemoteBootstrapTest {
     rpc::MessengerBuilder(CURRENT_TEST_NAME()).Build(&messenger_);
     client_.reset(new RemoteBootstrapClient(GetTabletId(),
                                             fs_manager_.get(),
-                                            messenger_,
-                                            fs_manager_->uuid()));
+                                            messenger_));
     ASSERT_OK(GetRaftConfigLeader(tablet_peer_->consensus()
         ->ConsensusState(consensus::CONSENSUS_CONFIG_COMMITTED), &leader_));
 
     HostPort host_port;
-    HostPortFromPB(leader_.last_known_addr(), &host_port);
-    ASSERT_OK(client_->Start(leader_.permanent_uuid(), host_port, &meta_));
+    ASSERT_OK(HostPortFromPB(leader_.last_known_addr(), &host_port));
+    ASSERT_OK(client_->Start(host_port, &meta_));
   }
 
  protected:

http://git-wip-us.apache.org/repos/asf/kudu/blob/993a686e/src/kudu/tserver/remote_bootstrap_client.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/remote_bootstrap_client.cc b/src/kudu/tserver/remote_bootstrap_client.cc
index acd86e1..3eba8db 100644
--- a/src/kudu/tserver/remote_bootstrap_client.cc
+++ b/src/kudu/tserver/remote_bootstrap_client.cc
@@ -93,12 +93,10 @@ using tablet::TabletSuperBlockPB;
 
 RemoteBootstrapClient::RemoteBootstrapClient(std::string tablet_id,
                                              FsManager* fs_manager,
-                                             shared_ptr<Messenger> messenger,
-                                             string client_permanent_uuid)
+                                             shared_ptr<Messenger> messenger)
     : tablet_id_(std::move(tablet_id)),
       fs_manager_(fs_manager),
       messenger_(std::move(messenger)),
-      permanent_uuid_(std::move(client_permanent_uuid)),
       started_(false),
       downloaded_wal_(false),
       downloaded_blocks_(false),
@@ -148,27 +146,26 @@ Status RemoteBootstrapClient::SetTabletToReplace(const scoped_refptr<TabletMetad
   return Status::OK();
 }
 
-Status RemoteBootstrapClient::Start(const string& bootstrap_peer_uuid,
-                                    const HostPort& bootstrap_peer_addr,
+Status RemoteBootstrapClient::Start(const HostPort& bootstrap_source_addr,
                                     scoped_refptr<TabletMetadata>* meta) {
   CHECK(!started_);
   start_time_micros_ = GetCurrentTimeMicros();
 
   Sockaddr addr;
-  RETURN_NOT_OK(SockaddrFromHostPort(bootstrap_peer_addr, &addr));
+  RETURN_NOT_OK(SockaddrFromHostPort(bootstrap_source_addr, &addr));
   if (addr.IsWildcard()) {
     return Status::InvalidArgument("Invalid wildcard address to remote bootstrap from",
                                    Substitute("$0 (resolved to $1)",
-                                              bootstrap_peer_addr.host(), addr.host()));
+                                              bootstrap_source_addr.host(), addr.host()));
   }
   LOG_WITH_PREFIX(INFO) << "Beginning remote bootstrap session"
-                        << " from remote peer at address " << bootstrap_peer_addr.ToString();
+                        << " from remote peer at address " << bootstrap_source_addr.ToString();
 
   // Set up an RPC proxy for the RemoteBootstrapService.
   proxy_.reset(new RemoteBootstrapServiceProxy(messenger_, addr));
 
   BeginRemoteBootstrapSessionRequestPB req;
-  req.set_requestor_uuid(permanent_uuid_);
+  req.set_requestor_uuid(fs_manager_->uuid());
   req.set_tablet_id(tablet_id_);
 
   rpc::RpcController controller;
@@ -180,7 +177,8 @@ Status RemoteBootstrapClient::Start(const string& bootstrap_peer_uuid,
   RETURN_NOT_OK_UNWIND_PREPEND(proxy_->BeginRemoteBootstrapSession(req, &resp, &controller),
                                controller,
                                "Unable to begin remote bootstrap session");
-
+  string bootstrap_peer_uuid = resp.has_responder_uuid()
+      ? resp.responder_uuid() : "(unknown uuid)";
   if (resp.superblock().tablet_data_state() != tablet::TABLET_DATA_READY) {
     Status s = Status::IllegalState("Remote peer (" + bootstrap_peer_uuid + ")" +
                                     " is currently remotely bootstrapping itself!",
@@ -250,7 +248,7 @@ Status RemoteBootstrapClient::Start(const string& bootstrap_peer_uuid,
 
 Status RemoteBootstrapClient::FetchAll(TabletStatusListener* status_listener) {
   CHECK(started_);
-  status_listener_ = CHECK_NOTNULL(status_listener);
+  status_listener_ = status_listener;
 
   // Download all the files (serially, for now, but in parallel in the future).
   RETURN_NOT_OK(DownloadBlocks());
@@ -557,7 +555,8 @@ Status RemoteBootstrapClient::VerifyData(uint64_t offset, const DataChunkPB&
chu
 }
 
 string RemoteBootstrapClient::LogPrefix() {
-  return Substitute("T $0 P $1: Remote bootstrap client: ", tablet_id_, permanent_uuid_);
+  return Substitute("T $0 P $1: Remote bootstrap client: ",
+                    tablet_id_, fs_manager_->uuid());
 }
 
 } // namespace tserver

http://git-wip-us.apache.org/repos/asf/kudu/blob/993a686e/src/kudu/tserver/remote_bootstrap_client.h
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/remote_bootstrap_client.h b/src/kudu/tserver/remote_bootstrap_client.h
index df95038..e461ec9 100644
--- a/src/kudu/tserver/remote_bootstrap_client.h
+++ b/src/kudu/tserver/remote_bootstrap_client.h
@@ -71,10 +71,8 @@ class RemoteBootstrapClient {
 
   // Construct the remote bootstrap client.
   // 'fs_manager' and 'messenger' must remain valid until this object is destroyed.
-  // 'client_permanent_uuid' is the permanent UUID of the caller server.
   RemoteBootstrapClient(std::string tablet_id, FsManager* fs_manager,
-                        std::shared_ptr<rpc::Messenger> messenger,
-                        std::string client_permanent_uuid);
+                        std::shared_ptr<rpc::Messenger> messenger);
 
   // Attempt to clean up resources on the remote end by sending an
   // EndRemoteBootstrapSession() RPC
@@ -96,9 +94,7 @@ class RemoteBootstrapClient {
   // in progress. If the 'metadata' pointer is passed as NULL, it is ignored,
   // otherwise the TabletMetadata object resulting from the initial remote
   // bootstrap response is returned.
-  // TODO: Rename these parameters to bootstrap_source_*.
-  Status Start(const std::string& bootstrap_peer_uuid,
-               const HostPort& bootstrap_peer_addr,
+  Status Start(const HostPort& bootstrap_source_addr,
                scoped_refptr<tablet::TabletMetadata>* metadata);
 
   // Runs a "full" remote bootstrap, copying the physical layout of a tablet
@@ -180,7 +176,6 @@ class RemoteBootstrapClient {
   const std::string tablet_id_;
   FsManager* const fs_manager_;
   const std::shared_ptr<rpc::Messenger> messenger_;
-  const std::string permanent_uuid_;
 
   // State flags that enforce the progress of remote bootstrap.
   bool started_;            // Session started.

http://git-wip-us.apache.org/repos/asf/kudu/blob/993a686e/src/kudu/tserver/remote_bootstrap_service.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/remote_bootstrap_service.cc b/src/kudu/tserver/remote_bootstrap_service.cc
index e6c99aa..a6ea959 100644
--- a/src/kudu/tserver/remote_bootstrap_service.cc
+++ b/src/kudu/tserver/remote_bootstrap_service.cc
@@ -141,6 +141,7 @@ void RemoteBootstrapServiceImpl::BeginRemoteBootstrapSession(
     ResetSessionExpirationUnlocked(session_id);
   }
 
+  resp->set_responder_uuid(fs_manager_->uuid());
   resp->set_session_id(session_id);
   resp->set_session_idle_timeout_millis(FLAGS_remote_bootstrap_idle_timeout_ms);
   resp->mutable_superblock()->CopyFrom(session->tablet_superblock());

http://git-wip-us.apache.org/repos/asf/kudu/blob/993a686e/src/kudu/tserver/ts_tablet_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/ts_tablet_manager.cc b/src/kudu/tserver/ts_tablet_manager.cc
index 072b028..729498e 100644
--- a/src/kudu/tserver/ts_tablet_manager.cc
+++ b/src/kudu/tserver/ts_tablet_manager.cc
@@ -338,9 +338,9 @@ Status TSTabletManager::StartRemoteBootstrap(
     const StartRemoteBootstrapRequestPB& req,
     boost::optional<TabletServerErrorPB::Code>* error_code) {
   const string& tablet_id = req.tablet_id();
-  const string& bootstrap_peer_uuid = req.bootstrap_peer_uuid();
-  HostPort bootstrap_peer_addr;
-  RETURN_NOT_OK(HostPortFromPB(req.bootstrap_peer_addr(), &bootstrap_peer_addr));
+  const string& bootstrap_source_uuid = req.bootstrap_peer_uuid();
+  HostPort bootstrap_source_addr;
+  RETURN_NOT_OK(HostPortFromPB(req.bootstrap_peer_addr(), &bootstrap_source_addr));
   int64_t leader_term = req.caller_term();
 
   const string kLogPrefix = LogPrefix(tablet_id);
@@ -410,19 +410,18 @@ Status TSTabletManager::StartRemoteBootstrap(
   }
 
   string init_msg = kLogPrefix + Substitute("Initiating remote bootstrap from Peer $0 ($1)",
-                                            bootstrap_peer_uuid, bootstrap_peer_addr.ToString());
+                                            bootstrap_source_uuid,
+                                            bootstrap_source_addr.ToString());
   LOG(INFO) << init_msg;
   TRACE(init_msg);
 
-  gscoped_ptr<RemoteBootstrapClient> rb_client(
-      new RemoteBootstrapClient(tablet_id, fs_manager_, server_->messenger(),
-                                fs_manager_->uuid()));
+  RemoteBootstrapClient rb_client(tablet_id, fs_manager_, server_->messenger());
 
   // Download and persist the remote superblock in TABLET_DATA_COPYING state.
   if (replacing_tablet) {
-    RETURN_NOT_OK(rb_client->SetTabletToReplace(meta, leader_term));
+    RETURN_NOT_OK(rb_client.SetTabletToReplace(meta, leader_term));
   }
-  RETURN_NOT_OK(rb_client->Start(bootstrap_peer_uuid, bootstrap_peer_addr, &meta));
+  RETURN_NOT_OK(rb_client.Start(bootstrap_source_addr, &meta));
 
   // From this point onward, the superblock is persisted in TABLET_DATA_COPYING
   // state, and we need to tombtone the tablet if additional steps prior to
@@ -431,18 +430,18 @@ Status TSTabletManager::StartRemoteBootstrap(
   // Registering a non-initialized TabletPeer offers visibility through the Web UI.
   RegisterTabletPeerMode mode = replacing_tablet ? REPLACEMENT_PEER : NEW_PEER;
   scoped_refptr<TabletPeer> tablet_peer = CreateAndRegisterTabletPeer(meta, mode);
-  string peer_str = bootstrap_peer_uuid + " (" + bootstrap_peer_addr.ToString() + ")";
+  string peer_str = bootstrap_source_uuid + " (" + bootstrap_source_addr.ToString() + ")";
 
   // Download all of the remote files.
-  TOMBSTONE_NOT_OK(rb_client->FetchAll(tablet_peer->status_listener()), meta,
+  TOMBSTONE_NOT_OK(rb_client.FetchAll(tablet_peer->status_listener()), meta,
                    "Remote bootstrap: Unable to fetch data from remote peer " +
-                   bootstrap_peer_uuid + " (" + bootstrap_peer_addr.ToString() + ")");
+                   bootstrap_source_uuid + " (" + bootstrap_source_addr.ToString() + ")");
 
   MAYBE_FAULT(FLAGS_fault_crash_after_rb_files_fetched);
 
   // Write out the last files to make the new replica visible and update the
   // TabletDataState in the superblock to TABLET_DATA_READY.
-  TOMBSTONE_NOT_OK(rb_client->Finish(), meta, "Remote bootstrap: Failure calling Finish()");
+  TOMBSTONE_NOT_OK(rb_client.Finish(), meta, "Remote bootstrap: Failure calling Finish()");
 
   // We run this asynchronously. We don't tombstone the tablet if this fails,
   // because if we were to fail to open the tablet, on next startup, it's in a


Mime
View raw message