kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [kudu] branch master updated: metrics: modify MergeFrom to a void function
Date Thu, 19 Sep 2019 20:04:17 GMT
This is an automated email from the ASF dual-hosted git repository.

adar 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 4c157da  metrics: modify MergeFrom to a void function
4c157da is described below

commit 4c157da6ceb203420d530c41f760c075360928ed
Author: zhangyifan27 <chinazhangyifan@163.com>
AuthorDate: Thu Sep 19 14:05:19 2019 +0800

    metrics: modify MergeFrom to a void function
    
    Change-Id: Ie00d7d7989b8956295dff7088d6f94c75e64554d
    Reviewed-on: http://gerrit.cloudera.org:8080/14262
    Reviewed-by: Adar Dembo <adar@cloudera.com>
    Tested-by: Adar Dembo <adar@cloudera.com>
---
 src/kudu/util/metrics-test.cc | 32 ++++++++++++++++----------------
 src/kudu/util/metrics.cc      |  5 ++---
 src/kudu/util/metrics.h       | 19 +++++++------------
 3 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/src/kudu/util/metrics-test.cc b/src/kudu/util/metrics-test.cc
index 6e8e4c8..7e5bbbc 100644
--- a/src/kudu/util/metrics-test.cc
+++ b/src/kudu/util/metrics-test.cc
@@ -103,14 +103,14 @@ TEST_F(MetricsTest, SimpleCounterMergeTest) {
     new Counter(&METRIC_test_counter);
   requests->IncrementBy(2);
   requests_for_merge->IncrementBy(3);
-  ASSERT_TRUE(requests_for_merge->MergeFrom(requests));
+  requests_for_merge->MergeFrom(requests);
   ASSERT_EQ(2, requests->value());
   ASSERT_EQ(5, requests_for_merge->value());
   requests->IncrementBy(7);
-  ASSERT_TRUE(requests_for_merge->MergeFrom(requests));
+  requests_for_merge->MergeFrom(requests);
   ASSERT_EQ(9, requests->value());
   ASSERT_EQ(14, requests_for_merge->value());
-  ASSERT_TRUE(requests_for_merge->MergeFrom(requests_for_merge));
+  requests_for_merge->MergeFrom(requests_for_merge);
   ASSERT_EQ(14, requests_for_merge->value());
 }
 
