You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2016/09/08 21:05:46 UTC

[3/3] kudu git commit: c++ client: expose private GetTablet API

c++ client: expose private GetTablet API

The new API can be used to look up tablet information (i.e. replicas and
roles) using a tablet ID.

I'm going to use it in kudu-admin; the "change config" operation needs to
know which tserver hosts the tablet's leader replica. Without this new API,
we'd have to go through the scan token API, which means either forcing the
user to also provide the table name, or abusing the scan token API by
creating tokens for _all_ tables, then hunting for the matching tablet.

As such, I've opted to make this API "private". I've done so via
KUDU_NO_EXPORT, though I could have just as easily declared it a private
method and used friendship. This way it can more easily be tested (as tests
do not use the "exported" client library).

Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5
Reviewed-on: http://gerrit.cloudera.org:8080/4179
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>


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

Branch: refs/heads/master
Commit: e33bac441c7d90fc22796b651622928d5043c66b
Parents: 1de4629
Author: Adar Dembo <ad...@cloudera.com>
Authored: Tue Aug 30 19:06:06 2016 -0700
Committer: Adar Dembo <ad...@cloudera.com>
Committed: Thu Sep 8 21:05:21 2016 +0000

----------------------------------------------------------------------
 src/kudu/client/client-internal.cc | 14 ++++++
 src/kudu/client/client-test.cc     | 37 ++++++++++++++++
 src/kudu/client/client.cc          | 77 +++++++++++++++++++++++++++++----
 src/kudu/client/client.h           | 22 ++++++++++
 4 files changed, 141 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/e33bac44/src/kudu/client/client-internal.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-internal.cc b/src/kudu/client/client-internal.cc
index 25ce831..2b0b51c 100644
--- a/src/kudu/client/client-internal.cc
+++ b/src/kudu/client/client-internal.cc
@@ -62,6 +62,8 @@ using master::DeleteTableResponsePB;
 using master::GetLeaderMasterRpc;
 using master::GetTableSchemaRequestPB;
 using master::GetTableSchemaResponsePB;
+using master::GetTabletLocationsRequestPB;
+using master::GetTabletLocationsResponsePB;
 using master::IsAlterTableDoneRequestPB;
 using master::IsAlterTableDoneResponsePB;
 using master::IsCreateTableDoneRequestPB;
@@ -232,6 +234,18 @@ template
 Status KuduClient::Data::SyncLeaderMasterRpc(
     const MonoTime& deadline,
     KuduClient* client,
+    const GetTabletLocationsRequestPB& req,
+    GetTabletLocationsResponsePB* resp,
+    const char* func_name,
+    const boost::function<Status(MasterServiceProxy*,
+                                 const GetTabletLocationsRequestPB&,
+                                 GetTabletLocationsResponsePB*,
+                                 RpcController*)>& func,
+    vector<uint32_t> required_feature_flags);
+template
+Status KuduClient::Data::SyncLeaderMasterRpc(
+    const MonoTime& deadline,
+    KuduClient* client,
     const ListTablesRequestPB& req,
     ListTablesResponsePB* resp,
     const char* func_name,

http://git-wip-us.apache.org/repos/asf/kudu/blob/e33bac44/src/kudu/client/client-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index c2306c9..ac50834 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -3825,5 +3825,42 @@ TEST_F(ClientTest, TestTableNumReplicas) {
   }
 }
 
+// Tests that KuduClient::GetTablet() returns the same tablet information as
+// KuduScanToken::tablet().
+TEST_F(ClientTest, TestGetTablet) {
+  KuduScanTokenBuilder builder(client_table_.get());
+  vector<KuduScanToken*> tokens;
+  ElementDeleter deleter(&tokens);
+  ASSERT_OK(builder.Build(&tokens));
+
+  ASSERT_EQ(2, tokens.size());
+  for (const auto* t : tokens) {
+    const KuduTablet& tablet = t->tablet();
+    ASSERT_EQ(1, tablet.replicas().size());
+    const KuduReplica* replica = tablet.replicas()[0];
+    ASSERT_TRUE(replica->is_leader());
+    const MiniTabletServer* ts = cluster_->mini_tablet_server(0);
+    ASSERT_EQ(ts->server()->instance_pb().permanent_uuid(),
+        replica->ts().uuid());
+    ASSERT_EQ(ts->bound_rpc_addr().host(), replica->ts().hostname());
+    ASSERT_EQ(ts->bound_rpc_addr().port(), replica->ts().port());
+
+    unique_ptr<KuduTablet> tablet_copy;
+    {
+      KuduTablet* ptr;
+      ASSERT_OK(client_->GetTablet(tablet.id(), &ptr));
+      tablet_copy.reset(ptr);
+    }
+    ASSERT_EQ(tablet.id(), tablet_copy->id());
+    ASSERT_EQ(1, tablet_copy->replicas().size());
+    const KuduReplica* replica_copy = tablet_copy->replicas()[0];
+
+    ASSERT_EQ(replica->is_leader(), replica_copy->is_leader());
+    ASSERT_EQ(replica->ts().uuid(), replica_copy->ts().uuid());
+    ASSERT_EQ(replica->ts().hostname(), replica_copy->ts().hostname());
+    ASSERT_EQ(replica->ts().port(), replica_copy->ts().port());
+  }
+}
+
 } // namespace client
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/e33bac44/src/kudu/client/client.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index dfefff8..4609f2f 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -72,6 +72,8 @@ using kudu::master::DeleteTableRequestPB;
 using kudu::master::DeleteTableResponsePB;
 using kudu::master::GetTableSchemaRequestPB;
 using kudu::master::GetTableSchemaResponsePB;
+using kudu::master::GetTabletLocationsRequestPB;
+using kudu::master::GetTabletLocationsResponsePB;
 using kudu::master::ListTablesRequestPB;
 using kudu::master::ListTablesResponsePB;
 using kudu::master::ListTabletServersRequestPB;
@@ -79,6 +81,7 @@ using kudu::master::ListTabletServersResponsePB;
 using kudu::master::ListTabletServersResponsePB_Entry;
 using kudu::master::MasterServiceProxy;
 using kudu::master::TabletLocationsPB;
+using kudu::master::TSInfoPB;
 using kudu::rpc::Messenger;
 using kudu::rpc::MessengerBuilder;
 using kudu::rpc::RpcController;
@@ -88,6 +91,7 @@ using std::set;
 using std::string;
 using std::unique_ptr;
 using std::vector;
