Improve scans dashboard

* Adds a ring buffer to the scanners manager which holds historical scan details, so that the scan dashboard can show recently completed scans. The size of the buffer is configurable with a new experimental flag, --scan-history-count which defaults to 20. A new 'state' column has been added to indicate the state of the scan (active, expired, failed, or complete). * Mustache-ifies the scans dashboard. * The dashboard previously showed raw values, with pretty-printed values as a tooltip. In order to match the maintenance manager dashboard, these have been swapped so that pretty-printed values are more prominent. * Adds a new pseudo-SQL query description column, replacing the predicates columns. This allows the table, projected columns, and optimized predicates to be shown in a very concise way. Previously the table name and projected columns weren't exposed at all. * Removes 'from disk' from statistics names, since it was misleading, or downright wrong. The tooltip for the cells, bytes, and cblocks read columns now indicates that the values could have been read from cache or from disk. It would probably be nice to add a cache hit rate to the stats, but I didn't want to include it in this commit as it's already big. * Adds aggregated totals for scan stats. * A bunch of other polish, like tooltip descriptions of the non-completely-obvious column headers, and adding a link from the tablet ID to the replica's page. example: https://i.imgur.com/WLMK263.png Change-Id: Iadbdee00d8a343fd3728c6ec8ee252d64d0e416a Reviewed-on: http://gerrit.cloudera.org:8080/8891 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin <[email protected]>
wnch · Dec 22, 2017 · 5356334 · 5356334
1 parent d78b272
commit 5356334
Show file tree

Hide file tree

Showing 17 changed files with 505 additions and 248 deletions.
diff --git a/src/kudu/cfile/cfile_reader.cc b/src/kudu/cfile/cfile_reader.cc
@@ -939,9 +939,9 @@ Status CFileIterator::ReadCurrentDataBlock(const IndexTreeIterator &idx_iter,
     num_rows_in_block = bd->Count();
   }
 
-  io_stats_.cells_read_from_disk += num_rows_in_block;
-  io_stats_.data_blocks_read_from_disk++;
-  io_stats_.bytes_read_from_disk += data_block.size();
+  io_stats_.cells_read += num_rows_in_block;
+  io_stats_.cblocks_read++;
+  io_stats_.bytes_read += data_block.size();
 
   prep_block->idx_in_block_ = 0;
   prep_block->num_rows_in_block_ = num_rows_in_block;

diff --git a/src/kudu/common/column_predicate-test.cc b/src/kudu/common/column_predicate-test.cc
@@ -1096,7 +1096,7 @@ TEST_F(TestColumnPredicate, TestRedaction) {
   ASSERT_NE("", gflags::SetCommandLineOption("redact", "log"));
   ColumnSchema column_i32("a", INT32, true);
   int32_t one_32 = 1;
-  ASSERT_EQ("`a` = <redacted>", ColumnPredicate::Equality(column_i32, &one_32).ToString());
+  ASSERT_EQ("a = <redacted>", ColumnPredicate::Equality(column_i32, &one_32).ToString());
 }
 
 } // namespace kudu
diff --git a/src/kudu/common/column_predicate.cc b/src/kudu/common/column_predicate.cc
@@ -27,8 +27,10 @@
 #include "kudu/common/rowblock.h"
 #include "kudu/common/schema.h"
 #include "kudu/common/types.h"
+#include "kudu/gutil/strings/join.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/bitmap.h"
+#include "kudu/util/logging.h"
 #include "kudu/util/memory/arena.h"
 
 using std::move;
@@ -627,41 +629,36 @@ void ColumnPredicate::Evaluate(const ColumnBlock& block, SelectionVector* sel) c
 
 string ColumnPredicate::ToString() const {
   switch (predicate_type()) {
-    case PredicateType::None: return strings::Substitute("`$0` NONE", column_.name());
+    case PredicateType::None: return strings::Substitute("$0 NONE", column_.name());
     case PredicateType::Range: {
       if (lower_ == nullptr) {
-        return strings::Substitute("`$0` < $1", column_.name(), column_.Stringify(upper_));
-      } else if (upper_ == nullptr) {
-        return strings::Substitute("`$0` >= $1", column_.name(), column_.Stringify(lower_));
-      } else {
-        return strings::Substitute("`$0` >= $1 AND `$0` < $2",
-                                   column_.name(),
-                                   column_.Stringify(lower_),
-                                   column_.Stringify(upper_));
+        return strings::Substitute("$0 < $1", column_.name(), column_.Stringify(upper_));
       }
+      if (upper_ == nullptr) {
+        return strings::Substitute("$0 >= $1", column_.name(), column_.Stringify(lower_));
+      }
+      return strings::Substitute("$0 >= $1 AND $0 < $2",
+                                 column_.name(),
+                                 column_.Stringify(lower_),
+                                 column_.Stringify(upper_));
+
     };
     case PredicateType::Equality: {
-      return strings::Substitute("`$0` = $1", column_.name(), column_.Stringify(lower_));
+      return strings::Substitute("$0 = $1", column_.name(), column_.Stringify(lower_));
     };
     case PredicateType::IsNotNull: {
-      return strings::Substitute("`$0` IS NOT NULL", column_.name());
+      return strings::Substitute("$0 IS NOT NULL", column_.name());
     };
     case PredicateType::IsNull: {
-      return strings::Substitute("`$0` IS NULL", column_.name());
+      return strings::Substitute("$0 IS NULL", column_.name());
     };
     case PredicateType::InList: {
-      string ss = "`";
+      string ss;
       ss.append(column_.name());
-      ss.append("` IN (");
-      bool is_first = true;
-      for (auto* value : values_) {
-        if (is_first) {
-          is_first = false;
-        } else {
-          ss.append(", ");
-        }
-        ss.append(column_.Stringify(value));
-      }
+      ss.append(" IN (");
+      ss.append(KUDU_REDACT(JoinMapped(values_,
+                                       [&] (const void* value) { return column_.Stringify(value); },
+                                       ", ")));
       ss.append(")");
       return ss;
     };

diff --git a/src/kudu/common/encoded_key.cc b/src/kudu/common/encoded_key.cc
@@ -119,21 +119,23 @@ Status EncodedKey::IncrementEncodedKey(const Schema& tablet_schema,
 }
 
 string EncodedKey::Stringify(const Schema &schema) const {
-  if (num_key_cols_ == 1) {
-    return schema.column(0).Stringify(raw_keys_.front());
-  }
+  DCHECK_EQ(schema.num_key_columns(), num_key_cols_);
+  DCHECK_EQ(schema.num_key_columns(), raw_keys_.size());
 
   faststring s;
   s.append("(");
-  for (int i = 0; i < num_key_cols_; i++) {
+  for (int i = 0; i < raw_keys_.size(); i++) {
     if (i > 0) {
-      s.append(",");
-    }
-    if (i < raw_keys_.size()) {
-      s.append(schema.column(i).Stringify(raw_keys_[i]));
-    } else {
-      s.append("*");
+      if (schema.column(i).type_info()->IsMinValue(raw_keys_[i])) {
+        // If the value is the minimum, short-circuit to avoid printing keys such as
+        // '(2, -9223372036854775808, -9223372036854775808, -9223372036854775808)',
+        // and instead print '(2)'. The minimum values are usually filled in
+        // automatically upon decoding, so it makes sense to omit them.
+        break;
+      }
+      s.append(", ");
     }
+    s.append(schema.column(i).Stringify(raw_keys_[i]));
   }
   s.append(")");
   return s.ToString();

diff --git a/src/kudu/common/generic_iterators.cc b/src/kudu/common/generic_iterators.cc
@@ -71,7 +71,7 @@ void AddIterStats(const RowwiseIterator& iter,
   iter.GetIteratorStats(&iter_stats);
   DCHECK_EQ(stats->size(), iter_stats.size());
   for (int i = 0; i < iter_stats.size(); i++) {
-    (*stats)[i].AddStats(iter_stats[i]);
+    (*stats)[i] += iter_stats[i];
   }
 }
 } // anonymous namespace

diff --git a/src/kudu/common/iterator_stats.cc b/src/kudu/common/iterator_stats.cc
@@ -27,39 +27,47 @@ using std::string;
 using strings::Substitute;
 
 IteratorStats::IteratorStats()
-    : data_blocks_read_from_disk(0),
-      bytes_read_from_disk(0),
-      cells_read_from_disk(0) {
+    : cells_read(0),
+      bytes_read(0),
+      cblocks_read(0) {
 }
 
 string IteratorStats::ToString() const {
-  return Substitute("data_blocks_read_from_disk=$0 "
-                    "bytes_read_from_disk=$1 "
-                    "cells_read_from_disk=$2",
-                    data_blocks_read_from_disk,
-                    bytes_read_from_disk,
-                    cells_read_from_disk);
+  return Substitute("cells_read=$0 bytes_read=$1 cblocks_read=$2",
+                    cells_read, bytes_read, cblocks_read);
 }
 
-void IteratorStats::AddStats(const IteratorStats& other) {
-  data_blocks_read_from_disk += other.data_blocks_read_from_disk;
-  bytes_read_from_disk += other.bytes_read_from_disk;
-  cells_read_from_disk += other.cells_read_from_disk;
+IteratorStats& IteratorStats::operator+=(const IteratorStats& other) {
+  cells_read += other.cells_read;
+  bytes_read += other.bytes_read;
+  cblocks_read += other.cblocks_read;
   DCheckNonNegative();
+  return *this;
 }
 
-void IteratorStats::SubtractStats(const IteratorStats& other) {
-  data_blocks_read_from_disk -= other.data_blocks_read_from_disk;
-  bytes_read_from_disk -= other.bytes_read_from_disk;
-  cells_read_from_disk -= other.cells_read_from_disk;
+IteratorStats& IteratorStats::operator-=(const IteratorStats& other) {
+  cells_read -= other.cells_read;
+  bytes_read -= other.bytes_read;
+  cblocks_read -= other.cblocks_read;
   DCheckNonNegative();
+  return *this;
 }
 
-void IteratorStats::DCheckNonNegative() const {
-  DCHECK_GE(data_blocks_read_from_disk, 0);
-  DCHECK_GE(bytes_read_from_disk, 0);
-  DCHECK_GE(cells_read_from_disk, 0);
+IteratorStats IteratorStats::operator+(const IteratorStats& other) {
+  IteratorStats copy = *this;
+  copy += other;
+  return copy;
 }
 
+IteratorStats IteratorStats::operator-(const IteratorStats& other) {
+  IteratorStats copy = *this;
+  copy -= other;
+  return copy;
+}
 
+void IteratorStats::DCheckNonNegative() const {
+  DCHECK_GE(cells_read, 0);
+  DCHECK_GE(bytes_read, 0);
+  DCHECK_GE(cblocks_read, 0);
+}
 } // namespace kudu
diff --git a/src/kudu/common/iterator_stats.h b/src/kudu/common/iterator_stats.h
@@ -27,23 +27,24 @@ struct IteratorStats {
 
   std::string ToString() const;
 
-  // The number of data blocks read from disk (or cache) by the iterator.
-  int64_t data_blocks_read_from_disk;
+  // The number of cells which were read from disk --  regardless of whether
+  // they were decoded/materialized.
+  int64_t cells_read;
 
   // The number of bytes read from disk (or cache) by the iterator.
-  int64_t bytes_read_from_disk;
+  int64_t bytes_read;
 
-  // The number of cells which were read from disk --  regardless of whether
-  // they were decoded/materialized.
-  int64_t cells_read_from_disk;
+  // The number of CFile data blocks read from disk (or cache) by the iterator.
+  int64_t cblocks_read;
 
   // Add statistics contained 'other' to this object (for each field
   // in this object, increment it by the value of the equivalent field
   // in 'other').
-  void AddStats(const IteratorStats& other);
+  IteratorStats& operator+=(const IteratorStats& other);
+  IteratorStats& operator-=(const IteratorStats& other);
 
-  // Same, except subtract.
-  void SubtractStats(const IteratorStats& other);
+  IteratorStats operator+(const IteratorStats& other);
+  IteratorStats operator-(const IteratorStats& other);
 
  private:
   // DCHECK that all of the stats are non-negative. This is a no-op in