@@ -133,7 +133,7 @@ TEST_F(MetricsTest, SimpleStringGaugeForMergeTest) {
     new StringGauge(&METRIC_test_string_gauge, "Healthy");
   scoped_refptr<StringGauge> state_for_merge =
     new StringGauge(&METRIC_test_string_gauge, "Recovering");
-  ASSERT_TRUE(state_for_merge->MergeFrom(state));
+  state_for_merge->MergeFrom(state);
   ASSERT_EQ("Healthy", state->value());
   ASSERT_EQ(unordered_set<string>({"Recovering", "Healthy"}),
             state_for_merge->unique_values());
@@ -148,22 +148,22 @@ TEST_F(MetricsTest, SimpleStringGaugeForMergeTest) {
 
   state_old->set_value("Unavailable");
   ASSERT_EQ("Unavailable", state_old->value());
-  ASSERT_TRUE(state_for_merge_old->MergeFrom(state_old));
+  state_for_merge_old->MergeFrom(state_old);
   ASSERT_EQ(unordered_set<string>({"Unavailable", "Recovering", "Healthy"}),
             state_for_merge_old->unique_values());
 
   state->set_value("Under-replicated");
-  ASSERT_TRUE(state_for_merge->MergeFrom(state));
+  state_for_merge->MergeFrom(state);
   ASSERT_EQ(unordered_set<string>({"Under-replicated", "Healthy", "Recovering"}),
             state_for_merge->unique_values());
 
-  ASSERT_TRUE(state_for_merge->MergeFrom(state_for_merge_old));
+  state_for_merge->MergeFrom(state_for_merge_old);
   ASSERT_EQ(unordered_set<string>({"Unavailable", "Healthy", "Recovering"}),
             state_for_merge_old->unique_values());
   ASSERT_EQ(unordered_set<string>({"Unavailable", "Under-replicated", "Healthy", "Recovering"}),
             state_for_merge->unique_values());
 
-  ASSERT_TRUE(state_for_merge->MergeFrom(state_for_merge));
+  state_for_merge->MergeFrom(state_for_merge);
   ASSERT_EQ(unordered_set<string>({"Unavailable", "Under-replicated", "Healthy", "Recovering"}),
             state_for_merge->unique_values());
 }
@@ -187,14 +187,14 @@ TEST_F(MetricsTest, SimpleAtomicGaugeMergeTest) {
     METRIC_test_gauge.Instantiate(entity_, 2);
   scoped_refptr<AtomicGauge<uint64_t> > mem_usage_for_merge =
     METRIC_test_gauge.Instantiate(entity_same_attr_, 3);
-  ASSERT_TRUE(mem_usage_for_merge->MergeFrom(mem_usage));
+  mem_usage_for_merge->MergeFrom(mem_usage);
   ASSERT_EQ(2, mem_usage->value());
   ASSERT_EQ(5, mem_usage_for_merge->value());
   mem_usage->IncrementBy(7);
-  ASSERT_TRUE(mem_usage_for_merge->MergeFrom(mem_usage));
+  mem_usage_for_merge->MergeFrom(mem_usage);
   ASSERT_EQ(9, mem_usage->value());
   ASSERT_EQ(14, mem_usage_for_merge->value());
-  ASSERT_TRUE(mem_usage_for_merge->MergeFrom(mem_usage_for_merge));
+  mem_usage_for_merge->MergeFrom(mem_usage_for_merge);
   ASSERT_EQ(14, mem_usage_for_merge->value());
 }
 
@@ -284,15 +284,15 @@ TEST_F(MetricsTest, SimpleFunctionGaugeMergeTest) {
     METRIC_test_func_gauge.InstantiateFunctionGauge(
       entity_same_attr_, Bind(&MyFunction, Unretained(&metric_val_for_merge)));
 
-  ASSERT_TRUE(gauge_for_merge->MergeFrom(gauge));
+  gauge_for_merge->MergeFrom(gauge);
   ASSERT_EQ(1001, gauge->value());
   ASSERT_EQ(1002, gauge->value());
   ASSERT_EQ(2234, gauge_for_merge->value());
   ASSERT_EQ(2234, gauge_for_merge->value());
-  ASSERT_TRUE(gauge_for_merge->MergeFrom(gauge));
+  gauge_for_merge->MergeFrom(gauge);
   ASSERT_EQ(3237, gauge_for_merge->value());
   ASSERT_EQ(3237, gauge_for_merge->value());
-  ASSERT_TRUE(gauge_for_merge->MergeFrom(gauge_for_merge));
+  gauge_for_merge->MergeFrom(gauge_for_merge);
   ASSERT_EQ(3237, gauge_for_merge->value());
 }
 
@@ -362,7 +362,7 @@ TEST_F(MetricsTest, SimpleHistogramMergeTest) {
   hist->IncrementBy(6, 1);
   hist_for_merge->Increment(1);
   hist_for_merge->IncrementBy(3, 3);
-  ASSERT_TRUE(hist_for_merge->MergeFrom(hist));
+  hist_for_merge->MergeFrom(hist);
   ASSERT_EQ(2, hist->histogram()->MinValue());
   ASSERT_EQ(4, hist->histogram()->MeanValue());
   ASSERT_EQ(6, hist->histogram()->MaxValue());
@@ -378,7 +378,7 @@ TEST_F(MetricsTest, SimpleHistogramMergeTest) {
   ASSERT_EQ(3, hist_for_merge->histogram()->ValueAtPercentile(50.0));
   ASSERT_EQ(3, hist_for_merge->histogram()->ValueAtPercentile(90.0));
   ASSERT_EQ(6, hist_for_merge->histogram()->ValueAtPercentile(100.0));
-  ASSERT_TRUE(hist_for_merge->MergeFrom(hist_for_merge));
+  hist_for_merge->MergeFrom(hist_for_merge);
   ASSERT_EQ(6, hist_for_merge->histogram()->TotalCount());
   ASSERT_EQ(18, hist_for_merge->histogram()->TotalSum());
 }
diff --git a/src/kudu/util/metrics.cc b/src/kudu/util/metrics.cc
index 8f1f0c8..6102ef7 100644
--- a/src/kudu/util/metrics.cc
+++ b/src/kudu/util/metrics.cc
@@ -708,9 +708,9 @@ void StringGauge::set_value(const string& value) {
   unique_values_.clear();
 }
 
-bool StringGauge::MergeFrom(const scoped_refptr<Metric>& other) {
+void StringGauge::MergeFrom(const scoped_refptr<Metric>& other) {
   if (PREDICT_FALSE(this == other.get())) {
-    return true;
+    return;
   }
   UpdateModificationEpoch();
 
@@ -720,7 +720,6 @@ bool StringGauge::MergeFrom(const scoped_refptr<Metric>& other)
{
   std::lock_guard<simple_spinlock> l(lock_);
   FillUniqueValuesUnlocked();
   unique_values_.insert(other_values.begin(), other_values.end());
-  return true;
 }
 
 void StringGauge::WriteValue(JsonWriter* writer) const {
diff --git a/src/kudu/util/metrics.h b/src/kudu/util/metrics.h
index 82b46a9..072c556 100644
--- a/src/kudu/util/metrics.h
+++ b/src/kudu/util/metrics.h
@@ -746,9 +746,8 @@ class Metric : public RefCountedThreadSafe<Metric> {
   static void IncrementEpoch();
 
   // Merges 'other' into this Metric object.
-  // Return false if any error occurs, otherwise return true.
-  // NOTE: If merge with self, do nothing and return true directly.
-  virtual bool MergeFrom(const scoped_refptr<Metric>& other) = 0;
+  // NOTE: If merge with self, do nothing.
+  virtual void MergeFrom(const scoped_refptr<Metric>& other) = 0;
 
  protected:
   explicit Metric(const MetricPrototype* prototype);
@@ -954,7 +953,7 @@ class StringGauge : public Gauge {
   virtual bool IsUntouched() const override {
     return false;
   }
-  bool MergeFrom(const scoped_refptr<Metric>& other) OVERRIDE;
+  void MergeFrom(const scoped_refptr<Metric>& other) OVERRIDE;
 
  protected:
   FRIEND_TEST(MetricsTest, SimpleStringGaugeForMergeTest);
@@ -1004,11 +1003,10 @@ class AtomicGauge : public Gauge {
   virtual bool IsUntouched() const override {
     return false;
   }
-  bool MergeFrom(const scoped_refptr<Metric>& other) override {
+  void MergeFrom(const scoped_refptr<Metric>& other) override {
     if (PREDICT_TRUE(this != other.get())) {
       IncrementBy(down_cast<AtomicGauge<T>*>(other.get())->value());
     }
-    return true;
   }
  protected:
   virtual void WriteValue(JsonWriter* writer) const OVERRIDE {
@@ -1140,11 +1138,10 @@ class FunctionGauge : public Gauge {
   }
 
   // value() will be constant after MergeFrom()
-  bool MergeFrom(const scoped_refptr<Metric>& other) override {
+  void MergeFrom(const scoped_refptr<Metric>& other) override {
     if (PREDICT_TRUE(this != other.get())) {
       DetachToConstant(value() + down_cast<FunctionGauge<T>*>(other.get())->value());
     }
-    return true;
   }
 
  private:
@@ -1203,11 +1200,10 @@ class Counter : public Metric {
     return value() == 0;
   }
 
-  bool MergeFrom(const scoped_refptr<Metric>& other) override {
+  void MergeFrom(const scoped_refptr<Metric>& other) override {
     if (PREDICT_TRUE(this != other.get())) {
       IncrementBy(down_cast<Counter*>(other.get())->value());
     }
-    return true;
   }
 
  private:
@@ -1278,11 +1274,10 @@ class Histogram : public Metric {
     return TotalCount() == 0;
   }
 
-  bool MergeFrom(const scoped_refptr<Metric>& other) override {
+  void MergeFrom(const scoped_refptr<Metric>& other) override {
     if (PREDICT_TRUE(this != other.get())) {
       histogram_->MergeFrom(*(down_cast<Histogram*>(other.get())->histogram()));
     }
-    return true;
   }
 
  private:


Mime
View raw message