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 <[email protected]>
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 <[email protected]>
Tested-by: Adar Dembo <[email protected]>
---
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:
