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

[1/3] kudu git commit: KUDU-687: expose additional tablet metadata in C++ client

Repository: kudu
Updated Branches:
  refs/heads/master 56aec48db -> f5021e061


KUDU-687: expose additional tablet metadata in C++ client

This patch adds new data-only KuduReplica and KuduTablet classes.
Along with KuduTabletServer, the C++ client now has more parity with the
Java client w.r.t. exposing tablet metadata.

For now, this is only exposed via KuduScanToken, because it's already doing
the work to figure out which replicas exist where. That's an odd fit for
ksck (which doesn't scan, at least not using the client), but it should
stave off any controversy stemming from adding dubious public APIs. In the
future, it wouldn't be unreasonable for these classes to be exposed via
KuduTable in some way.

There are four public signature changes:
- Addition of KuduScanToken::tablet(): this is backwards compatible.
- Addition of KuduTabletServer::port(): this is backwards compatible.
- Change to the KuduScanToken constructor: this should be backwards
  compatible, because it's private and so shouldn't used by applications.
- Removal of KuduScanToken::TabletServers(): this is an incompatible change.
  I think it's OK because Impala is the only significant C++ client user and
  it's not even using scan tokens yet.

Change-Id: I3cbbb1df8d3adf60425541b57e68595bbf6e92ff
Reviewed-on: http://gerrit.cloudera.org:8080/4146
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/6a92db4d
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/6a92db4d
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/6a92db4d

Branch: refs/heads/master
Commit: 6a92db4d08a635238c866df8ee6198f590347cda
Parents: 56aec48
Author: Adar Dembo <ad...@cloudera.com>
Authored: Fri Aug 26 19:25:56 2016 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Mon Aug 29 21:18:24 2016 +0000

----------------------------------------------------------------------
 docs/release_notes.adoc                   |   3 +
 src/kudu/client/CMakeLists.txt            |   2 +
 src/kudu/client/client.cc                 |  78 ++++++++++++---
 src/kudu/client/client.h                  | 131 +++++++++++++++++--------
 src/kudu/client/meta_cache.cc             |  14 ++-
 src/kudu/client/meta_cache.h              |   5 +-
 src/kudu/client/replica-internal.cc       |  38 +++++++
 src/kudu/client/replica-internal.h        |  42 ++++++++
 src/kudu/client/scan_token-internal.cc    |  63 ++++++++----
 src/kudu/client/scan_token-internal.h     |  16 +--
 src/kudu/client/scan_token-test.cc        |  49 +++++++--
 src/kudu/client/tablet-internal.cc        |  42 ++++++++
 src/kudu/client/tablet-internal.h         |  42 ++++++++
 src/kudu/client/tablet_server-internal.cc |   8 +-
 src/kudu/client/tablet_server-internal.h  |   5 +-
 15 files changed, 444 insertions(+), 94 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/6a92db4d/docs/release_notes.adoc
----------------------------------------------------------------------
diff --git a/docs/release_notes.adoc b/docs/release_notes.adoc
index b629a2f..4060bd6 100644
--- a/docs/release_notes.adoc
+++ b/docs/release_notes.adoc
@@ -49,6 +49,9 @@ detailed below.
 - KuduSession methods in the C++ library are no longer advertised as thread-safe
   to have one set of semantics for both C++ and Java Kudu client libraries.
 
+- The KuduScanToken::TabletServers method in the C++ library has been removed.
+  The same information can now be found in the KuduScanToken::tablet method.
+
 [[rn_0.10.0]]
 == Release notes specific to 0.10.0
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/6a92db4d/src/kudu/client/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/client/CMakeLists.txt b/src/kudu/client/CMakeLists.txt
index eb38862..d0ebf12 100644
--- a/src/kudu/client/CMakeLists.txt
+++ b/src/kudu/client/CMakeLists.txt
@@ -40,12 +40,14 @@ set(CLIENT_SRCS
   scan_predicate.cc
   scan_token-internal.cc
   scanner-internal.cc
+  replica-internal.cc
   resource_metrics.cc
   schema.cc
   session-internal.cc
   table-internal.cc
   table_alterer-internal.cc
   table_creator-internal.cc
+  tablet-internal.cc
   tablet_server-internal.cc
   value.cc
   write_op.cc

http://git-wip-us.apache.org/repos/asf/kudu/blob/6a92db4d/src/kudu/client/client.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index e7275a0..739c536 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -34,6 +34,7 @@
 #include "kudu/client/error-internal.h"
 #include "kudu/client/error_collector.h"
 #include "kudu/client/meta_cache.h"
+#include "kudu/client/replica-internal.h"
 #include "kudu/client/row_result.h"
 #include "kudu/client/scan_predicate-internal.h"
 #include "kudu/client/scan_token-internal.h"
@@ -43,6 +44,7 @@
 #include "kudu/client/table-internal.h"
 #include "kudu/client/table_alterer-internal.h"
 #include "kudu/client/table_creator-internal.h"
+#include "kudu/client/tablet-internal.h"
 #include "kudu/client/tablet_server-internal.h"
 #include "kudu/client/write_op.h"
 #include "kudu/common/common.pb.h"
@@ -313,10 +315,12 @@ Status KuduClient::ListTabletServers(vector<KuduTabletServer*>* tablet_servers)
   }
   for (int i = 0; i < resp.servers_size(); i++) {
     const ListTabletServersResponsePB_Entry& e = resp.servers(i);
-    auto ts = new KuduTabletServer();
+    HostPort hp;
+    RETURN_NOT_OK(HostPortFromPB(e.registration().rpc_addresses(0), &hp));
+    unique_ptr<KuduTabletServer> ts(new KuduTabletServer);
     ts->data_ = new KuduTabletServer::Data(e.instance_id().permanent_uuid(),
-                                           e.registration().rpc_addresses(0).host());
-    tablet_servers->push_back(ts);
+                                           hp);
+    tablet_servers->push_back(ts.release());
   }
   return Status::OK();
 }
@@ -1305,9 +1309,10 @@ Status KuduScanner::GetCurrentServer(KuduTabletServer** server) {
     return Status::IllegalState(strings::Substitute("No HostPort found for RemoteTabletServer $0",
                                                     rts->ToString()));
   }
-  *server = new KuduTabletServer();
-  (*server)->data_ = new KuduTabletServer::Data(rts->permanent_uuid(),
-                                                host_ports[0].host());
+  unique_ptr<KuduTabletServer> client_server(new KuduTabletServer);
+  client_server->data_ = new KuduTabletServer::Data(rts->permanent_uuid(),
+                                                    host_ports[0]);
+  *server = client_server.release();
   return Status::OK();
 }
 
@@ -1315,8 +1320,8 @@ Status KuduScanner::GetCurrentServer(KuduTabletServer** server) {
 // KuduScanToken
 ////////////////////////////////////////////////////////////
 
-KuduScanToken::KuduScanToken(KuduScanToken::Data* data)
-    : data_(data) {
+KuduScanToken::KuduScanToken()
+    : data_(nullptr) {
 }
 
 KuduScanToken::~KuduScanToken() {
@@ -1327,8 +1332,8 @@ Status KuduScanToken::IntoKuduScanner(KuduScanner** scanner) const {
   return data_->IntoKuduScanner(scanner);
 }
 
-const vector<KuduTabletServer*>& KuduScanToken::TabletServers() const {
-  return data_->TabletServers();
+const KuduTablet& KuduScanToken::tablet() const {
+  return data_->tablet();
 }
 
 Status KuduScanToken::Serialize(string* buf) const {
@@ -1336,9 +1341,10 @@ Status KuduScanToken::Serialize(string* buf) const {
 }
 
 Status KuduScanToken::DeserializeIntoScanner(KuduClient* client,
-                                         const string& serialized_token,
-                                         KuduScanner** scanner) {
-  return KuduScanToken::Data::DeserializeIntoScanner(client, serialized_token, scanner);
+                                             const string& serialized_token,
+                                             KuduScanner** scanner) {
+  return KuduScanToken::Data::DeserializeIntoScanner(
+      client, serialized_token, scanner);
 }
 
 ////////////////////////////////////////////////////////////
@@ -1416,6 +1422,46 @@ Status KuduScanTokenBuilder::Build(vector<KuduScanToken*>* tokens) {
 }
 
 ////////////////////////////////////////////////////////////
+// KuduReplica
+////////////////////////////////////////////////////////////
+
+KuduReplica::KuduReplica()
+  : data_(nullptr) {
+}
+
+KuduReplica::~KuduReplica() {
+  delete data_;
+}
+
+bool KuduReplica::is_leader() const {
+  return data_->is_leader_;
+}
+
+const KuduTabletServer& KuduReplica::ts() const {
+  return *data_->ts_;
+}
+
+////////////////////////////////////////////////////////////
+// KuduTablet
+////////////////////////////////////////////////////////////
+
+KuduTablet::KuduTablet()
+  : data_(nullptr) {
+}
+
+KuduTablet::~KuduTablet() {
+  delete data_;
+}
+
+const string& KuduTablet::id() const {
+  return data_->id_;
+}
+
+const vector<const KuduReplica*>& KuduTablet::replicas() const {
+  return data_->replicas_;
+}
+
+////////////////////////////////////////////////////////////
 // KuduTabletServer
 ////////////////////////////////////////////////////////////
 
@@ -1432,7 +1478,11 @@ const string& KuduTabletServer::uuid() const {
 }
 
 const string& KuduTabletServer::hostname() const {
-  return data_->hostname_;
+  return data_->hp_.host();
+}
+
+uint16_t KuduTabletServer::port() const {
+  return data_->hp_.port();
 }
 
 } // namespace client

http://git-wip-us.apache.org/repos/asf/kudu/blob/6a92db4d/src/kudu/client/client.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.h b/src/kudu/client/client.h
index f551765..2b33e9b 100644
--- a/src/kudu/client/client.h
+++ b/src/kudu/client/client.h
@@ -429,6 +429,94 @@ class KUDU_EXPORT KuduClient : public sp::enable_shared_from_this<KuduClient> {
   DISALLOW_COPY_AND_ASSIGN(KuduClient);
 };
 
+/// @brief In-memory representation of a remote tablet server.
+class KUDU_EXPORT KuduTabletServer {
+ public:
+  ~KuduTabletServer();
+
+  /// @return The UUID which is globally unique and guaranteed not to change
+  ///   for the lifetime of the tablet server.
+  const std::string& uuid() const;
+
+  /// @return Hostname of the first RPC address that this tablet server
+  ///   is listening on.
+  const std::string& hostname() const;
+
+  /// @return Port number of the first RPC address that this tablet server
+  ///   is listening on.
+  uint16_t port() const;
+
+ private:
+  class KUDU_NO_EXPORT Data;
+
+  friend class KuduClient;
+  friend class KuduScanner;
+  friend class KuduScanTokenBuilder;
+
+  KuduTabletServer();
+
+  // Owned.
+  Data* data_;
+
+  DISALLOW_COPY_AND_ASSIGN(KuduTabletServer);
+};
+
+/// @brief In-memory representation of a remote tablet's replica.
+class KUDU_EXPORT KuduReplica {
+ public:
+  ~KuduReplica();
+
+  /// @return Whether or not this replica is a Raft leader.
+  ///
+  /// @note This information may be stale; the role of a replica may change at
+  /// any time.
+  bool is_leader() const;
+
+  /// @return The tablet server hosting this remote replica.
+  const KuduTabletServer& ts() const;
+
+ private:
+  friend class KuduScanTokenBuilder;
+
+  class KUDU_NO_EXPORT Data;
+
+  KuduReplica();
+
+  // Owned.
+  Data* data_;
+
+  DISALLOW_COPY_AND_ASSIGN(KuduReplica);
+};
+
+/// @brief In-memory representation of a remote tablet.
+class KUDU_EXPORT KuduTablet {
+ public:
+  ~KuduTablet();
+
+  /// @return The ID which is globally unique and guaranteed not to change
+  ///    for the lifetime of the tablet.
+  const std::string& id() const;
+
+  /// @return The replicas of this tablet. The KuduTablet retains ownership
+  /// over the replicas.
+  ///
+  /// @note This information may be stale; replicas may be added or removed
+  /// from Raft configurations at any time.
+  const std::vector<const KuduReplica*>& replicas() const;
+
+ private:
+  friend class KuduScanTokenBuilder;
+
+  class KUDU_NO_EXPORT Data;
+
+  KuduTablet();
+
+  // Owned.
+  Data* data_;
+
+  DISALLOW_COPY_AND_ASSIGN(KuduTablet);
+};
+
 /// @brief A helper class to create a new table with the desired options.
 class KUDU_EXPORT KuduTableCreator {
  public:
@@ -1645,7 +1733,7 @@ class KUDU_EXPORT KuduScanner {
 /// instantiating the scanners on those nodes.
 ///
 /// Scan token locality information can be inspected using the
-/// KuduScanToken::TabletServers() method.
+/// KuduScanToken::tablet() function.
 class KUDU_EXPORT KuduScanToken {
  public:
 
@@ -1663,15 +1751,8 @@ class KUDU_EXPORT KuduScanToken {
   /// @return Operation result status.
   Status IntoKuduScanner(KuduScanner** scanner) const WARN_UNUSED_RESULT;
 
-  /// Get hint on candidate servers which may be hosting the source tablet.
-  ///
-  /// This method should be considered a hint, not a definitive answer,
-  /// since tablet to tablet server assignments may change in response to
-  /// external events such as failover or load balancing.
-  ///
-  /// @return Tablet servers who may be hosting the tablet which
-  ///   this scan is retrieving rows from.
-  const std::vector<KuduTabletServer*>& TabletServers() const;
+  /// @return Tablet that this scan will retrieve rows from.
+  const KuduTablet& tablet() const;
 
   /// Serialize the token into a string.
   ///
@@ -1702,7 +1783,7 @@ class KUDU_EXPORT KuduScanToken {
 
   friend class KuduScanTokenBuilder;
 
-  explicit KuduScanToken(Data* data);
+  KuduScanToken();
 
   // Owned.
   Data* data_;
@@ -1815,34 +1896,6 @@ class KUDU_EXPORT KuduScanTokenBuilder {
   DISALLOW_COPY_AND_ASSIGN(KuduScanTokenBuilder);
 };
 
-/// @brief In-memory representation of a remote tablet server.
-class KUDU_EXPORT KuduTabletServer {
- public:
-  ~KuduTabletServer();
-
-  /// @return The UUID which is globally unique and guaranteed not to change
-  ///   for the lifetime of the tablet server.
-  const std::string& uuid() const;
-
-  /// @return Hostname of the first RPC address that this tablet server
-  ///   is listening on.
-  const std::string& hostname() const;
-
- private:
-  class KUDU_NO_EXPORT Data;
-
-  friend class KuduClient;
-  friend class KuduScanner;
-  friend class KuduScanTokenBuilder;
-
-  KuduTabletServer();
-
-  // Owned.
-  Data* data_;
-
-  DISALLOW_COPY_AND_ASSIGN(KuduTabletServer);
-};
-
 } // namespace client
 } // namespace kudu
 #endif

http://git-wip-us.apache.org/repos/asf/kudu/blob/6a92db4d/src/kudu/client/meta_cache.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/meta_cache.cc b/src/kudu/client/meta_cache.cc
index 9ae6714..d3507ef 100644
--- a/src/kudu/client/meta_cache.cc
+++ b/src/kudu/client/meta_cache.cc
@@ -135,7 +135,7 @@ void RemoteTabletServer::Update(const master::TSInfoPB& pb) {
   }
 }
 
-string RemoteTabletServer::permanent_uuid() const {
+const string& RemoteTabletServer::permanent_uuid() const {
   return uuid_;
 }
 
@@ -231,6 +231,7 @@ bool RemoteTablet::HasLeader() const {
 }
 
 void RemoteTablet::GetRemoteTabletServers(vector<RemoteTabletServer*>* servers) const {
+  servers->clear();
   std::lock_guard<simple_spinlock> l(lock_);
   for (const RemoteReplica& replica : replicas_) {
     if (replica.failed) {
@@ -240,6 +241,17 @@ void RemoteTablet::GetRemoteTabletServers(vector<RemoteTabletServer*>* servers)
   }
 }
 
+void RemoteTablet::GetRemoteReplicas(vector<RemoteReplica>* replicas) const {
+  replicas->clear();
+  std::lock_guard<simple_spinlock> l(lock_);
+  for (const auto& r : replicas_) {
+    if (r.failed) {
+      continue;
+    }
+    replicas->push_back(r);
+  }
+}
+
 void RemoteTablet::MarkTServerAsLeader(const RemoteTabletServer* server) {
   bool found = false;
   std::lock_guard<simple_spinlock> l(lock_);

http://git-wip-us.apache.org/repos/asf/kudu/blob/6a92db4d/src/kudu/client/meta_cache.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/meta_cache.h b/src/kudu/client/meta_cache.h
index b621500..a46a8a8 100644
--- a/src/kudu/client/meta_cache.h
+++ b/src/kudu/client/meta_cache.h
@@ -92,7 +92,7 @@ class RemoteTabletServer {
   void GetHostPorts(std::vector<HostPort>* host_ports) const;
 
   // Returns the remote server's uuid.
-  std::string permanent_uuid() const;
+  const std::string& permanent_uuid() const;
 
  private:
   // Internal callback for DNS resolution.
@@ -214,6 +214,9 @@ class RemoteTablet : public RefCountedThreadSafe<RemoteTablet> {
   // failed replicas.
   void GetRemoteTabletServers(std::vector<RemoteTabletServer*>* servers) const;
 
+  // Writes this tablet's replicas to 'replicas'. Skips failed replicas.
+  void GetRemoteReplicas(std::vector<RemoteReplica>* replicas) const;
+
   // Return true if the tablet currently has a known LEADER replica
   // (i.e the next call to LeaderTServer() is likely to return non-NULL)
   bool HasLeader() const;

http://git-wip-us.apache.org/repos/asf/kudu/blob/6a92db4d/src/kudu/client/replica-internal.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/replica-internal.cc b/src/kudu/client/replica-internal.cc
new file mode 100644
index 0000000..0fac695
--- /dev/null
+++ b/src/kudu/client/replica-internal.cc
@@ -0,0 +1,38 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "kudu/client/replica-internal.h"
+
+#include <memory>
+
+#include "kudu/client/client.h"
+
+namespace kudu {
+namespace client {
+
+using std::unique_ptr;
+
+KuduReplica::Data::Data(bool is_leader, unique_ptr<KuduTabletServer> ts)
+    : is_leader_(is_leader),
+      ts_(std::move(ts)) {
+}
+
+KuduReplica::Data::~Data() {
+}
+
+} // namespace client
+} // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/6a92db4d/src/kudu/client/replica-internal.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/replica-internal.h b/src/kudu/client/replica-internal.h
new file mode 100644
index 0000000..9f0674d
--- /dev/null
+++ b/src/kudu/client/replica-internal.h
@@ -0,0 +1,42 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+#pragma once
+
+#include <memory>
+
+#include "kudu/client/client.h"
+#include "kudu/gutil/macros.h"
+
+
+namespace kudu {
+namespace client {
+
+class KuduTabletServer;
+
+class KuduReplica::Data {
+ public:
+  Data(bool is_leader, std::unique_ptr<KuduTabletServer> ts);
+  ~Data();
+
+  const bool is_leader_;
+  const std::unique_ptr<KuduTabletServer> ts_;
+
+  DISALLOW_COPY_AND_ASSIGN(Data);
+};
+
+} // namespace client
+} // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/6a92db4d/src/kudu/client/scan_token-internal.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/scan_token-internal.cc b/src/kudu/client/scan_token-internal.cc
index c724daf..a9bddd5 100644
--- a/src/kudu/client/scan_token-internal.cc
+++ b/src/kudu/client/scan_token-internal.cc
@@ -25,30 +25,33 @@
 #include "kudu/client/client-internal.h"
 #include "kudu/client/client.h"
 #include "kudu/client/meta_cache.h"
+#include "kudu/client/replica-internal.h"
 #include "kudu/client/scanner-internal.h"
+#include "kudu/client/tablet-internal.h"
 #include "kudu/client/tablet_server-internal.h"
 #include "kudu/common/wire_protocol.h"
 #include "kudu/gutil/stl_util.h"
+#include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/pb_util.h"
 #include "kudu/util/status.h"
 
 using std::string;
 using std::unique_ptr;
 using std::vector;
+using strings::Substitute;
 
 namespace kudu {
 namespace client {
 
 KuduScanToken::Data::Data(KuduTable* table,
                           ScanTokenPB message,
-                          vector<KuduTabletServer*> tablet_servers)
+                          unique_ptr<KuduTablet> tablet)
     : table_(table),
       message_(std::move(message)),
-      tablet_servers_(std::move(tablet_servers)) {
+      tablet_(std::move(tablet)) {
 }
 
 KuduScanToken::Data::~Data() {
-  ElementDeleter deleter(&tablet_servers_);
 }
 
 Status KuduScanToken::Data::IntoKuduScanner(KuduScanner** scanner) const {
@@ -96,7 +99,7 @@ Status KuduScanToken::Data::PBIntoScanner(KuduClient* client,
     }
     DataType expectedType = schema->column(columnIdx).type_info()->type();
     if (column.type() != expectedType) {
-      return Status::InvalidArgument(strings::Substitute(
+      return Status::InvalidArgument(Substitute(
             "invalid type $0 for column '$1' in scan token, expected: $2",
             column.type(), column.name(), expectedType));
     }
@@ -245,31 +248,49 @@ Status KuduScanTokenBuilder::Data::Build(vector<KuduScanToken*>* tokens) {
       continue;
     }
 
-    vector<internal::RemoteTabletServer*> remote_tablet_servers;
-    tablet->GetRemoteTabletServers(&remote_tablet_servers);
+    vector<internal::RemoteReplica> replicas;
+    tablet->GetRemoteReplicas(&replicas);
 
-    vector<KuduTabletServer*> tablet_servers;
-    ElementDeleter deleter(&tablet_servers);
+    vector<const KuduReplica*> client_replicas;
+    ElementDeleter deleter(&client_replicas);
 
-    for (internal::RemoteTabletServer* remote_tablet_server : remote_tablet_servers) {
+    // Convert the replicas from their internal format to something appropriate
+    // for clients.
+    for (const auto& r : replicas) {
       vector<HostPort> host_ports;
-      remote_tablet_server->GetHostPorts(&host_ports);
+      r.ts->GetHostPorts(&host_ports);
       if (host_ports.empty()) {
-        return Status::IllegalState(strings::Substitute("No host found for tablet server $0",
-                                                        remote_tablet_server->ToString()));
+        return Status::IllegalState(Substitute(
+            "No host found for tablet server $0", r.ts->ToString()));
       }
-      KuduTabletServer* tablet_server = new KuduTabletServer;
-      tablet_server->data_ = new KuduTabletServer::Data(remote_tablet_server->permanent_uuid(),
-                                                        host_ports[0].host());
-      tablet_servers.push_back(tablet_server);
+      unique_ptr<KuduTabletServer> client_ts(new KuduTabletServer);
+      client_ts->data_ = new KuduTabletServer::Data(r.ts->permanent_uuid(),
+                                                    host_ports[0]);
+      bool is_leader = r.role == consensus::RaftPeerPB::LEADER;
+      unique_ptr<KuduReplica> client_replica(new KuduReplica);
+      client_replica->data_ = new KuduReplica::Data(is_leader,
+                                                    std::move(client_ts));
+      client_replicas.push_back(client_replica.release());
     }
+
+    unique_ptr<KuduTablet> client_tablet(new KuduTablet);
+    client_tablet->data_ = new KuduTablet::Data(tablet->tablet_id(),
+                                                std::move(client_replicas));
+    client_replicas.clear();
+
+    // Create the scan token itself.
     ScanTokenPB message;
     message.CopyFrom(pb);
-    message.set_lower_bound_partition_key(tablet->partition().partition_key_start());
-    message.set_upper_bound_partition_key(tablet->partition().partition_key_end());
-    tokens->push_back(new KuduScanToken(new KuduScanToken::Data(table,
-                                                                std::move(message),
-                                                                std::move(tablet_servers))));
+    message.set_lower_bound_partition_key(
+        tablet->partition().partition_key_start());
+    message.set_upper_bound_partition_key(
+        tablet->partition().partition_key_end());
+    unique_ptr<KuduScanToken> client_scan_token(new KuduScanToken);
+    client_scan_token->data_ =
+        new KuduScanToken::Data(table,
+                                std::move(message),
+                                std::move(client_tablet));
+    tokens->push_back(client_scan_token.release());
     pruner.RemovePartitionKeyRange(tablet->partition().partition_key_end());
   }
   return Status::OK();

http://git-wip-us.apache.org/repos/asf/kudu/blob/6a92db4d/src/kudu/client/scan_token-internal.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/scan_token-internal.h b/src/kudu/client/scan_token-internal.h
index c2096a1..65bef42 100644
--- a/src/kudu/client/scan_token-internal.h
+++ b/src/kudu/client/scan_token-internal.h
@@ -17,8 +17,9 @@
 
 #pragma once
 
-#include <vector>
+#include <memory>
 #include <string>
+#include <vector>
 
 #include "kudu/client/client.h"
 #include "kudu/client/client.pb.h"
@@ -31,13 +32,13 @@ class KuduScanToken::Data {
  public:
   explicit Data(KuduTable* table,
                 ScanTokenPB message,
-                std::vector<KuduTabletServer*> tablet_servers);
+                std::unique_ptr<KuduTablet> tablet);
   ~Data();
 
   Status IntoKuduScanner(KuduScanner** scanner) const;
 
-  const std::vector<KuduTabletServer*>& TabletServers() const {
-    return tablet_servers_;
+  const KuduTablet& tablet() const {
+    return *tablet_;
   }
 
   Status Serialize(std::string* buf) const;
@@ -47,14 +48,13 @@ class KuduScanToken::Data {
                                        KuduScanner** scanner);
 
  private:
-
   static Status PBIntoScanner(KuduClient* client,
                               const ScanTokenPB& message,
                               KuduScanner** scanner);
 
-  KuduTable* table_;
-  ScanTokenPB message_;
-  std::vector<KuduTabletServer*> tablet_servers_;
+  const KuduTable* table_;
+  const ScanTokenPB message_;
+  const std::unique_ptr<KuduTablet> tablet_;
 };
 
 class KuduScanTokenBuilder::Data {

http://git-wip-us.apache.org/repos/asf/kudu/blob/6a92db4d/src/kudu/client/scan_token-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/scan_token-test.cc b/src/kudu/client/scan_token-test.cc
index 05bb93d..95de9f1 100644
--- a/src/kudu/client/scan_token-test.cc
+++ b/src/kudu/client/scan_token-test.cc
@@ -19,23 +19,27 @@
 #include <memory>
 #include <string>
 #include <thread>
+#include <unordered_set>
 #include <vector>
 
 #include "kudu/client/client.h"
 #include "kudu/gutil/stl_util.h"
 #include "kudu/integration-tests/mini_cluster.h"
+#include "kudu/tserver/mini_tablet_server.h"
+#include "kudu/tserver/tablet_server.h"
 #include "kudu/util/test_util.h"
 
+namespace kudu {
+namespace client {
+
+using sp::shared_ptr;
 using std::atomic;
 using std::string;
 using std::thread;
 using std::unique_ptr;
+using std::unordered_set;
 using std::vector;
-
-namespace kudu {
-namespace client {
-
-using sp::shared_ptr;
+using tserver::MiniTabletServer;
 
 class ScanTokenTest : public KuduTest {
 
@@ -101,6 +105,31 @@ class ScanTokenTest : public KuduTest {
     return rows;
   }
 
+  void VerifyTabletInfo(const vector<KuduScanToken*>& tokens) {
+    unordered_set<string> tablet_ids;
+    for (auto t : tokens) {
+      tablet_ids.insert(t->tablet().id());
+
+      // Check that there's only one replica; this is a non-replicated table.
+      ASSERT_EQ(1, t->tablet().replicas().size());
+
+      // Check that this replica is a leader; since there's only one tserver,
+      // it must be.
+      const KuduReplica* r = t->tablet().replicas()[0];
+      ASSERT_TRUE(r->is_leader());
+
+      // Check that the tserver associated with the replica is the sole tserver
+      // started for this cluster.
+      const MiniTabletServer* ts = cluster_->mini_tablet_server(0);
+      ASSERT_EQ(ts->server()->instance_pb().permanent_uuid(),
+                r->ts().uuid());
+      ASSERT_EQ(ts->bound_rpc_addr().host(), r->ts().hostname());
+      ASSERT_EQ(ts->bound_rpc_addr().port(), r->ts().port());
+    }
+    // Check that there are no duplicate tablet IDs.
+    ASSERT_EQ(tokens.size(), tablet_ids.size());
+  }
+
   shared_ptr<KuduClient> client_;
   gscoped_ptr<MiniCluster> cluster_;
 };
@@ -149,6 +178,7 @@ TEST_F(ScanTokenTest, TestScanTokens) {
 
     ASSERT_EQ(8, tokens.size());
     ASSERT_EQ(200, CountRows(tokens));
+    NO_FATALS(VerifyTabletInfo(tokens));
   }
 
   { // range predicate
@@ -163,6 +193,7 @@ TEST_F(ScanTokenTest, TestScanTokens) {
 
     ASSERT_EQ(4, tokens.size());
     ASSERT_EQ(100, CountRows(tokens));
+    NO_FATALS(VerifyTabletInfo(tokens));
   }
 
   { // equality predicate
@@ -177,6 +208,7 @@ TEST_F(ScanTokenTest, TestScanTokens) {
 
     ASSERT_EQ(1, tokens.size());
     ASSERT_EQ(1, CountRows(std::move(tokens)));
+    NO_FATALS(VerifyTabletInfo(tokens));
   }
 
   { // primary key bound
@@ -191,6 +223,7 @@ TEST_F(ScanTokenTest, TestScanTokens) {
 
     ASSERT_EQ(4, tokens.size());
     ASSERT_EQ(60, CountRows(tokens));
+    NO_FATALS(VerifyTabletInfo(tokens));
   }
 }
 
@@ -257,6 +290,7 @@ TEST_F(ScanTokenTest, TestScanTokensWithNonCoveringRange) {
 
     ASSERT_EQ(6, tokens.size());
     ASSERT_EQ(300, CountRows(tokens));
+    NO_FATALS(VerifyTabletInfo(tokens));
   }
 
   { // range predicate
@@ -271,6 +305,7 @@ TEST_F(ScanTokenTest, TestScanTokensWithNonCoveringRange) {
 
     ASSERT_EQ(4, tokens.size());
     ASSERT_EQ(200, CountRows(tokens));
+    NO_FATALS(VerifyTabletInfo(tokens));
   }
 
   { // equality predicate
@@ -284,7 +319,8 @@ TEST_F(ScanTokenTest, TestScanTokensWithNonCoveringRange) {
     ASSERT_OK(builder.Build(&tokens));
 
     ASSERT_EQ(1, tokens.size());
-    ASSERT_EQ(1, CountRows(std::move(tokens)));
+    ASSERT_EQ(1, CountRows(tokens));
+    NO_FATALS(VerifyTabletInfo(tokens));
   }
 
   { // primary key bound
@@ -299,6 +335,7 @@ TEST_F(ScanTokenTest, TestScanTokensWithNonCoveringRange) {
 
     ASSERT_EQ(2, tokens.size());
     ASSERT_EQ(40, CountRows(tokens));
+    NO_FATALS(VerifyTabletInfo(tokens));
   }
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/6a92db4d/src/kudu/client/tablet-internal.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/tablet-internal.cc b/src/kudu/client/tablet-internal.cc
new file mode 100644
index 0000000..06b49a0
--- /dev/null
+++ b/src/kudu/client/tablet-internal.cc
@@ -0,0 +1,42 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "kudu/client/tablet-internal.h"
+
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "kudu/gutil/stl_util.h"
+
+using std::string;
+using std::vector;
+
+namespace kudu {
+namespace client {
+
+KuduTablet::Data::Data(string id, vector<const KuduReplica*> replicas)
+    : id_(std::move(id)),
+      replicas_(std::move(replicas)) {
+}
+
+KuduTablet::Data::~Data() {
+  STLDeleteElements(&replicas_);
+}
+
+} // namespace client
+} // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/6a92db4d/src/kudu/client/tablet-internal.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/tablet-internal.h b/src/kudu/client/tablet-internal.h
new file mode 100644
index 0000000..401c725
--- /dev/null
+++ b/src/kudu/client/tablet-internal.h
@@ -0,0 +1,42 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+#pragma once
+
+#include <string>
+#include <vector>
+
+#include "kudu/client/client.h"
+#include "kudu/gutil/macros.h"
+
+namespace kudu {
+namespace client {
+
+class KuduReplica;
+
+class KuduTablet::Data {
+ public:
+  Data(std::string id, std::vector<const KuduReplica*> replicas);
+  ~Data();
+
+  const std::string id_;
+  std::vector<const KuduReplica*> replicas_;
+
+  DISALLOW_COPY_AND_ASSIGN(Data);
+};
+
+} // namespace client
+} // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/6a92db4d/src/kudu/client/tablet_server-internal.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/tablet_server-internal.cc b/src/kudu/client/tablet_server-internal.cc
index d5ad1c7..057fd2d 100644
--- a/src/kudu/client/tablet_server-internal.cc
+++ b/src/kudu/client/tablet_server-internal.cc
@@ -17,14 +17,18 @@
 
 #include "kudu/client/tablet_server-internal.h"
 
+#include <string>
+
+#include "kudu/util/net/net_util.h"
+
 using std::string;
 
 namespace kudu {
 namespace client {
 
-KuduTabletServer::Data::Data(string uuid, string hostname)
+KuduTabletServer::Data::Data(string uuid, HostPort hp)
     : uuid_(std::move(uuid)),
-      hostname_(std::move(hostname)) {
+      hp_(std::move(hp)) {
 }
 
 KuduTabletServer::Data::~Data() {

http://git-wip-us.apache.org/repos/asf/kudu/blob/6a92db4d/src/kudu/client/tablet_server-internal.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/tablet_server-internal.h b/src/kudu/client/tablet_server-internal.h
index edc903f..0e57ac4 100644
--- a/src/kudu/client/tablet_server-internal.h
+++ b/src/kudu/client/tablet_server-internal.h
@@ -21,17 +21,18 @@
 
 #include "kudu/client/client.h"
 #include "kudu/gutil/macros.h"
+#include "kudu/util/net/net_util.h"
 
 namespace kudu {
 namespace client {
 
 class KuduTabletServer::Data {
  public:
-  Data(std::string uuid, std::string hostname);
+  Data(std::string uuid, HostPort hp);
   ~Data();
 
   const std::string uuid_;
-  const std::string hostname_;
+  const HostPort hp_;
 
   DISALLOW_COPY_AND_ASSIGN(Data);
 };


[2/3] kudu git commit: KUDU-687: use client in ksck for master operations

Posted by to...@apache.org.
KUDU-687: use client in ksck for master operations

This patch modifies ksck to use the client for all master operations,
restricting direct access only for tserver operations. In doing so, ksck can
now work with multi-master clusters, retrying operations when the leader
master dies.

Interesting things of note:
- I went back and forth on what the semantics of KsckMaster::Connect() ought
  to be. At first I thought we should ping every master, but in the end I
  settled on building the client, which just verifies the existence of a
  leader master. My justification: today's ksck isn't really concerned with
  master health; the master merely provides information that is consumed
  during the real checks: of table and tablet integrity.
- I relented on commit cf009d4 and restored a MiniCluster method to find the
  leader master. It's got the same signature as the ExternalMiniCluster
  variant so that Mike's common cluster interface (a work in progress) can
  expose it.

Change-Id: Icbd6b28ea1b2c88e02cd451095e4dec94b0ebfc3
Reviewed-on: http://gerrit.cloudera.org:8080/4147
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/b5aa4a76
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/b5aa4a76
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/b5aa4a76

Branch: refs/heads/master
Commit: b5aa4a76c172f4cc2160c6cef7795977218b0d4c
Parents: 6a92db4
Author: Adar Dembo <ad...@cloudera.com>
Authored: Sat Aug 27 15:51:41 2016 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Mon Aug 29 21:18:28 2016 +0000

----------------------------------------------------------------------
 src/kudu/client/schema.h                       |   2 +
 src/kudu/integration-tests/cluster_verifier.cc |   4 +-
 src/kudu/integration-tests/mini_cluster.cc     |  45 ++++++-
 src/kudu/integration-tests/mini_cluster.h      |  23 ++--
 src/kudu/tools/CMakeLists.txt                  |   2 +-
 src/kudu/tools/ksck-test.cc                    |   4 +-
 src/kudu/tools/ksck.h                          |  10 +-
 src/kudu/tools/ksck_remote-test.cc             |  55 +++++---
 src/kudu/tools/ksck_remote.cc                  | 136 +++++++-------------
 src/kudu/tools/ksck_remote.h                   |  31 ++---
 src/kudu/tools/tool_action_cluster.cc          |  19 ++-
 11 files changed, 164 insertions(+), 167 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/b5aa4a76/src/kudu/client/schema.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/schema.h b/src/kudu/client/schema.h
index 099185a..2f85c56 100644
--- a/src/kudu/client/schema.h
+++ b/src/kudu/client/schema.h
@@ -32,6 +32,7 @@ class Schema;
 class TestWorkload;
 
 namespace tools {
+class RemoteKsckMaster;
 class TsAdminClient;
 }
 
@@ -488,6 +489,7 @@ class KUDU_EXPORT KuduSchema {
   friend class internal::LookupRpc;
   friend class internal::MetaCacheEntry;
   friend class internal::WriteRpc;
+  friend class kudu::tools::RemoteKsckMaster;
   friend class kudu::tools::TsAdminClient;
 
   friend KuduSchema KuduSchemaFromSchema(const Schema& schema);

http://git-wip-us.apache.org/repos/asf/kudu/blob/b5aa4a76/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 69c4195..c701779 100644
--- a/src/kudu/integration-tests/cluster_verifier.cc
+++ b/src/kudu/integration-tests/cluster_verifier.cc
@@ -77,10 +77,10 @@ void ClusterVerifier::CheckCluster() {
 }
 
 Status ClusterVerifier::DoKsck() {
-  Sockaddr addr = cluster_->leader_master()->bound_rpc_addr();
+  HostPort hp = cluster_->leader_master()->bound_rpc_hostport();
 
   std::shared_ptr<KsckMaster> master;
-  RETURN_NOT_OK(RemoteKsckMaster::Build(addr, &master));
+  RETURN_NOT_OK(RemoteKsckMaster::Build({ hp.ToString() }, &master));
   std::shared_ptr<KsckCluster> cluster(new KsckCluster(master));
   std::shared_ptr<Ksck> ksck(new Ksck(cluster));
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/b5aa4a76/src/kudu/integration-tests/mini_cluster.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/mini_cluster.cc b/src/kudu/integration-tests/mini_cluster.cc
index 626ed2c..4cf05e6 100644
--- a/src/kudu/integration-tests/mini_cluster.cc
+++ b/src/kudu/integration-tests/mini_cluster.cc
@@ -194,34 +194,34 @@ void MiniCluster::ShutdownMasters() {
   mini_masters_.clear();
 }
 
-MiniMaster* MiniCluster::mini_master(int idx) {
+MiniMaster* MiniCluster::mini_master(int idx) const {
   CHECK_GE(idx, 0) << "Master idx must be >= 0";
   CHECK_LT(idx, mini_masters_.size()) << "Master idx must be < num masters started";
   return mini_masters_[idx].get();
 }
 
-MiniTabletServer* MiniCluster::mini_tablet_server(int idx) {
+MiniTabletServer* MiniCluster::mini_tablet_server(int idx) const {
   CHECK_GE(idx, 0) << "TabletServer idx must be >= 0";
   CHECK_LT(idx, mini_tablet_servers_.size()) << "TabletServer idx must be < 'num_ts_started_'";
   return mini_tablet_servers_[idx].get();
 }
 
-string MiniCluster::GetMasterFsRoot(int idx) {
+string MiniCluster::GetMasterFsRoot(int idx) const {
   return JoinPathSegments(fs_root_, Substitute("master-$0-root", idx));
 }
 
-string MiniCluster::GetTabletServerFsRoot(int idx) {
+string MiniCluster::GetTabletServerFsRoot(int idx) const {
   return JoinPathSegments(fs_root_, Substitute("ts-$0-root", idx));
 }
 
-Status MiniCluster::WaitForTabletServerCount(int count) {
+Status MiniCluster::WaitForTabletServerCount(int count) const {
   vector<shared_ptr<master::TSDescriptor>> descs;
   return WaitForTabletServerCount(count, MatchMode::MATCH_TSERVERS, &descs);
 }
 
 Status MiniCluster::WaitForTabletServerCount(int count,
                                              MatchMode mode,
-                                             vector<shared_ptr<TSDescriptor>>* descs) {
+                                             vector<shared_ptr<TSDescriptor>>* descs) const {
   unordered_set<int> masters_to_search;
   for (int i = 0; i < num_masters(); i++) {
     if (!mini_master(i)->master()->IsShutdown()) {
@@ -278,7 +278,7 @@ Status MiniCluster::WaitForTabletServerCount(int count,
 }
 
 Status MiniCluster::CreateClient(KuduClientBuilder* builder,
-                                 client::sp::shared_ptr<KuduClient>* client) {
+                                 client::sp::shared_ptr<KuduClient>* client) const {
   KuduClientBuilder default_builder;
   if (builder == nullptr) {
     builder = &default_builder;
@@ -291,4 +291,35 @@ Status MiniCluster::CreateClient(KuduClientBuilder* builder,
   return builder->Build(client);
 }
 
+Status MiniCluster::GetLeaderMasterIndex(int* idx) const {
+  const MonoTime kDeadline = MonoTime::Now() + MonoDelta::FromSeconds(5);
+
+  int leader_idx = -1;
+  while (MonoTime::Now() < kDeadline) {
+    for (int i = 0; i < num_masters(); i++) {
+      if (mini_master(i)->master()->IsShutdown()) {
+        continue;
+      }
+      master::CatalogManager* catalog =
+          mini_master(i)->master()->catalog_manager();
+      master::CatalogManager::ScopedLeaderSharedLock l(catalog);
+      if (l.first_failed_status().ok()) {
+        leader_idx = i;
+        break;
+      }
+    }
+    if (leader_idx == -1) {
+      SleepFor(MonoDelta::FromMilliseconds(100));
+    } else {
+      break;
+    }
+  }
+  if (leader_idx == -1) {
+    return Status::NotFound("Leader master was not found within deadline");
+  }
+
+  *idx = leader_idx;
+  return Status::OK();
+}
+
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/b5aa4a76/src/kudu/integration-tests/mini_cluster.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/mini_cluster.h b/src/kudu/integration-tests/mini_cluster.h
index 8e64b3f..b54b17a 100644
--- a/src/kudu/integration-tests/mini_cluster.h
+++ b/src/kudu/integration-tests/mini_cluster.h
@@ -104,26 +104,26 @@ class MiniCluster {
   // If this cluster is configured for a single non-distributed
   // master, return the single master. Exits with a CHECK failure if
   // there are multiple masters.
-  master::MiniMaster* mini_master() {
+  master::MiniMaster* mini_master() const {
     CHECK_EQ(mini_masters_.size(), 1);
     return mini_master(0);
   }
 
   // Returns the Master at index 'idx' for this MiniCluster.
-  master::MiniMaster* mini_master(int idx);
+  master::MiniMaster* mini_master(int idx) const;
 
   // Return number of mini masters.
   int num_masters() const { return mini_masters_.size(); }
 
   // Returns the TabletServer at index 'idx' of this MiniCluster.
   // 'idx' must be between 0 and 'num_tablet_servers' -1.
-  tserver::MiniTabletServer* mini_tablet_server(int idx);
+  tserver::MiniTabletServer* mini_tablet_server(int idx) const;
 
   int num_tablet_servers() const { return mini_tablet_servers_.size(); }
 
-  std::string GetMasterFsRoot(int indx);
+  std::string GetMasterFsRoot(int indx) const;
 
-  std::string GetTabletServerFsRoot(int idx);
+  std::string GetTabletServerFsRoot(int idx) const;
 
   // Wait until the number of registered tablet servers reaches the given
   // count on all masters. Returns Status::TimedOut if the desired count is not
@@ -140,9 +140,9 @@ class MiniCluster {
     // Do not perform any matching on the retrieved tservers.
     DO_NOT_MATCH_TSERVERS,
   };
-  Status WaitForTabletServerCount(int count);
+  Status WaitForTabletServerCount(int count) const;
   Status WaitForTabletServerCount(int count, MatchMode mode,
-                                  std::vector<std::shared_ptr<master::TSDescriptor>>* descs);
+                                  std::vector<std::shared_ptr<master::TSDescriptor>>* descs) const;
 
   // Create a client configured to talk to this cluster. Builder may contain
   // override options for the client. The master address will be overridden to
@@ -151,7 +151,14 @@ class MiniCluster {
   //
   // REQUIRES: the cluster must have already been Start()ed.
   Status CreateClient(client::KuduClientBuilder* builder,
-                      client::sp::shared_ptr<client::KuduClient>* client);
+                      client::sp::shared_ptr<client::KuduClient>* client) const;
+
+  // Determine the leader master of the cluster. Sets 'idx' to the leader
+  // master's index (for calls to to mini_master()).
+  //
+  // Note: if a leader election occurs after this method is executed, the
+  // last result may not be valid.
+  Status GetLeaderMasterIndex(int* idx) const;
 
  private:
   enum {

http://git-wip-us.apache.org/repos/asf/kudu/blob/b5aa4a76/src/kudu/tools/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/tools/CMakeLists.txt b/src/kudu/tools/CMakeLists.txt
index fe95bad..05a72e3 100644
--- a/src/kudu/tools/CMakeLists.txt
+++ b/src/kudu/tools/CMakeLists.txt
@@ -110,7 +110,7 @@ set(KUDU_TEST_LINK_LIBS
   integration-tests
   ${KUDU_MIN_TEST_LIBS})
 ADD_KUDU_TEST(ksck-test)
-ADD_KUDU_TEST(ksck_remote-test)
+ADD_KUDU_TEST(ksck_remote-test RESOURCE_LOCK "master-rpc-ports")
 ADD_KUDU_TEST(kudu-admin-test)
 ADD_KUDU_TEST_DEPENDENCIES(kudu-admin-test
   kudu-admin)

http://git-wip-us.apache.org/repos/asf/kudu/blob/b5aa4a76/src/kudu/tools/ksck-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/ksck-test.cc b/src/kudu/tools/ksck-test.cc
index aae4656..cc2d4c7 100644
--- a/src/kudu/tools/ksck-test.cc
+++ b/src/kudu/tools/ksck-test.cc
@@ -85,7 +85,7 @@ class MockKsckMaster : public KsckMaster {
       : fetch_info_status_(Status::OK()) {
   }
 
-  virtual Status Connect() const OVERRIDE {
+  virtual Status Connect() OVERRIDE {
     return fetch_info_status_;
   }
 
@@ -218,7 +218,7 @@ class KsckTest : public KuduTest {
                            bool is_leader,
                            bool is_running) {
     shared_ptr<KsckTabletReplica> replica(new KsckTabletReplica(assignment_plan_.back(),
-                                                                is_leader, !is_leader));
+                                                                is_leader));
     shared_ptr<MockKsckTabletServer> ts = static_pointer_cast<MockKsckTabletServer>(
             master_->tablet_servers_.at(assignment_plan_.back()));
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/b5aa4a76/src/kudu/tools/ksck.h
----------------------------------------------------------------------
diff --git a/src/kudu/tools/ksck.h b/src/kudu/tools/ksck.h
index c512831..6a61b08 100644
--- a/src/kudu/tools/ksck.h
+++ b/src/kudu/tools/ksck.h
@@ -69,9 +69,8 @@ struct ChecksumOptions {
 // Representation of a tablet replica on a tablet server.
 class KsckTabletReplica {
  public:
-  KsckTabletReplica(const std::string ts_uuid, const bool is_leader, const bool is_follower)
+  KsckTabletReplica(const std::string ts_uuid, const bool is_leader)
       : is_leader_(is_leader),
-        is_follower_(is_follower),
         is_running_(false),
         ts_uuid_(ts_uuid) {
   }
@@ -80,17 +79,12 @@ class KsckTabletReplica {
     return is_leader_;
   }
 
-  const bool& is_follower() const {
-    return is_follower_;
-  }
-
   const std::string& ts_uuid() const {
     return ts_uuid_;
   }
 
  private:
   const bool is_leader_;
-  const bool is_follower_;
   bool is_running_;
   const std::string ts_uuid_;
   DISALLOW_COPY_AND_ASSIGN(KsckTabletReplica);
@@ -259,7 +253,7 @@ class KsckMaster {
   virtual ~KsckMaster() { }
 
   // Connects to the configured Master.
-  virtual Status Connect() const = 0;
+  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.

http://git-wip-us.apache.org/repos/asf/kudu/blob/b5aa4a76/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 7c8c303..b8857ce 100644
--- a/src/kudu/tools/ksck_remote-test.cc
+++ b/src/kudu/tools/ksck_remote-test.cc
@@ -24,11 +24,11 @@
 #include "kudu/tools/data_gen_util.h"
 #include "kudu/tools/ksck_remote.h"
 #include "kudu/util/monotime.h"
+#include "kudu/util/promise.h"
 #include "kudu/util/random.h"
 #include "kudu/util/test_util.h"
 
 DECLARE_int32(heartbeat_interval_ms);
-DECLARE_int32(tablets_batch_size_max);
 
 namespace kudu {
 namespace tools {
@@ -72,21 +72,20 @@ class RemoteKsckTest : public KuduTest {
     // Speed up testing, saves about 700ms per TEST_F.
     FLAGS_heartbeat_interval_ms = 10;
 
-    // Fetch the tablets in smaller batches to regression test a bug
-    // previously seen in the batching code.
-    FLAGS_tablets_batch_size_max = 5;
-
     MiniClusterOptions opts;
+
+    // Hard-coded ports for the masters. This is safe, as these tests run under
+    // a resource lock (see CMakeLists.txt in this directory).
+    // TODO we should have a generic method to obtain n free ports.
+    opts.master_rpc_ports = { 11010, 11011, 11012 };
+
+    opts.num_masters = opts.master_rpc_ports.size();
     opts.num_tablet_servers = 3;
     mini_cluster_.reset(new MiniCluster(env_.get(), opts));
     ASSERT_OK(mini_cluster_->Start());
 
-    master_rpc_addr_ = mini_cluster_->mini_master()->bound_rpc_addr();
-
     // Connect to the cluster.
-    ASSERT_OK(client::KuduClientBuilder()
-                     .add_master_server_addr(master_rpc_addr_.ToString())
-                     .Build(&client_));
+    ASSERT_OK(mini_cluster_->CreateClient(nullptr, &client_));
 
     // Create one table.
     gscoped_ptr<KuduTableCreator> table_creator(client_->NewTableCreator());
@@ -97,11 +96,18 @@ class RemoteKsckTest : public KuduTest {
                      .split_rows(GenerateSplitRows())
                      .Create());
     // Make sure we can open the table.
-    ASSERT_OK(client_->OpenTable(kTableName, &client_table_));
+    shared_ptr<KuduTable> client_table;
+    ASSERT_OK(client_->OpenTable(kTableName, &client_table));
 
-    ASSERT_OK(RemoteKsckMaster::Build(master_rpc_addr_, &master_));
-    cluster_.reset(new KsckCluster(master_));
-    ksck_.reset(new Ksck(cluster_));
+    vector<string> master_addresses;
+    for (int i = 0; i < mini_cluster_->num_masters(); i++) {
+        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));
+    ksck_.reset(new Ksck(cluster));
   }
 
   virtual void TearDown() OVERRIDE {
@@ -180,6 +186,7 @@ class RemoteKsckTest : public KuduTest {
     return Status::OK();
   }
 
+  std::shared_ptr<MiniCluster> mini_cluster_;
   std::shared_ptr<Ksck> ksck_;
   shared_ptr<client::KuduClient> client_;
 
@@ -187,12 +194,7 @@ class RemoteKsckTest : public KuduTest {
   std::stringstream err_stream_;
 
  private:
-  Sockaddr master_rpc_addr_;
-  std::shared_ptr<MiniCluster> mini_cluster_;
   client::KuduSchema schema_;
-  shared_ptr<client::KuduTable> client_table_;
-  std::shared_ptr<KsckMaster> master_;
-  std::shared_ptr<KsckCluster> cluster_;
   Random random_;
 };
 
@@ -318,5 +320,20 @@ TEST_F(RemoteKsckTest, DISABLED_TestChecksumSnapshotCurrentTimestamp) {
   writer_thread->Join();
 }
 
+TEST_F(RemoteKsckTest, TestLeaderMasterDown) {
+  // Make sure ksck's client is created with the current leader master.
+  ASSERT_OK(ksck_->CheckMasterRunning());
+
+  // Shut down the leader master.
+  int leader_idx;
+  ASSERT_OK(mini_cluster_->GetLeaderMasterIndex(&leader_idx));
+  mini_cluster_->mini_master(leader_idx)->Shutdown();
+
+  // Try to ksck. The underlying client will need to find the new leader master
+  // in order for the test to pass.
+  ASSERT_OK(ksck_->FetchTableAndTabletInfo());
+  ASSERT_OK(ksck_->FetchInfoFromTabletServers());
+}
+
 } // namespace tools
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/b5aa4a76/src/kudu/tools/ksck_remote.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/ksck_remote.cc b/src/kudu/tools/ksck_remote.cc
index 641271c..c2cc85c 100644
--- a/src/kudu/tools/ksck_remote.cc
+++ b/src/kudu/tools/ksck_remote.cc
@@ -17,9 +17,11 @@
 
 #include "kudu/tools/ksck_remote.h"
 
+#include "kudu/client/client.h"
 #include "kudu/common/schema.h"
 #include "kudu/common/wire_protocol.h"
 #include "kudu/gutil/map-util.h"
+#include "kudu/gutil/stl_util.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/gutil/strings/util.h"
 #include "kudu/util/net/net_util.h"
@@ -27,13 +29,19 @@
 
 DEFINE_bool(checksum_cache_blocks, false, "Should the checksum scanners cache the read blocks");
 DEFINE_int64(timeout_ms, 1000 * 60, "RPC timeout in milliseconds");
-DEFINE_int32(tablets_batch_size_max, 100, "How many tablets to get from the Master per RPC");
 
 namespace kudu {
 namespace tools {
 
 static const std::string kMessengerName = "ksck";
 
+using client::KuduClient;
+using client::KuduClientBuilder;
+using client::KuduReplica;
+using client::KuduScanToken;
+using client::KuduScanTokenBuilder;
+using client::KuduTable;
+using client::KuduTabletServer;
 using rpc::Messenger;
 using rpc::MessengerBuilder;
 using rpc::RpcController;
@@ -156,9 +164,9 @@ class ChecksumStepper {
       return; // Deletes 'this'.
     }
     if (resp_.has_resource_metrics() || resp_.has_rows_checksummed()) {
-      auto bytes = resp_.resource_metrics().cfile_cache_miss_bytes() +
+      int64_t bytes = resp_.resource_metrics().cfile_cache_miss_bytes() +
           resp_.resource_metrics().cfile_cache_hit_bytes();
-      callbacks_->Progress(resp_.rows_checksummed(), bytes);;
+      callbacks_->Progress(resp_.rows_checksummed(), bytes);
     }
     DCHECK(resp_.has_checksum());
     checksum_ = resp_.checksum();
@@ -251,35 +259,32 @@ void RemoteKsckTabletServer::RunTabletChecksumScanAsync(
   ignore_result(stepper.release()); // Deletes self on callback.
 }
 
-Status RemoteKsckMaster::Connect() const {
-  master::PingRequestPB req;
-  master::PingResponsePB resp;
-  RpcController rpc;
-  rpc.set_timeout(GetDefaultTimeout());
-  return proxy_->Ping(req, &resp, &rpc);
+Status RemoteKsckMaster::Connect() {
+  client::sp::shared_ptr<KuduClient> client;
+  KuduClientBuilder builder;
+  builder.master_server_addrs(master_addresses_);
+  return builder.Build(&client_);
 }
 
-Status RemoteKsckMaster::Build(const Sockaddr& address, shared_ptr<KsckMaster>* master) {
+Status RemoteKsckMaster::Build(const vector<string>& master_addresses,
+                               shared_ptr<KsckMaster>* master) {
   shared_ptr<Messenger> messenger;
   MessengerBuilder builder(kMessengerName);
   RETURN_NOT_OK(builder.Build(&messenger));
-  master->reset(new RemoteKsckMaster(address, messenger));
+  master->reset(new RemoteKsckMaster(master_addresses, messenger));
   return Status::OK();
 }
 
 Status RemoteKsckMaster::RetrieveTabletServers(TSMap* tablet_servers) {
-  master::ListTabletServersRequestPB req;
-  master::ListTabletServersResponsePB resp;
-  RpcController rpc;
+  vector<KuduTabletServer*> servers;
+  ElementDeleter deleter(&servers);
+  RETURN_NOT_OK(client_->ListTabletServers(&servers));
 
-  rpc.set_timeout(GetDefaultTimeout());
-  RETURN_NOT_OK(proxy_->ListTabletServers(req, &resp, &rpc));
   tablet_servers->clear();
-  for (const master::ListTabletServersResponsePB_Entry& e : resp.servers()) {
-    HostPortPB addr = e.registration().rpc_addresses(0);
+  for (const auto* s : servers) {
     shared_ptr<RemoteKsckTabletServer> ts(
-        new RemoteKsckTabletServer(e.instance_id().permanent_uuid(),
-                                   HostPort(addr.host(), addr.port()),
+        new RemoteKsckTabletServer(s->uuid(),
+                                   HostPort(s->hostname(), s->port()),
                                    messenger_));
     RETURN_NOT_OK(ts->Init());
     InsertOrDie(tablet_servers, ts->uuid(), ts);
@@ -288,21 +293,17 @@ Status RemoteKsckMaster::RetrieveTabletServers(TSMap* tablet_servers) {
 }
 
 Status RemoteKsckMaster::RetrieveTablesList(vector<shared_ptr<KsckTable>>* tables) {
-  master::ListTablesRequestPB req;
-  master::ListTablesResponsePB resp;
-  RpcController rpc;
-
-  rpc.set_timeout(GetDefaultTimeout());
-  RETURN_NOT_OK(proxy_->ListTables(req, &resp, &rpc));
-  if (resp.has_error()) {
-    return StatusFromPB(resp.error().status());
-  }
+  vector<string> table_names;
+  RETURN_NOT_OK(client_->ListTables(&table_names));
+
   vector<shared_ptr<KsckTable>> tables_temp;
-  for (const master::ListTablesResponsePB_TableInfo& info : resp.tables()) {
-    Schema schema;
-    int num_replicas;
-    RETURN_NOT_OK(GetTableInfo(info.name(), &schema, &num_replicas));
-    shared_ptr<KsckTable> table(new KsckTable(info.name(), schema, num_replicas));
+  for (const auto& n : table_names) {
+    client::sp::shared_ptr<KuduTable> t;
+    RETURN_NOT_OK(client_->OpenTable(n, &t));
+
+    shared_ptr<KsckTable> table(new KsckTable(n,
+                                              *t->schema().schema_,
+                                              t->num_replicas()));
     tables_temp.push_back(table);
   }
   tables->assign(tables_temp.begin(), tables_temp.end());
@@ -311,71 +312,28 @@ Status RemoteKsckMaster::RetrieveTablesList(vector<shared_ptr<KsckTable>>* table
 
 Status RemoteKsckMaster::RetrieveTabletsList(const shared_ptr<KsckTable>& table) {
   vector<shared_ptr<KsckTablet>> tablets;
-  bool more_tablets = true;
-  string next_key;
-  int retries = 0;
-  while (more_tablets) {
-    Status s = GetTabletsBatch(table, &next_key, tablets, &more_tablets);
-    if (s.IsServiceUnavailable() && retries++ < 25) {
-      SleepFor(MonoDelta::FromMilliseconds(100 * retries));
-    } else if (!s.ok()) {
-      return s;
-    }
-  }
-
-  table->set_tablets(tablets);
-  return Status::OK();
-}
 
-Status RemoteKsckMaster::GetTabletsBatch(const shared_ptr<KsckTable>& table,
-                                         string* next_partition_key,
-                                         vector<shared_ptr<KsckTablet>>& tablets,
-                                         bool* more_tablets) {
-  master::GetTableLocationsRequestPB req;
-  master::GetTableLocationsResponsePB resp;
-  RpcController rpc;
-
-  req.mutable_table()->set_table_name(table->name());
-  req.set_max_returned_locations(FLAGS_tablets_batch_size_max);
-  req.set_partition_key_start(*next_partition_key);
-
-  rpc.set_timeout(GetDefaultTimeout());
-  RETURN_NOT_OK(proxy_->GetTableLocations(req, &resp, &rpc));
-  for (const master::TabletLocationsPB& locations : resp.tablet_locations()) {
-    if (locations.partition().partition_key_start() < *next_partition_key) {
-      // We've already seen this partition.
-      continue;
-    }
+  client::sp::shared_ptr<KuduTable> client_table;
+  RETURN_NOT_OK(client_->OpenTable(table->name(), &client_table));
 
-    *next_partition_key = ImmediateSuccessor(locations.partition().partition_key_start());
+  vector<KuduScanToken*> tokens;
+  ElementDeleter deleter(&tokens);
 
-    shared_ptr<KsckTablet> tablet(new KsckTablet(table.get(), locations.tablet_id()));
+  KuduScanTokenBuilder builder(client_table.get());
+  RETURN_NOT_OK(builder.Build(&tokens));
+  for (const auto* t : tokens) {
+    shared_ptr<KsckTablet> tablet(
+        new KsckTablet(table.get(), t->tablet().id()));
     vector<shared_ptr<KsckTabletReplica>> replicas;
-    for (const master::TabletLocationsPB_ReplicaPB& replica : locations.replicas()) {
-      bool is_leader = replica.role() == consensus::RaftPeerPB::LEADER;
-      bool is_follower = replica.role() == consensus::RaftPeerPB::FOLLOWER;
+    for (const auto* r : t->tablet().replicas()) {
       replicas.push_back(shared_ptr<KsckTabletReplica>(
-          new KsckTabletReplica(replica.ts_info().permanent_uuid(), is_leader, is_follower)));
+          new KsckTabletReplica(r->ts().uuid(), r->is_leader())));
     }
     tablet->set_replicas(replicas);
     tablets.push_back(tablet);
   }
-  *more_tablets = resp.tablet_locations().size() == FLAGS_tablets_batch_size_max;
-  return Status::OK();
-}
 
-Status RemoteKsckMaster::GetTableInfo(const string& table_name, Schema* schema, int* num_replicas) {
-  master::GetTableSchemaRequestPB req;
-  master::GetTableSchemaResponsePB resp;
-  RpcController rpc;
-
-  req.mutable_table()->set_table_name(table_name);
-
-  rpc.set_timeout(GetDefaultTimeout());
-  RETURN_NOT_OK(proxy_->GetTableSchema(req, &resp, &rpc));
-
-  RETURN_NOT_OK(SchemaFromPB(resp.schema(), schema));
-  *num_replicas = resp.num_replicas();
+  table->set_tablets(tablets);
   return Status::OK();
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/b5aa4a76/src/kudu/tools/ksck_remote.h
----------------------------------------------------------------------
diff --git a/src/kudu/tools/ksck_remote.h b/src/kudu/tools/ksck_remote.h
index 8068bb0..ee049d6 100644
--- a/src/kudu/tools/ksck_remote.h
+++ b/src/kudu/tools/ksck_remote.h
@@ -22,8 +22,7 @@
 #include <string>
 #include <vector>
 
-#include "kudu/master/master.h"
-#include "kudu/master/master.proxy.h"
+#include "kudu/client/client.h"
 #include "kudu/rpc/messenger.h"
 #include "kudu/server/server_base.h"
 #include "kudu/server/server_base.proxy.h"
@@ -76,11 +75,12 @@ class RemoteKsckTabletServer : public KsckTabletServer {
 class RemoteKsckMaster : public KsckMaster {
  public:
 
-  static Status Build(const Sockaddr& address, std::shared_ptr<KsckMaster>* master);
+  static Status Build(const std::vector<std::string>& master_addresses,
+                      std::shared_ptr<KsckMaster>* master);
 
   virtual ~RemoteKsckMaster() { }
 
-  virtual Status Connect() const OVERRIDE;
+  virtual Status Connect() OVERRIDE;
 
   virtual Status RetrieveTabletServers(TSMap* tablet_servers) OVERRIDE;
 
@@ -90,25 +90,16 @@ class RemoteKsckMaster : public KsckMaster {
 
  private:
 
-  explicit RemoteKsckMaster(const Sockaddr& address,
-                            const std::shared_ptr<rpc::Messenger>& messenger)
-      : messenger_(messenger),
-        proxy_(new master::MasterServiceProxy(messenger, address)) {
+  RemoteKsckMaster(const std::vector<std::string>& master_addresses,
+                   const std::shared_ptr<rpc::Messenger>& messenger)
+      : master_addresses_(master_addresses),
+        messenger_(messenger) {
   }
 
-  Status GetTableInfo(const std::string& table_name, Schema* schema, int* num_replicas);
-
-  // Used to get a batch of tablets from the master, passing a pointer to the
-  // seen last key that will be used as the new start key. The
-  // last_partition_key is updated to point at the new last key that came in
-  // the batch.
-  Status GetTabletsBatch(const std::shared_ptr<KsckTable>& table,
-                         std::string* last_partition_key,
-                         std::vector<std::shared_ptr<KsckTablet> >& tablets,
-                         bool* more_tablets);
+  const std::vector<std::string> master_addresses_;
+  const std::shared_ptr<rpc::Messenger> messenger_;
 
-  std::shared_ptr<rpc::Messenger> messenger_;
-  std::shared_ptr<master::MasterServiceProxy> proxy_;
+  client::sp::shared_ptr<client::KuduClient> client_;
 };
 
 } // namespace tools

http://git-wip-us.apache.org/repos/asf/kudu/blob/b5aa4a76/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 93ed5db..d519b93 100644
--- a/src/kudu/tools/tool_action_cluster.cc
+++ b/src/kudu/tools/tool_action_cluster.cc
@@ -26,10 +26,8 @@
 
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/strings/split.h"
-#include "kudu/master/master.h"
 #include "kudu/tools/ksck.h"
 #include "kudu/tools/ksck_remote.h"
-#include "kudu/util/net/net_util.h"
 #include "kudu/util/status.h"
 
 #define PUSH_PREPEND_NOT_OK(s, statuses, msg) do { \
@@ -65,15 +63,11 @@ namespace tools {
 namespace {
 
 Status RunKsck(const RunnerContext& context) {
-  vector<Sockaddr> master_addrs;
-  string master_address = FindOrDie(context.required_args, "master_address");
-  RETURN_NOT_OK_PREPEND(ParseAddressList(master_address,
-                                         master::Master::kDefaultPort,
-                                         &master_addrs),
-                        "unable to parse master address");
-
+  string master_addresses_str = FindOrDie(context.required_args,
+                                          "master_addresses");
+  vector<string> master_addresses = strings::Split(master_addresses_str, ",");
   shared_ptr<KsckMaster> master;
-  RETURN_NOT_OK_PREPEND(RemoteKsckMaster::Build(master_addrs[0], &master),
+  RETURN_NOT_OK_PREPEND(RemoteKsckMaster::Build(master_addresses, &master),
                         "unable to build KsckMaster");
 
   shared_ptr<KsckCluster> cluster(new KsckCluster(master));
@@ -132,7 +126,10 @@ unique_ptr<Mode> BuildClusterMode() {
       "actively receiving inserts or updates.";
   unique_ptr<Action> ksck = ActionBuilder(
       { "ksck", desc }, &RunKsck)
-    .AddRequiredParameter({ "master_address", "Kudu Master RPC address of form hostname:port" })
+    .AddRequiredParameter({
+        "master_addresses",
+        "Comma-separated list of Kudu Master addressess where each address is "
+        "of form hostname:port" })
     .AddOptionalParameter("checksum_scan")
     .AddOptionalParameter("checksum_snapshot")
     .AddOptionalParameter("color")


[3/3] kudu git commit: cfile: replace DumpIteratorOptions with number of rows

Posted by to...@apache.org.
cfile: replace DumpIteratorOptions with number of rows

As of commit 9884fab, DumpIterator() doesn't print anything at all when
print_rows is false. That makes the option rather useless, so I'm replacing
it here with the raw number of rows.

Change-Id: I5d9e80e50926a71d22de4f88a7af6a8091bb2063
Reviewed-on: http://gerrit.cloudera.org:8080/4150
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/f5021e06
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/f5021e06
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/f5021e06

Branch: refs/heads/master
Commit: f5021e0619a838d94beea24be015952122712c2f
Parents: b5aa4a7
Author: Adar Dembo <ad...@cloudera.com>
Authored: Sun Aug 28 12:16:05 2016 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Mon Aug 29 21:19:46 2016 +0000

----------------------------------------------------------------------
 src/kudu/cfile/cfile-dump.cc |  9 +--------
 src/kudu/cfile/cfile_util.cc | 41 +++++++++++++++++++--------------------
 src/kudu/cfile/cfile_util.h  | 20 ++-----------------
 src/kudu/tools/fs_tool.cc    |  9 ++++-----
 4 files changed, 27 insertions(+), 52 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/f5021e06/src/kudu/cfile/cfile-dump.cc
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/cfile-dump.cc b/src/kudu/cfile/cfile-dump.cc
index 66c700f..2b279d0 100644
--- a/src/kudu/cfile/cfile-dump.cc
+++ b/src/kudu/cfile/cfile-dump.cc
@@ -28,7 +28,6 @@
 
 DEFINE_bool(print_meta, true, "print the header and footer from the file");
 DEFINE_bool(iterate_rows, true, "iterate each row in the file");
-DEFINE_bool(print_rows, true, "print each row in the file");
 DEFINE_int32(num_iterations, 1, "number of times to iterate the file");
 
 namespace kudu {
@@ -63,11 +62,9 @@ Status DumpFile(const string& block_id_str) {
     gscoped_ptr<CFileIterator> it;
     RETURN_NOT_OK(reader->NewIterator(&it, CFileReader::DONT_CACHE_BLOCK));
 
-    DumpIteratorOptions opts;
-    opts.print_rows = FLAGS_print_rows;
     for (int i = 0; i < FLAGS_num_iterations; i++) {
       RETURN_NOT_OK(it->SeekToFirst());
-      RETURN_NOT_OK(DumpIterator(*reader, it.get(), &cout, opts, 0));
+      RETURN_NOT_OK(DumpIterator(*reader, it.get(), &cout, 0, 0));
     }
   }
 
@@ -86,10 +83,6 @@ int main(int argc, char **argv) {
     return 1;
   }
 
-  if (!FLAGS_iterate_rows) {
-    FLAGS_print_rows = false;
-  }
-
   kudu::Status s = kudu::cfile::DumpFile(argv[1]);
   if (!s.ok()) {
     std::cerr << "Error: " << s.ToString() << std::endl;

http://git-wip-us.apache.org/repos/asf/kudu/blob/f5021e06/src/kudu/cfile/cfile_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/cfile_util.cc b/src/kudu/cfile/cfile_util.cc
index fa6b20e..393e45c 100644
--- a/src/kudu/cfile/cfile_util.cc
+++ b/src/kudu/cfile/cfile_util.cc
@@ -35,7 +35,7 @@ static const int kBufSize = 1024*1024;
 Status DumpIterator(const CFileReader& reader,
                     CFileIterator* it,
                     std::ostream* out,
-                    const DumpIteratorOptions& opts,
+                    int num_rows,
                     int indent) {
 
   Arena arena(8192, 8*1024*1024);
@@ -48,33 +48,32 @@ Status DumpIterator(const CFileReader& reader,
   string strbuf;
   size_t count = 0;
   while (it->HasNext()) {
-    size_t n = opts.nrows == 0 ? max_rows : std::min(max_rows, opts.nrows - count);
+    size_t n = num_rows == 0 ? max_rows : std::min(max_rows, num_rows - count);
     if (n == 0) break;
 
     RETURN_NOT_OK(it->CopyNextValues(&n, &cb));
 
-    if (opts.print_rows) {
-      if (reader.is_nullable()) {
-        for (size_t i = 0; i < n; i++) {
-          strbuf.append(indent, ' ');
-          const void *ptr = cb.nullable_cell_ptr(i);
-          if (ptr != nullptr) {
-            type->AppendDebugStringForValue(ptr, &strbuf);
-          } else {
-            strbuf.append("NULL");
-          }
-          strbuf.push_back('\n');
-        }
-      } else {
-        for (size_t i = 0; i < n; i++) {
-          strbuf.append(indent, ' ');
-          type->AppendDebugStringForValue(cb.cell_ptr(i), &strbuf);
-          strbuf.push_back('\n');
+    if (reader.is_nullable()) {
+      for (size_t i = 0; i < n; i++) {
+        strbuf.append(indent, ' ');
+        const void *ptr = cb.nullable_cell_ptr(i);
+        if (ptr != nullptr) {
+          type->AppendDebugStringForValue(ptr, &strbuf);
+        } else {
+          strbuf.append("NULL");
         }
+        strbuf.push_back('\n');
+      }
+    } else {
+      for (size_t i = 0; i < n; i++) {
+        strbuf.append(indent, ' ');
+        type->AppendDebugStringForValue(cb.cell_ptr(i), &strbuf);
+        strbuf.push_back('\n');
       }
-      *out << strbuf;
-      strbuf.clear();
     }
+
+    *out << strbuf;
+    strbuf.clear();
     arena.Reset();
     count += n;
   }

http://git-wip-us.apache.org/repos/asf/kudu/blob/f5021e06/src/kudu/cfile/cfile_util.h
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/cfile_util.h b/src/kudu/cfile/cfile_util.h
index 2022141..54a456b 100644
--- a/src/kudu/cfile/cfile_util.h
+++ b/src/kudu/cfile/cfile_util.h
@@ -78,28 +78,12 @@ struct ReaderOptions {
   std::shared_ptr<MemTracker> parent_mem_tracker;
 };
 
-struct DumpIteratorOptions {
-  // If true, print values of rows, otherwise only print aggregate
-  // information.
-  bool print_rows;
-
-  // Number of rows to iterate over. If 0, will iterate over all rows.
-  size_t nrows;
-
-  DumpIteratorOptions()
-      : print_rows(false), nrows(0) {
-  }
-};
-
 // Dumps the contents of a cfile to 'out'; 'reader' and 'iterator'
-// must be initialized. See cfile/cfile-dump.cc and tools/fs_tool.cc
-// for sample usage.
-//
-// See also: DumpIteratorOptions
+// must be initialized. If 'num_rows' is 0, all rows will be printed.
 Status DumpIterator(const CFileReader& reader,
                     CFileIterator* it,
                     std::ostream* out,
-                    const DumpIteratorOptions& opts,
+                    int num_rows,
                     int indent);
 
 // Return the length of the common prefix shared by the two strings.

http://git-wip-us.apache.org/repos/asf/kudu/blob/f5021e06/src/kudu/tools/fs_tool.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/fs_tool.cc b/src/kudu/tools/fs_tool.cc
index 63543bf..960a82b 100644
--- a/src/kudu/tools/fs_tool.cc
+++ b/src/kudu/tools/fs_tool.cc
@@ -50,7 +50,6 @@ namespace tools {
 using cfile::CFileIterator;
 using cfile::CFileReader;
 using cfile::DumpIterator;
-using cfile::DumpIteratorOptions;
 using cfile::ReaderOptions;
 using fs::ReadableBlock;
 using log::LogReader;
@@ -454,16 +453,16 @@ Status FsTool::DumpCFileBlockInternal(const BlockId& block_id,
 
   std::cout << Indent(indent) << "CFile Header: "
             << reader->header().ShortDebugString() << std::endl;
+  if (detail_level_ <= HEADERS_ONLY) {
+    return Status::OK();
+  }
   std::cout << Indent(indent) << reader->footer().num_values()
             << " values:" << std::endl;
 
   gscoped_ptr<CFileIterator> it;
   RETURN_NOT_OK(reader->NewIterator(&it, CFileReader::DONT_CACHE_BLOCK));
   RETURN_NOT_OK(it->SeekToFirst());
-  DumpIteratorOptions iter_opts;
-  iter_opts.nrows = opts.nrows;
-  iter_opts.print_rows = detail_level_ > HEADERS_ONLY;
-  return DumpIterator(*reader, it.get(), &std::cout, iter_opts, indent + 2);
+  return DumpIterator(*reader, it.get(), &std::cout, opts.nrows, indent + 2);
 }
 
 Status FsTool::DumpDeltaCFileBlockInternal(const Schema& schema,