You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2019/10/17 00:58:56 UTC

[kudu] branch branch-1.11.x updated (0d95261 -> 16bdf47)

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a change to branch branch-1.11.x
in repository https://gitbox.apache.org/repos/asf/kudu.git.


    from 0d95261  Bump version to 1.11.0 (non-SNAPSHOT)
     new 8899096  [clock] more info on refusal to advance hybrid timestamp
     new 068e3b4  ksck: print tablet server states
     new a9a5001  CapturingLogAppender: synchronize access to captured log text
     new 16bdf47  master: GetTableStatistics should use signed ints

The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../org/apache/kudu/spark/kudu/DefaultSource.scala |  6 ++-
 .../org/apache/kudu/test/CapturingLogAppender.java | 10 ++++-
 src/kudu/client/client-internal.cc                 | 19 +++++++++
 src/kudu/client/client-internal.h                  |  7 ++++
 src/kudu/client/client.cc                          | 12 +-----
 src/kudu/client/client.h                           |  2 +
 src/kudu/client/table_statistics-internal.h        |  6 +--
 src/kudu/clock/hybrid_clock.cc                     | 20 +++++----
 src/kudu/master/master.proto                       |  4 +-
 src/kudu/tools/ksck.cc                             |  7 ++++
 src/kudu/tools/ksck.h                              |  5 +++
 src/kudu/tools/ksck_remote.cc                      | 46 +++++++++++++++------
 src/kudu/tools/ksck_results.cc                     | 20 ++++++++-
 src/kudu/tools/ksck_results.h                      | 24 +++++++----
 src/kudu/tools/kudu-tool-test.cc                   | 47 +++++++++++++++++++++-
 15 files changed, 185 insertions(+), 50 deletions(-)


[kudu] 02/04: ksck: print tablet server states

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch branch-1.11.x
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 068e3b4545d91f7147ce8d1bd8ebbaaa9af5faa1
Author: Andrew Wong <aw...@apache.org>
AuthorDate: Mon Oct 14 20:11:01 2019 -0700

    ksck: print tablet server states
    
    This adds a table of tserver states to the ksck output:
    
    ...
    Tablet Server States
                  Server              |      State
    ----------------------------------+------------------
     767c081ea5564c9a8ed8615c00658e1b | MAINTENANCE_MODE
    
    Tablet Server Summary
    ...
    
    When there are no special tablet server states, the section doesn't get
    printed.
    
    I wanted to reuse some of the client code to list tablet servers but didn't
    want to change the public interface, so this patch also rejiggers the
    ListTabletServers client logic into the client data class.
    
    Change-Id: I4cec8c85a08bcb4f7be9e98a34d4b5ae2f05f3be
    Reviewed-on: http://gerrit.cloudera.org:8080/14436
    Reviewed-by: Adar Dembo <ad...@cloudera.com>
    Tested-by: Kudu Jenkins
    (cherry picked from commit b5716b71854cee432d188b46f5d66d200484f4f0)
    Reviewed-on: http://gerrit.cloudera.org:8080/14464
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
 src/kudu/client/client-internal.cc | 19 ++++++++++++++++
 src/kudu/client/client-internal.h  |  7 ++++++
 src/kudu/client/client.cc          | 12 +---------
 src/kudu/client/client.h           |  2 ++
 src/kudu/tools/ksck.cc             |  7 ++++++
 src/kudu/tools/ksck.h              |  5 +++++
 src/kudu/tools/ksck_remote.cc      | 46 ++++++++++++++++++++++++++++----------
 src/kudu/tools/ksck_results.cc     | 20 ++++++++++++++++-
 src/kudu/tools/ksck_results.h      | 24 ++++++++++++++------
 src/kudu/tools/kudu-tool-test.cc   | 26 +++++++++++++++++++++
 10 files changed, 137 insertions(+), 31 deletions(-)

diff --git a/src/kudu/client/client-internal.cc b/src/kudu/client/client-internal.cc
index befa03c..75c7200 100644
--- a/src/kudu/client/client-internal.cc
+++ b/src/kudu/client/client-internal.cc
@@ -99,6 +99,8 @@ using master::IsAlterTableDoneRequestPB;
 using master::IsAlterTableDoneResponsePB;
 using master::IsCreateTableDoneRequestPB;
 using master::IsCreateTableDoneResponsePB;
