Skip to content

Commit

Permalink
[docdb] Initialize timestamp_ms_ for MillisLag (#5081, #5762)
Browse files Browse the repository at this point in the history
Summary:
Before we were not initializing `timestamp_ms_` which caused flaky errors for `MetricsTest.SimpleLagTest` - changing this to now initialize to `now`.
Also fixing ASAN issue in `MetricsTest::DoLagTest`

Test Plan:
```
ybd --cxx-test util_metrics-test --gtest_filter MetricsTest.SimpleLagTest -n 100
ybd --cxx-test util_metrics-test --gtest_filter MetricsTest.SimpleAtomicLagTest -n 100
```

Reviewers: hector

Reviewed By: hector

Subscribers: bogdan, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D9431
  • Loading branch information
hulien22 committed Sep 23, 2020
1 parent a508b63 commit e5494f1
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 8 deletions.
18 changes: 12 additions & 6 deletions src/yb/util/metrics-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,22 @@ class MetricsTest : public YBTest {
protected:
template <class LagType>
void DoLagTest(const MillisLagPrototype& metric) {
auto lag = new LagType(&metric);
scoped_refptr<LagType> lag = new LagType(&metric);
ASSERT_EQ(metric.description(), lag->prototype()->description());
SleepFor(MonoDelta::FromMilliseconds(500));
// Internal timestamp is set to the time when the metric was created.
// So this lag is measure of the time elapsed since the metric was
// created and the check time.
ASSERT_GE(lag->lag_ms(), 500);
auto now_ms = std::chrono::duration_cast<std::chrono::milliseconds>(
std::chrono::system_clock::now().time_since_epoch()).count();
SleepFor(MonoDelta::FromMilliseconds(100));
ASSERT_LT(now_ms, lag->lag_ms());
lag->UpdateTimestampInMilliseconds(now_ms);
ASSERT_GT(10000, lag->lag_ms());
// Set the timestamp to some time in the future to verify that the metric can correctly deal
// with this case.
// Verify that the update happened correctly. The lag time should
// be close to 0, but giving it extra time to account for slow
// tests.
ASSERT_LT(lag->lag_ms(), 200);
// Set the timestamp to some time in the future to verify that the
// metric can correctly deal with this case.
lag->UpdateTimestampInMilliseconds(now_ms * 2);
ASSERT_EQ(0, lag->lag_ms());
}
Expand Down
11 changes: 10 additions & 1 deletion src/yb/util/metrics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,10 @@ scoped_refptr<MillisLag> MillisLagPrototype::Instantiate(
return entity->FindOrCreateMillisLag(this);
}

MillisLag::MillisLag(const MillisLagPrototype* proto) : Metric(proto) {
MillisLag::MillisLag(const MillisLagPrototype* proto)
: Metric(proto),
timestamp_ms_(static_cast<int64_t>(std::chrono::duration_cast<std::chrono::milliseconds>(
std::chrono::system_clock::now().time_since_epoch()).count())) {
}

Status MillisLag::WriteAsJson(JsonWriter* writer, const MetricJsonOptions& opts) const {
Expand Down Expand Up @@ -809,6 +812,12 @@ Status MillisLag::WriteForPrometheus(
return writer->WriteSingleEntry(attr, prototype_->name(), lag_ms());
}

AtomicMillisLag::AtomicMillisLag(const MillisLagPrototype* proto)
: MillisLag(proto),
atomic_timestamp_ms_(static_cast<int64_t>(std::chrono::duration_cast<std::chrono::milliseconds>(
std::chrono::system_clock::now().time_since_epoch()).count())) {
}

Status AtomicMillisLag::WriteAsJson(JsonWriter* writer, const MetricJsonOptions& opts) const {
if (prototype_->level() < opts.level) {
return Status::OK();
Expand Down
2 changes: 1 addition & 1 deletion src/yb/util/metrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -1345,7 +1345,7 @@ class MillisLag : public Metric {

class AtomicMillisLag : public MillisLag {
public:
explicit AtomicMillisLag(const MillisLagPrototype* proto) : MillisLag(proto) {}
explicit AtomicMillisLag(const MillisLagPrototype* proto);

int64_t lag_ms() const override {
return std::max(static_cast<int64_t>(0),
Expand Down

0 comments on commit e5494f1

Please sign in to comment.