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/12/05 02:06:47 UTC

[kudu] branch master updated: KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI

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

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


The following commit(s) were added to refs/heads/master by this push:
     new caf2b50  KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI
caf2b50 is described below

commit caf2b50529ce59bb4847e76c3e32f2574bae7809
Author: helifu <hz...@corp.netease.com>
AuthorDate: Mon Dec 2 09:58:44 2019 +0800

    KUDU-2986 p2: adjust the 'live_row_count' metric in master's Web UI
    
    Hide the 'live_row_count' metric in the master's Web UI while
    there is any tablet which doesn't support live row count. And
    unhide it after the legacy tablets are all deleted.
    
    http://master:8051/metrics?ids=5448c0a8658f467ea373d9044d567011
    [
        {
            "type": "table",
            "id": "5448c0a8658f467ea373d9044d567011",
            "attributes": {
                "table_name": "testtable"
            },
            "metrics": [
                {
                    "name": "on_disk_size",
                    "value": 8417243
                }
            ]
        }
    ]
    
    Change-Id: I4c7ac5ca7e8ce9dcc37035a7bc46ca69060d6533
    Reviewed-on: http://gerrit.cloudera.org:8080/14601
    Tested-by: Kudu Jenkins
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
 src/kudu/master/catalog_manager.cc | 31 ++++++++++++++---
 src/kudu/master/master-test.cc     | 70 ++++++++++++++++++++++++++++++++++++++
 src/kudu/util/metrics.h            | 15 +++++---
 3 files changed, 106 insertions(+), 10 deletions(-)

diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index 6a96d33..9d2ce01 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -5587,13 +5587,25 @@ void TableInfo::UpdateMetrics(const string& tablet_id,
         - static_cast<int64_t>(old_stats.on_disk_size()));
     if (new_stats.has_live_row_count()) {
       DCHECK(!metrics_->ContainsTabletNoLiveRowCount(tablet_id));
-      metrics_->live_row_count->IncrementBy(
-          static_cast<int64_t>(new_stats.live_row_count())
-          - static_cast<int64_t>(old_stats.live_row_count()));
+
+      // The function "IncrementBy" makes the metric visible by validating the epoch.
+      // So, it's very important to skip calling it while the metric is invisible.
+      if (!metrics_->live_row_count->IsInvisible()) {
+        metrics_->live_row_count->IncrementBy(
+            static_cast<int64_t>(new_stats.live_row_count())
+            - static_cast<int64_t>(old_stats.live_row_count()));
+      }
     } else if (!old_stats.has_on_disk_size()) {
-      // The new reported stat doesn't support 'live_row_count' and
-      // this is the first report stat of the tablet.
+      // For an existing tablet, either it supports 'live_row_count' or not. Obviously,
+      // it doesn't support this feature in this branch. So we gather the tablet uuid to
+      // make sure later that the table doesn't support this either. But the new_stats
+      // will keep coming, so it's necessary to limit the operation to one time only.
+      // Thus, we use the 'on_disk_size' metric of the old_stats as a switch when it is
+      // transitioned from "uninitialized stats" to "has_on_disk_size()".
       metrics_->AddTabletNoLiveRowCount(tablet_id);
+
+      // Make the metric invisible by invalidating the epoch.
+      metrics_->live_row_count->InvalidateEpoch();
     }
   }
 }
@@ -5607,6 +5619,15 @@ void TableInfo::RemoveMetrics(const string& tablet_id,
       metrics_->live_row_count->IncrementBy(-static_cast<int64_t>(old_stats.live_row_count()));
     } else {
       metrics_->DeleteTabletNoLiveRowCount(tablet_id);
+
+      // Make the metric visible again after all of the legacy tablets are deleted.
+      if (metrics_->TableSupportsLiveRowCount()) {
+        uint64_t live_row_count = 0;
+        for (const auto& e : tablet_map_) {
+          live_row_count += e.second->GetStats().live_row_count();
+        }
+        metrics_->live_row_count->IncrementBy(static_cast<int64_t>(live_row_count));
+      }
     }
   }
 }
diff --git a/src/kudu/master/master-test.cc b/src/kudu/master/master-test.cc
index bc35210..9f2fc06 100644
--- a/src/kudu/master/master-test.cc
+++ b/src/kudu/master/master-test.cc
@@ -52,6 +52,7 @@
 #include "kudu/gutil/dynamic_annotations.h"
 #include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/map-util.h"
+#include "kudu/gutil/ref_counted.h"
 #include "kudu/gutil/strings/split.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/gutil/strings/util.h"
@@ -70,6 +71,7 @@
 #include "kudu/security/token.pb.h"
 #include "kudu/security/token_verifier.h"
 #include "kudu/server/rpc_server.h"
+#include "kudu/tablet/metadata.pb.h"
 #include "kudu/util/atomic.h"
 #include "kudu/util/countdown_latch.h"
 #include "kudu/util/curl_util.h"
@@ -1800,6 +1802,74 @@ TEST_F(MasterTest, TestGetTableStatistics) {
   ASSERT_EQ(FLAGS_live_row_count_for_testing, resp.live_row_count());
 }
 
+TEST_F(MasterTest, TestHideLiveRowCountInTableMetrics) {
+  const char* kTableName = "testtable";
+  const Schema kTableSchema({ ColumnSchema("key", INT32) }, 1);
+  ASSERT_OK(CreateTable(kTableName, kTableSchema));
+
+  vector<scoped_refptr<TableInfo>> tables;
+  {
+    CatalogManager::ScopedLeaderSharedLock l(master_->catalog_manager());
+    ASSERT_OK(master_->catalog_manager()->GetAllTables(&tables));
+    ASSERT_EQ(1, tables.size());
+  }
+  vector<scoped_refptr<TabletInfo>> tablets;
+  tables[0]->GetAllTablets(&tablets);
+
+  const auto call_update_metrics = [&] (
+      scoped_refptr<TabletInfo>& tablet,
+      bool support_live_row_count) {
+    tablet::ReportedTabletStatsPB old_stats;
+    tablet::ReportedTabletStatsPB new_stats;
+    new_stats.set_on_disk_size(1);
+    if (support_live_row_count) {
+      old_stats.set_live_row_count(1);
+      new_stats.set_live_row_count(1);
+    }
+    tables[0]->UpdateMetrics(tablet->id(), old_stats, new_stats);
+  };
+
+  // Trigger to cause 'live_row_count' invisible.
+  {
+    for (int i = 0; i < 100; ++i) {
+      for (int j = 0; j < tablets.size(); ++j) {
+        NO_FATALS(call_update_metrics(tablets[j], (tablets.size() - 1 != j)));
+      }
+    }
+
+    EasyCurl c;
+    faststring buf;
+    ASSERT_OK(c.FetchURL(Substitute("http://$0/metrics?ids=$1",
+                                    mini_master_->bound_http_addr().ToString(),
+                                    tables[0]->id()),
+                         &buf));
+    string raw = buf.ToString();
+    ASSERT_STR_CONTAINS(raw, kTableName);
+    ASSERT_STR_CONTAINS(raw, "on_disk_size");
+    ASSERT_STR_NOT_CONTAINS(raw, "live_row_count");
+  }
+
+  // Trigger to cause 'live_row_count' visible.
+  {
+    tables[0]->RemoveMetrics(tablets.back()->id(), tablet::ReportedTabletStatsPB());
+    for (int i = 0; i < 100; ++i) {
+      for (int j = 0; j < tablets.size(); ++j) {
+        NO_FATALS(call_update_metrics(tablets[j], true));
+      }
+    }
+    EasyCurl c;
+    faststring buf;
+    ASSERT_OK(c.FetchURL(Substitute("http://$0/metrics?ids=$1",
+                                    mini_master_->bound_http_addr().ToString(),
+                                    tables[0]->id()),
+                         &buf));
+    string raw = buf.ToString();
+    ASSERT_STR_CONTAINS(raw, kTableName);
+    ASSERT_STR_CONTAINS(raw, "on_disk_size");
+    ASSERT_STR_CONTAINS(raw, "live_row_count");
+  }
+}
+
 class AuthzTokenMasterTest : public MasterTest,
                              public ::testing::WithParamInterface<bool> {};
 
diff --git a/src/kudu/util/metrics.h b/src/kudu/util/metrics.h
index 5a3e391..21683f2 100644
--- a/src/kudu/util/metrics.h
+++ b/src/kudu/util/metrics.h
@@ -779,6 +779,16 @@ class Metric : public RefCountedThreadSafe<Metric> {
   // NOTE: If merge with self, do nothing.
   virtual void MergeFrom(const scoped_refptr<Metric>& other) = 0;
 
+  // Invalidate 'm_epoch_', causing this metric to be invisible until its value changes.
+  void InvalidateEpoch() {
+    m_epoch_ = -1;
+  }
+
+  // Return true if this metric is invisible otherwise false.
+  bool IsInvisible() const {
+    return -1 == m_epoch_;
+  }
+
  protected:
   explicit Metric(const MetricPrototype* prototype);
   virtual ~Metric();
@@ -792,11 +802,6 @@ class Metric : public RefCountedThreadSafe<Metric> {
     }
   }
 
-  // Invalidate 'm_epoch_', causing this metric to be invisible until its value changes.
-  void InvalidateEpoch() {
-    m_epoch_ = -1;
-  }
-
   // Causes this metric to be skipped during a merge..
   void InvalidateForMerge() {
     invalid_for_merge_ = true;