+using strings::Substitute;
 
 MAKE_ENUM_LIMITS(kudu::client::KuduSession::FlushMode,
                  kudu::client::KuduSession::AUTO_FLUSH_SYNC,
@@ -394,6 +398,61 @@ shared_ptr<KuduSession> KuduClient::NewSession() {
   return ret;
 }
 
+Status KuduClient::GetTablet(const string& tablet_id, KuduTablet** tablet) {
+  GetTabletLocationsRequestPB req;
+  GetTabletLocationsResponsePB resp;
+
+  req.add_tablet_ids(tablet_id);
+  MonoTime deadline = MonoTime::Now() + default_admin_operation_timeout();
+  Status s =
+      data_->SyncLeaderMasterRpc<GetTabletLocationsRequestPB, GetTabletLocationsResponsePB>(
+          deadline,
+          this,
+          req,
+          &resp,
+          "GetTabletLocations",
+          &MasterServiceProxy::GetTabletLocations,
+          {});
+  RETURN_NOT_OK(s);
+  if (resp.has_error()) {
+    return StatusFromPB(resp.error().status());
+  }
+  if (resp.tablet_locations_size() != 1) {
+    return Status::IllegalState(Substitute(
+        "Expected only one tablet, but received $0",
+        resp.tablet_locations_size()));
+  }
+  const TabletLocationsPB& t = resp.tablet_locations(0);
+
+  vector<const KuduReplica*> replicas;
+  ElementDeleter deleter(&replicas);
+  for (const auto& r : t.replicas()) {
+    const TSInfoPB& ts_info = r.ts_info();
+    if (ts_info.rpc_addresses_size() == 0) {
+      return Status::IllegalState(Substitute(
+          "No RPC addresses found for tserver $0",
+          ts_info.permanent_uuid()));
+    }
+    HostPort hp;
+    RETURN_NOT_OK(HostPortFromPB(ts_info.rpc_addresses(0), &hp));
+    unique_ptr<KuduTabletServer> ts(new KuduTabletServer);
+    ts->data_ = new KuduTabletServer::Data(ts_info.permanent_uuid(), hp);
+
+    bool is_leader = r.role() == consensus::RaftPeerPB::LEADER;
+    unique_ptr<KuduReplica> replica(new KuduReplica);
+    replica->data_ = new KuduReplica::Data(is_leader, std::move(ts));
+
+    replicas.push_back(replica.release());
+  }
+
+  unique_ptr<KuduTablet> client_tablet(new KuduTablet);
+  client_tablet->data_ = new KuduTablet::Data(tablet_id, std::move(replicas));
+  replicas.clear();
+
+  *tablet = client_tablet.release();
+  return Status::OK();
+}
+
 bool KuduClient::IsMultiMaster() const {
   return data_->master_server_addrs_.size() > 1;
 }
@@ -572,8 +631,8 @@ Status KuduTableCreator::Create() {
                                                            *data_->schema_,
                                                            deadline,
                                                            !data_->range_partition_bounds_.empty()),
-                        strings::Substitute("Error creating table $0 on the master",
-                                            data_->table_name_));
+                        Substitute("Error creating table $0 on the master",
+                                   data_->table_name_));
 
   // Spin until the table is fully created, if requested.
   if (data_->wait_) {
@@ -1092,11 +1151,11 @@ struct CloseCallback {
 } // anonymous namespace
 
 string KuduScanner::ToString() const {
-  return strings::Substitute("$0: $1",
-                             data_->table_->name(),
-                             data_->configuration()
-                                   .spec()
-                                   .ToString(*data_->table_->schema().schema_));
+  return Substitute("$0: $1",
+                    data_->table_->name(),
+                    data_->configuration()
+                    .spec()
+                    .ToString(*data_->table_->schema().schema_));
 }
 
 Status KuduScanner::Open() {
@@ -1262,8 +1321,8 @@ Status KuduScanner::GetCurrentServer(KuduTabletServer** server) {
   vector<HostPort> host_ports;
   rts->GetHostPorts(&host_ports);
   if (host_ports.empty()) {
-    return Status::IllegalState(strings::Substitute("No HostPort found for RemoteTabletServer $0",
-                                                    rts->ToString()));
+    return Status::IllegalState(Substitute("No HostPort found for RemoteTabletServer $0",
+                                           rts->ToString()));
   }
   unique_ptr<KuduTabletServer> client_server(new KuduTabletServer);
   client_server->data_ = new KuduTabletServer::Data(rts->permanent_uuid(),

http://git-wip-us.apache.org/repos/asf/kudu/blob/e33bac44/src/kudu/client/client.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.h b/src/kudu/client/client.h
index 4b835f3..aa9c92c 100644
--- a/src/kudu/client/client.h
+++ b/src/kudu/client/client.h
@@ -70,6 +70,7 @@ class KuduStatusCallback;
 class KuduTable;
 class KuduTableAlterer;
 class KuduTableCreator;
+class KuduTablet;
 class KuduTabletServer;
 class KuduValue;
 class KuduWriteOperation;
@@ -345,6 +346,25 @@ class KUDU_EXPORT KuduClient : public sp::enable_shared_from_this<KuduClient> {
   /// @return A new session object; caller is responsible for destroying it.
   sp::shared_ptr<KuduSession> NewSession();
 
+  /// @cond false
+
+  /// Get tablet information for a tablet by ID.
+  ///
+  /// Private API.
+  ///
+  /// @todo This operation could benefit from the meta cache if it were
+  /// possible to look up using a tablet ID.
+  ///
+  /// @param [in] tablet_id
+  ///   Unique tablet identifier.
+  /// @param [out] tablet
+  ///   Tablet information. The caller takes ownership of the tablet.
+  /// @return Status object for the operation.
+  Status KUDU_NO_EXPORT GetTablet(const std::string& tablet_id,
+                                  KuduTablet** tablet);
+
+  /// @endcond
+
   /// Policy with which to choose amongst multiple replicas.
   enum ReplicaSelection {
     LEADER_ONLY,      ///< Select the LEADER replica.
@@ -477,6 +497,7 @@ class KUDU_EXPORT KuduReplica {
   const KuduTabletServer& ts() const;
 
  private:
+  friend class KuduClient;
   friend class KuduScanTokenBuilder;
 
   class KUDU_NO_EXPORT Data;
@@ -506,6 +527,7 @@ class KUDU_EXPORT KuduTablet {
   const std::vector<const KuduReplica*>& replicas() const;
 
  private:
+  friend class KuduClient;
   friend class KuduScanTokenBuilder;
 
   class KUDU_NO_EXPORT Data;