+using master::ListTabletServersResponsePB;
+using master::ListTabletServersRequestPB;
 using master::MasterFeatures;
 using master::MasterServiceProxy;
 using master::TableIdentifierPB;
@@ -446,6 +448,23 @@ bool KuduClient::Data::IsTabletServerLocal(const RemoteTabletServer& rts) const
   return false;
 }
 
+Status KuduClient::Data::ListTabletServers(KuduClient* client,
+                                           const MonoTime& deadline,
+                                           const ListTabletServersRequestPB& req,
+                                           ListTabletServersResponsePB* resp) const {
+  Synchronizer sync;
+  AsyncLeaderMasterRpc<ListTabletServersRequestPB, ListTabletServersResponsePB> rpc(
+      deadline, client, BackoffType::EXPONENTIAL, req, resp,
+      &MasterServiceProxy::ListTabletServersAsync, "ListTabletServers",
+      sync.AsStatusCallback(), {});
+  rpc.SendRpc();
+  RETURN_NOT_OK(sync.Wait());
+  if (resp->has_error()) {
+    return StatusFromPB(resp->error().status());
+  }
+  return Status::OK();
+}
+
 void KuduClient::Data::StoreAuthzToken(const string& table_id,
                                        const SignedTokenPB& token) {
   authz_token_cache_.Put(table_id, token);
diff --git a/src/kudu/client/client-internal.h b/src/kudu/client/client-internal.h
index be9d0ed..4d5a533 100644
--- a/src/kudu/client/client-internal.h
+++ b/src/kudu/client/client-internal.h
@@ -64,6 +64,8 @@ class ConnectToMasterResponsePB;
 class CreateTableRequestPB;
 class CreateTableResponsePB;
 class GetTableSchemaResponsePB;
+class ListTabletServersRequestPB;
+class ListTabletServersResponsePB;
 class MasterServiceProxy;
 class TableIdentifierPB;
 } // namespace master
@@ -169,6 +171,11 @@ class KuduClient::Data {
 
   bool IsTabletServerLocal(const internal::RemoteTabletServer& rts) const;
 
+  Status ListTabletServers(KuduClient* client,
+                           const MonoTime& deadline,
+                           const master::ListTabletServersRequestPB& req,
+                           master::ListTabletServersResponsePB* resp) const;
+
   // Returns a non-failed replica of the specified tablet based on the provided
   // selection criteria and tablet server blacklist.
   //
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index 28e67f9..93930d2 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -449,18 +449,8 @@ Status KuduClient::GetTableSchema(const string& table_name,
 Status KuduClient::ListTabletServers(vector<KuduTabletServer*>* tablet_servers) {
   ListTabletServersRequestPB req;
   ListTabletServersResponsePB resp;
-
   MonoTime deadline = MonoTime::Now() + default_admin_operation_timeout();
-  Synchronizer sync;
-  AsyncLeaderMasterRpc<ListTabletServersRequestPB, ListTabletServersResponsePB> rpc(
-      deadline, this, BackoffType::EXPONENTIAL, req, &resp,
-      &MasterServiceProxy::ListTabletServersAsync, "ListTabletServers",
-      sync.AsStatusCallback(), {});
-  rpc.SendRpc();
-  RETURN_NOT_OK(sync.Wait());
-  if (resp.has_error()) {
-    return StatusFromPB(resp.error().status());
-  }
+  RETURN_NOT_OK(data_->ListTabletServers(this, deadline, req, &resp));
   for (int i = 0; i < resp.servers_size(); i++) {
     const ListTabletServersResponsePB_Entry& e = resp.servers(i);
     HostPort hp;
diff --git a/src/kudu/client/client.h b/src/kudu/client/client.h
index 2412f8b..f4dc618 100644
--- a/src/kudu/client/client.h
+++ b/src/kudu/client/client.h
@@ -65,6 +65,7 @@ class KuduTable;
 
 namespace tools {
 class LeaderMasterProxy;
+class RemoteKsckCluster;
 } // namespace tools
 
 namespace client {
@@ -612,6 +613,7 @@ class KUDU_EXPORT KuduClient : public sp::enable_shared_from_this<KuduClient> {
   friend class kudu::AuthzTokenTest;
   friend class kudu::SecurityUnknownTskTest;
   friend class tools::LeaderMasterProxy;
+  friend class tools::RemoteKsckCluster;
 
   FRIEND_TEST(kudu::ClientStressTest, TestUniqueClientIds);
   FRIEND_TEST(ClientTest, TestCacheAuthzTokens);
diff --git a/src/kudu/tools/ksck.cc b/src/kudu/tools/ksck.cc
index 6c39b7f..e8b61bc 100644
--- a/src/kudu/tools/ksck.cc
+++ b/src/kudu/tools/ksck.cc
@@ -420,6 +420,9 @@ void Ksck::set_print_sections(const std::vector<std::string>& sections) {
     if (section_upper == "MASTER_SUMMARIES") {
       print_sections_flags_ |= PrintSections::MASTER_SUMMARIES;
     }
+    if (section_upper == "TSERVER_STATES") {
+      print_sections_flags_ |= PrintSections::TSERVER_STATES;
+    }
     if (section_upper == "TSERVER_SUMMARIES") {
       print_sections_flags_ |= PrintSections::TSERVER_SUMMARIES;
     }
@@ -461,6 +464,10 @@ Status Ksck::Run() {
   PUSH_PREPEND_NOT_OK(s, results_.error_messages, fetch_prefix);
   RETURN_NOT_OK_PREPEND(s, fetch_prefix);
 
+  // In getting table and tablet info, we should have also received info about
+  // the tablet servers, including any special states they might be in.
+  results_.ts_states = cluster_->ts_states();
+
   PUSH_PREPEND_NOT_OK(FetchInfoFromTabletServers(), results_.error_messages,
                       "error fetching info from tablet servers");
   PUSH_PREPEND_NOT_OK(CheckTabletServerUnusualFlags(), results_.warning_messages,
diff --git a/src/kudu/tools/ksck.h b/src/kudu/tools/ksck.h
index 71aa2b7..b631d1a 100644
--- a/src/kudu/tools/ksck.h
+++ b/src/kudu/tools/ksck.h
@@ -437,6 +437,10 @@ class KsckCluster {
     return tablet_servers_;
   }
 
+  const KsckTServerStateMap& ts_states() const {
+    return ts_states_;
+  }
+
   const std::vector<std::shared_ptr<KsckTable>>& tables() const {
     return tables_;
   }
@@ -478,6 +482,7 @@ class KsckCluster {
   KsckCluster() : filtered_tables_count_(0), filtered_tablets_count_(0) {}
   MasterList masters_;
   TSMap tablet_servers_;
+  KsckTServerStateMap ts_states_;
   std::vector<std::shared_ptr<KsckTable>> tables_;
   std::unique_ptr<ThreadPool> pool_;
 
diff --git a/src/kudu/tools/ksck_remote.cc b/src/kudu/tools/ksck_remote.cc
index b5b2912..d9a4c46 100644
--- a/src/kudu/tools/ksck_remote.cc
+++ b/src/kudu/tools/ksck_remote.cc
@@ -31,6 +31,7 @@
 #include <gflags/gflags_declare.h>
 #include <glog/logging.h>
 
+#include "kudu/client/client-internal.h"
 #include "kudu/client/client.h"
 #include "kudu/client/replica_controller-internal.h"
 #include "kudu/client/schema.h"
@@ -47,6 +48,7 @@
 #include "kudu/gutil/stl_util.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/master/master.h"
+#include "kudu/master/master.pb.h"
 #include "kudu/master/sys_catalog.h"
 #include "kudu/rebalance/cluster_status.h"
 #include "kudu/rpc/messenger.h"
@@ -57,6 +59,7 @@
 #include "kudu/tablet/tablet.pb.h"
 #include "kudu/tools/ksck.h"
 #include "kudu/tools/ksck_checksum.h"
+#include "kudu/tools/ksck_results.h"
 #include "kudu/tools/tool_action_common.h"
 #include "kudu/tserver/tablet_server.h"
 #include "kudu/tserver/tserver.pb.h"
@@ -79,9 +82,11 @@ using kudu::client::KuduScanToken;
 using kudu::client::KuduScanTokenBuilder;
 using kudu::client::KuduSchema;
 using kudu::client::KuduTable;
-using kudu::client::KuduTabletServer;
 using kudu::client::internal::ReplicaController;
 using kudu::cluster_summary::ServerHealth;
+using kudu::master::ListTabletServersRequestPB;
+using kudu::master::ListTabletServersResponsePB;
+using kudu::master::TServerStatePB;
 using kudu::rpc::Messenger;
 using kudu::rpc::MessengerBuilder;
 using kudu::rpc::RpcController;
@@ -494,21 +499,38 @@ Status RemoteKsckCluster::Build(const vector<string>& master_addresses,
 }
 
 Status RemoteKsckCluster::RetrieveTabletServers() {
-  vector<KuduTabletServer*> servers;
-  ElementDeleter deleter(&servers);
-  RETURN_NOT_OK(client_->ListTabletServers(&servers));
+  // Request that the tablet server states also be reported. This may include
+  // information about tablet servers that have not yet registered with the
+  // Masters.
+  ListTabletServersRequestPB req;
+  req.set_include_states(true);
+  ListTabletServersResponsePB resp;
+  RETURN_NOT_OK(client_->data_->ListTabletServers(
+      client_.get(), MonoTime::Now() + client_->default_admin_operation_timeout(), req, &resp));
 
   TSMap tablet_servers;
-  for (const auto* s : servers) {
-    auto ts = std::make_shared<RemoteKsckTabletServer>(
-        s->uuid(),
-        HostPort(s->hostname(), s->port()),
-        messenger_,
-        s->location());
+  KsckTServerStateMap ts_states;
+  for (int i = 0; i < resp.servers_size(); i++) {
+    const auto& ts_pb = resp.servers(i);
+    const auto& uuid = ts_pb.instance_id().permanent_uuid();
+    if (ts_pb.has_state() && ts_pb.state() != TServerStatePB::NONE) {
+      // We don't expect updates, but it's straightforward to handle, so let's
+      // update instead of die just in case.
+      EmplaceOrUpdate(&ts_states, uuid, ts_pb.state());
+    }
+    // If there's no registration for the tablet server, all we can really get
+    // is the state, so move on.
+    if (!ts_pb.has_registration()) {
+      continue;
+    }
+    HostPort hp;
+    RETURN_NOT_OK(HostPortFromPB(ts_pb.registration().rpc_addresses(0), &hp));
+    auto ts = std::make_shared<RemoteKsckTabletServer>(uuid, hp, messenger_, ts_pb.location());
     RETURN_NOT_OK(ts->Init());
-    InsertOrDie(&tablet_servers, ts->uuid(), ts);
+    EmplaceOrUpdate(&tablet_servers, uuid, std::move(ts));
   }
-  tablet_servers_.swap(tablet_servers);
+  tablet_servers_ = std::move(tablet_servers);
+  ts_states_ = std::move(ts_states);
   return Status::OK();
 }
 
diff --git a/src/kudu/tools/ksck_results.cc b/src/kudu/tools/ksck_results.cc
index 1ceea49..029b5f6 100644
--- a/src/kudu/tools/ksck_results.cc
+++ b/src/kudu/tools/ksck_results.cc
@@ -202,6 +202,13 @@ Status KsckResults::PrintTo(PrintMode mode, int sections, ostream& out) {
     }
   }
 
+  // Then, on any special tablet server states.
+  if (sections & PrintSections::TSERVER_STATES &&
+      !ts_states.empty()) {
+    RETURN_NOT_OK(PrintTServerStatesTable(ts_states, out));
+    out << endl;
+  }
+
   // Then, on the health of the tablet servers.
   if (sections & PrintSections::TSERVER_SUMMARIES) {
     std::sort(cluster_status.tserver_summaries.begin(),
@@ -407,6 +414,17 @@ Status PrintFlagTable(ServerType type,
   return flags_table.PrintTo(out);
 }
 
+Status PrintTServerStatesTable(const KsckTServerStateMap& ts_states, ostream& out) {
+  out << "Tablet Server States" << endl;
+  // We expect tserver states to be relatively uncommon, so print one per row.
+  DataTable table({"Server", "State"});
+  for (const auto& id_and_state : ts_states) {
+    table.AddRow({ id_and_state.first,
+                   TServerStatePB_Name(id_and_state.second) });
+  }
+  return table.PrintTo(out);
+}
+
 Status PrintVersionTable(const KsckVersionToServersMap& version_summaries,
                          int num_servers,
                          ostream& out) {
@@ -682,7 +700,7 @@ Status PrintTotalCounts(const KsckResults& results, std::ostream& out) {
 }
 
 void ServerHealthSummaryToPb(const ServerHealthSummary& summary,
-                                 ServerHealthSummaryPB* pb) {
+                             ServerHealthSummaryPB* pb) {
   switch (summary.health) {
     case ServerHealth::HEALTHY:
       pb->set_health(ServerHealthSummaryPB_ServerHealth_HEALTHY);
diff --git a/src/kudu/tools/ksck_results.h b/src/kudu/tools/ksck_results.h
index 34675db..ec69b6b 100644
--- a/src/kudu/tools/ksck_results.h
+++ b/src/kudu/tools/ksck_results.h
@@ -26,6 +26,7 @@
 
 #include <boost/optional/optional.hpp>
 
+#include "kudu/master/master.pb.h"
 #include "kudu/rebalance/cluster_status.h"
 #include "kudu/tablet/tablet.pb.h"  // IWYU pragma: keep
 #include "kudu/util/status.h"
@@ -79,15 +80,16 @@ struct PrintSections {
   enum Values {
     NONE = 0,
     MASTER_SUMMARIES = 1 << 0,
-    TSERVER_SUMMARIES = 1 << 1,
-    VERSION_SUMMARIES = 1 << 2,
-    TABLET_SUMMARIES = 1 << 3,
-    TABLE_SUMMARIES = 1 << 4,
-    CHECKSUM_RESULTS = 1 << 5,
-    TOTAL_COUNT = 1 << 6,
+    TSERVER_STATES = 1 << 1,
+    TSERVER_SUMMARIES = 1 << 2,
+    VERSION_SUMMARIES = 1 << 3,
+    TABLET_SUMMARIES = 1 << 4,
+    TABLE_SUMMARIES = 1 << 5,
+    CHECKSUM_RESULTS = 1 << 6,
+    TOTAL_COUNT = 1 << 7,
 
     // Print all sections above.
-    ALL_SECTIONS = 0b01111111
+    ALL_SECTIONS = 0b011111111
   };
 };
 
@@ -103,6 +105,8 @@ typedef std::unordered_map<std::string, std::string> KsckFlagTagsMap;
 // Convenience map version -> servers.
 typedef std::map<std::string, std::vector<std::string>> KsckVersionToServersMap;
 
+typedef std::map<std::string, master::TServerStatePB> KsckTServerStateMap;
+
 // Container for all the results of a series of ksck checks.
 struct KsckResults {
 
@@ -127,6 +131,9 @@ struct KsckResults {
   KsckFlagToServersMap tserver_flag_to_servers_map;
   KsckFlagTagsMap tserver_flag_tags_map;
 
+  // Any special states that the tablet servers may be in.
+  KsckTServerStateMap ts_states;
+
   // Collected results of the checksum scan.
   KsckChecksumResults checksum_results;
 
@@ -158,6 +165,9 @@ Status PrintFlagTable(cluster_summary::ServerType type,
                       const KsckFlagTagsMap& flag_tags_map,
                       std::ostream& out);
 
+Status PrintTServerStatesTable(const KsckTServerStateMap& ts_states,
+                               std::ostream& out);
+
 // Print a summary of the Kudu versions running across all servers from which
 // information could be fetched. Servers are grouped by version to make the
 // table compact.
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index a5066dc..d7db521 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -2935,6 +2935,18 @@ TEST_F(ToolTest, TestTServerListState) {
         Substitute("tserver list $0 --columns=uuid,state --format=csv", master_addr), &out));
   ASSERT_STR_CONTAINS(out, Substitute("$0,$1", ts_uuid, "MAINTENANCE_MODE"));
 
+  // Ksck should show a table showing the state.
+  {
+    string out;
+    NO_FATALS(RunActionStdoutString(
+        Substitute("cluster ksck $0", master_addr), &out));
+    ASSERT_STR_CONTAINS(out, Substitute(
+         "Tablet Server States\n"
+         "              Server              |      State\n"
+         "----------------------------------+------------------\n"
+         " $0 | MAINTENANCE_MODE", ts_uuid));
+  }
+
   // We should still see the state if the tserver hasn't been registered.
   // "Unregister" the server by restarting the cluster and only bringing up the
   // master, and verify that we still see the tserver in maintenance mode.
@@ -2944,6 +2956,16 @@ TEST_F(ToolTest, TestTServerListState) {
   NO_FATALS(RunActionStdoutString(
         Substitute("tserver list $0 --columns=uuid,state --format=csv", master_addr), &out));
   ASSERT_STR_CONTAINS(out, Substitute("$0,$1", ts_uuid, "MAINTENANCE_MODE"));
+  {
+    string out;
+    NO_FATALS(RunActionStdoutString(
+        Substitute("cluster ksck $0", master_addr), &out));
+    ASSERT_STR_CONTAINS(out, Substitute(
+         "Tablet Server States\n"
+         "              Server              |      State\n"
+         "----------------------------------+------------------\n"
+         " $0 | MAINTENANCE_MODE", ts_uuid));
+  }
 
   NO_FATALS(RunActionStdoutNone(Substitute("tserver state exit_maintenance $0 $1",
                                            master_addr, ts_uuid)));
@@ -2962,6 +2984,10 @@ TEST_F(ToolTest, TestTServerListState) {
           Substitute("tserver list $0 --columns=uuid,state --format=csv", master_addr), &out));
     ASSERT_STR_CONTAINS(out, Substitute("$0,$1", ts_uuid, "NONE"));
   });
+  // Ksck should also report that there aren't special tablet server states.
+  NO_FATALS(RunActionStdoutString(
+      Substitute("cluster ksck $0", master_addr), &out));
+  ASSERT_STR_NOT_CONTAINS(out, "Tablet Server States");
 }
 
 TEST_F(ToolTest, TestMasterList) {


[kudu] 04/04: master: GetTableStatistics should use signed ints

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch branch-1.11.x
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 16bdf47446a853290f791abc610391bb99f66c14
Author: Adar Dembo <ad...@cloudera.com>
AuthorDate: Wed Oct 16 13:12:27 2019 -0700

    master: GetTableStatistics should use signed ints
    
    The statistics are reported to the master as signed ints, so the master
    should likewise advertise them as such. Otherwise the live_row_count value
    of -1 (when a tablet doesn't support it) shows up as:
    
      TABLE test-workload
      on disk size: 0
      live row count: 18446744073709551615
    
    The public APIs were already treating them as signed; they were only only
    unsigned on the wire.
    
    Change-Id: I398cb85888bdc9463264692911788de2fef67dfc
    Reviewed-on: http://gerrit.cloudera.org:8080/14463
    Reviewed-by: Grant Henke <gr...@apache.org>
    Tested-by: Kudu Jenkins
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
    (cherry picked from commit adecebd9e22b50254d69c94c784d5cba3657890b)
    Reviewed-on: http://gerrit.cloudera.org:8080/14470
---
 src/kudu/client/table_statistics-internal.h |  6 +++---
 src/kudu/master/master.proto                |  4 ++--
 src/kudu/tools/kudu-tool-test.cc            | 21 +++++++++++++++++++--
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/src/kudu/client/table_statistics-internal.h b/src/kudu/client/table_statistics-internal.h
index 153e9f9..eda125d 100644
--- a/src/kudu/client/table_statistics-internal.h
+++ b/src/kudu/client/table_statistics-internal.h
@@ -31,7 +31,7 @@ using strings::Substitute;
 
 class KuduTableStatistics::Data {
  public:
-  Data(uint64_t on_disk_size, uint64_t live_row_count)
+  Data(int64_t on_disk_size, int64_t live_row_count)
       : on_disk_size_(on_disk_size),
         live_row_count_(live_row_count) {
   }
@@ -46,8 +46,8 @@ class KuduTableStatistics::Data {
     return display_string;
   }
 
-  const uint64_t on_disk_size_;
-  const uint64_t live_row_count_;
+  const int64_t on_disk_size_;
+  const int64_t live_row_count_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(Data);
diff --git a/src/kudu/master/master.proto b/src/kudu/master/master.proto
index 0c77150..bda76e2 100644
--- a/src/kudu/master/master.proto
+++ b/src/kudu/master/master.proto
@@ -553,8 +553,8 @@ message GetTableStatisticsResponsePB {
   optional MasterErrorPB error = 1;
 
   // The table statistics from table metrics.
-  optional uint64 on_disk_size = 2;
-  optional uint64 live_row_count = 3;
+  optional int64 on_disk_size = 2;
+  optional int64 live_row_count = 3;
 }
 
 message GetTableLocationsRequestPB {
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index d7db521..d2c1718 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -5481,7 +5481,6 @@ TEST_F(AuthzTServerChecksumTest, TestAuthorizeChecksum) {
     "--checksum_scan"
   };
   ASSERT_OK(RunKuduTool(checksum_args));
-
 }
 
 // Regression test for KUDU-2851.
@@ -5506,7 +5505,6 @@ TEST_F(ToolTest, TestFailedTableScan) {
   ASSERT_TRUE(s.IsRuntimeError());
   SCOPED_TRACE(stderr);
   ASSERT_STR_CONTAINS(stderr, "Timed out");
-
 }
 
 TEST_F(ToolTest, TestFailedTableCopy) {
@@ -5535,7 +5533,26 @@ TEST_F(ToolTest, TestFailedTableCopy) {
   ASSERT_TRUE(s.IsRuntimeError());
   SCOPED_TRACE(stderr);
   ASSERT_STR_CONTAINS(stderr, "Timed out");
+}
+
+TEST_F(ToolTest, TestGetTableStatisticsLiveRowCountNotSupported) {
+  ExternalMiniClusterOptions opts;
+  opts.extra_master_flags = { "--mock_table_metrics_for_testing=true",
+                              "--live_row_count_for_testing=-1" };
+  NO_FATALS(StartExternalMiniCluster(std::move(opts)));
+
+  // Create an empty table.
+  TestWorkload workload(cluster_.get());
+  workload.set_num_replicas(1);
+  workload.Setup();
 
+  string stdout;
+  NO_FATALS(RunActionStdoutString(
+      Substitute("table statistics $0 $1",
+                 cluster_->master()->bound_rpc_addr().ToString(),
+                 TestWorkload::kDefaultTableName),
+      &stdout));
+  ASSERT_STR_CONTAINS(stdout, "live row count: -1");
 }
 
 } // namespace tools


[kudu] 03/04: CapturingLogAppender: synchronize access to captured log text

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch branch-1.11.x
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit a9a5001ec9cd61c3806a581ca147ea70c1ee30a0
Author: Adar Dembo <ad...@cloudera.com>
AuthorDate: Mon Oct 14 13:24:09 2019 -0700

    CapturingLogAppender: synchronize access to captured log text
    
    I have no explanation for how this could happen, but the test failure I'm
    looking at speaks for itself:
    
      1) testSessionOnceClosed(org.apache.kudu.client.TestKuduClient)
      java.lang.StringIndexOutOfBoundsException: String index out of range: 419
    	at java.lang.String.<init>(String.java:205)
    	at java.lang.StringBuilder.toString(StringBuilder.java:407)
    	at org.apache.kudu.test.CapturingLogAppender.getAppendedText(CapturingLogAppender.java:72)
    	at org.apache.kudu.client.TestKuduClient.testSessionOnceClosed(TestKuduClient.java:1231)
    
    Change-Id: I84a5d0775cba5aa1d9df5484b5e9e621c908d42d
    Reviewed-on: http://gerrit.cloudera.org:8080/14431
    Reviewed-by: Greg Solovyev <gs...@cloudera.com>
    Tested-by: Kudu Jenkins
    Reviewed-by: Grant Henke <gr...@apache.org>
    (cherry picked from commit 0894eba26380036d6d6d59c29c3efc5b3a817641)
    Reviewed-on: http://gerrit.cloudera.org:8080/14467
---
 .../main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala  |  6 ++++--
 .../main/java/org/apache/kudu/test/CapturingLogAppender.java   | 10 ++++++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala b/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
index 74da812..907a4d5 100644
--- a/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
+++ b/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
@@ -268,8 +268,10 @@ class KuduRelation(
       table.getTableStatistics().getOnDiskSize
     } catch {
       case e: Exception =>
-        log.warn("Error while getting table statistic from master, maybe the current" +
-          " master doesn't support the rpc, please check the version.", e)
+        log.warn(
+          "Error while getting table statistic from master, maybe the current" +
+            " master doesn't support the rpc, please check the version.",
+          e)
         super.sizeInBytes
     }
   }
diff --git a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
index 932e14a..9227016 100644
--- a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
+++ b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
@@ -19,6 +19,7 @@ package org.apache.kudu.test;
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.Random;
+import javax.annotation.concurrent.GuardedBy;
 
 import com.google.common.base.Throwables;
 import org.apache.logging.log4j.core.LogEvent;
@@ -42,6 +43,11 @@ public class CapturingLogAppender extends AbstractAppender {
       .withPattern("%d{HH:mm:ss.SSS} [%p - %t] (%F:%L) %m%n")
       .build();
 
+  // The caller should detach the logger before calling getAppendedText().
+  // Nevertheless, for some reason it is still possible for additional
+  // append() calls to happen _after_ the logger is detached, which may race
+  // with getAppendedText().
+  @GuardedBy("this")
   private StringBuilder appended = new StringBuilder();
 
   public CapturingLogAppender() {
@@ -57,7 +63,7 @@ public class CapturingLogAppender extends AbstractAppender {
   }
 
   @Override
-  public void append(LogEvent event) {
+  public synchronized void append(LogEvent event) {
     appended.append(getLayout().toSerializable(event));
     if (event.getThrown() != null) {
       appended.append(Throwables.getStackTraceAsString(event.getThrown()));
@@ -68,7 +74,7 @@ public class CapturingLogAppender extends AbstractAppender {
   /**
    * @return all of the appended messages captured thus far, joined together.
    */
-  public String getAppendedText() {
+  public synchronized String getAppendedText() {
     return appended.toString();
   }
 


[kudu] 01/04: [clock] more info on refusal to advance hybrid timestamp

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch branch-1.11.x
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 8899096f3d1acfe59c59eea4e5e2f3fe21115b8e
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Tue Oct 15 10:52:31 2019 -0700

    [clock] more info on refusal to advance hybrid timestamp
    
    Enhanced the error message on the attempt to update hybrid timestamp
    beyond the maximum allowed error threshold.
    
    This patch does not contain any functional changes.
    
    Change-Id: I676fc89fb96fa5383ae354207b64bed3ffe00300
    Reviewed-on: http://gerrit.cloudera.org:8080/14455
    Tested-by: Kudu Jenkins
    Reviewed-by: Adar Dembo <ad...@cloudera.com>
    (cherry picked from commit 3bd293d0f32003e877e7f160365ffd238927e164)
    Reviewed-on: http://gerrit.cloudera.org:8080/14465
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
 src/kudu/clock/hybrid_clock.cc | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/kudu/clock/hybrid_clock.cc b/src/kudu/clock/hybrid_clock.cc
index 9a8579a..07170f6 100644
--- a/src/kudu/clock/hybrid_clock.cc
+++ b/src/kudu/clock/hybrid_clock.cc
@@ -277,16 +277,20 @@ Status HybridClock::Update(const Timestamp& to_update) {
 
   uint64_t to_update_physical = GetPhysicalValueMicros(to_update);
   uint64_t now_physical = GetPhysicalValueMicros(now);
-
-  // we won't update our clock if to_update is more than 'max_clock_sync_error_usec'
-  // into the future as it might have been corrupted or originated from an out-of-sync
-  // server.
-  if ((to_update_physical - now_physical) > FLAGS_max_clock_sync_error_usec) {
-    return Status::InvalidArgument("Tried to update clock beyond the max. error.");
+  DCHECK_GE(to_update_physical, now_physical);
+
+  // Don't update our clock if 'to_update' is more than
+  // '--max_clock_sync_error_usec' into the future as it might have been
+  // corrupted or originated from an out-of-sync server.
+  if (to_update_physical - now_physical > FLAGS_max_clock_sync_error_usec) {
+    return Status::InvalidArgument(Substitute(
+        "tried to update clock beyond the error threshold of $0us: "
+        "now $1, to_update $2 (now_physical $3, to_update_physical $4)",
+        FLAGS_max_clock_sync_error_usec,
+        now.ToUint64(), to_update.ToUint64(), now_physical, to_update_physical));
   }
 
-  // Our next timestamp must be higher than the one that we are updating
-  // from.
+  // Our next timestamp must be higher than the one that we are updating from.
   next_timestamp_ = to_update.value() + 1;
   return Status::OK();
 }