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/10/17 22:09:19 UTC
[kudu] branch branch-1.11.x updated (0afd48c -> eaae22b)
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a change to branch branch-1.11.x
in repository https://gitbox.apache.org/repos/asf/kudu.git.
from 0afd48c kudu-tool-test: deflake TestFsAddRemoveDataDirEndToEnd
new 26b9943 [clock] add exponential backoff in HybridClock::Init()
new eaae22b [clock] clean up hybrid_clock-test
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/kudu/clock/hybrid_clock-test.cc | 45 +++++++++++++++++++------------------
src/kudu/clock/hybrid_clock.cc | 4 +++-
2 files changed, 26 insertions(+), 23 deletions(-)
[kudu] 01/02: [clock] add exponential backoff in HybridClock::Init()
Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch branch-1.11.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 26b994365e66cd53b65e586f5ab30c2c21d4206c
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Wed Oct 16 15:18:49 2019 -0700
[clock] add exponential backoff in HybridClock::Init()
This patch introduces exponential back-off when polling the
synchronisation status of the underlying time service (i.e. calling
TimeService::WalltimeWithError()) during HybridClock::Init(). Now,
the polling interval starts with 1 millisecond and exponentially grows
up to 1 second.
Prior to this patch, the polling interval was one second, and in case
if time source synchronized faster than in one second,
HybridClock::Init() would incur extra latency (e.g., in case of tests
running with the built-in NTP client synchronized against MiniChronyd).
I verified that with the new approach ExternalMiniCluster-based tests
run faster:
before
[ OK ] AllTypesItest/9.TestTimestampPadding (6794 ms)
after
[ OK ] AllTypesItest/9.TestTimestampPadding (2730 ms)
Change-Id: I75924f1fdcf6a32684cda29bbfa959b00172b50e
Reviewed-on: http://gerrit.cloudera.org:8080/14472
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
(cherry picked from commit 1bb267bd8587473c567ca76426689e33ff4c9b89)
Reviewed-on: http://gerrit.cloudera.org:8080/14476
Reviewed-by: Grant Henke <gr...@apache.org>
---
src/kudu/clock/hybrid_clock.cc | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/kudu/clock/hybrid_clock.cc b/src/kudu/clock/hybrid_clock.cc
index 07170f6..99ea277 100644
--- a/src/kudu/clock/hybrid_clock.cc
+++ b/src/kudu/clock/hybrid_clock.cc
@@ -151,6 +151,7 @@ Status HybridClock::Init() {
Status s;
uint64_t now_usec;
uint64_t error_usec;
+ int poll_backoff_ms = 1;
do {
s = time_service_->WalltimeWithError(&now_usec, &error_usec);
if (!s.IsServiceUnavailable()) {
@@ -168,7 +169,8 @@ Status HybridClock::Init() {
}
need_log = false;
}
- SleepFor(MonoDelta::FromSeconds(1));
+ SleepFor(MonoDelta::FromMilliseconds(poll_backoff_ms));
+ poll_backoff_ms = std::min(2 * poll_backoff_ms, 1000);
} while (MonoTime::Now() < deadline);
if (!s.ok()) {
[kudu] 02/02: [clock] clean up hybrid_clock-test
Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch branch-1.11.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit eaae22b086e70af600efd39781871c5812277442
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Wed Oct 16 16:34:24 2019 -0700
[clock] clean up hybrid_clock-test
This patch contains a small clean up on the HybridClockTest:
* added ASSERT/CHECK for methods returning Status
* added scoped cleanup into TestClockDoesntGoBackwardsWithUpdates
Change-Id: I1c62cf51e11041b2369d29ff4f714876fd352cea
Reviewed-on: http://gerrit.cloudera.org:8080/14474
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
(cherry picked from commit 22f0ea360a5f8b820631248a60fd8c1c879d81e4)
Reviewed-on: http://gerrit.cloudera.org:8080/14490
Reviewed-by: Grant Henke <gr...@apache.org>
---
src/kudu/clock/hybrid_clock-test.cc | 45 +++++++++++++++++++------------------
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/src/kudu/clock/hybrid_clock-test.cc b/src/kudu/clock/hybrid_clock-test.cc
index 837bc9a..a427b8e 100644
--- a/src/kudu/clock/hybrid_clock-test.cc
+++ b/src/kudu/clock/hybrid_clock-test.cc
@@ -37,6 +37,7 @@
#include "kudu/util/monotime.h"
#include "kudu/util/random.h"
#include "kudu/util/random_util.h"
+#include "kudu/util/scoped_cleanup.h"
#include "kudu/util/status.h"
#include "kudu/util/test_macros.h"
#include "kudu/util/test_util.h"
@@ -67,15 +68,15 @@ class HybridClockTest : public KuduTest {
scoped_refptr<HybridClock> clock_;
};
-clock::MockNtp* mock_ntp(const scoped_refptr<HybridClock>& clock) {
+clock::MockNtp* mock_ntp(HybridClock* clock) {
return down_cast<clock::MockNtp*>(clock->time_service());
}
TEST(MockHybridClockTest, TestMockedSystemClock) {
google::FlagSaver saver;
FLAGS_time_source = "mock";
- scoped_refptr<HybridClock> clock(new HybridClock());
- clock->Init();
+ scoped_refptr<HybridClock> clock(new HybridClock);
+ ASSERT_OK(clock->Init());
Timestamp timestamp;
uint64_t max_error_usec;
clock->NowWithError(×tamp, &max_error_usec);
@@ -87,8 +88,8 @@ TEST(MockHybridClockTest, TestMockedSystemClock) {
// Now set an arbitrary time and check that is the time returned by the clock.
uint64_t time = 1234 * 1000;
uint64_t error = 100 * 1000;
- mock_ntp(clock)->SetMockClockWallTimeForTests(time);
- mock_ntp(clock)->SetMockMaxClockErrorForTests(error);
+ mock_ntp(clock.get())->SetMockClockWallTimeForTests(time);
+ mock_ntp(clock.get())->SetMockMaxClockErrorForTests(error);
clock->NowWithError(×tamp, &max_error_usec);
ASSERT_EQ(timestamp.ToUint64(),
HybridClock::TimestampFromMicrosecondsAndLogicalValue(time, 0).ToUint64());
@@ -108,14 +109,14 @@ TEST(MockHybridClockTest, TestMockedSystemClock) {
TEST(MockHybridClockTest, TestClockDealsWithWrapping) {
google::FlagSaver saver;
FLAGS_time_source = "mock";
- scoped_refptr<HybridClock> clock(new HybridClock());
- clock->Init();
- mock_ntp(clock)->SetMockClockWallTimeForTests(1000);
+ scoped_refptr<HybridClock> clock(new HybridClock);
+ ASSERT_OK(clock->Init());
+ mock_ntp(clock.get())->SetMockClockWallTimeForTests(1000);
Timestamp prev = clock->Now();
// Update the clock from 10us in the future
- clock->Update(HybridClock::TimestampFromMicroseconds(1010));
+ ASSERT_OK(clock->Update(HybridClock::TimestampFromMicroseconds(1010)));
// Now read the clock value enough times so that the logical value wraps
// over, and should increment the _physical_ portion of the clock.
@@ -129,7 +130,7 @@ TEST(MockHybridClockTest, TestClockDealsWithWrapping) {
// Advance the time microsecond by microsecond, and ensure the clock never
// goes backwards.
for (int time = 1001; time < 1020; time++) {
- mock_ntp(clock)->SetMockClockWallTimeForTests(time);
+ mock_ntp(clock.get())->SetMockClockWallTimeForTests(time);
Timestamp now = clock->Now();
// Clock should run strictly forwards.
@@ -192,10 +193,7 @@ TEST_F(HybridClockTest, TestWaitUntilAfter_TestCase1) {
Timestamp past_ts_changed = HybridClock::AddPhysicalTimeToTimestamp(
past_ts,
MonoDelta::FromMicroseconds(-3 * static_cast<int64_t>(max_error)));
-
- Status s = clock_->WaitUntilAfter(past_ts_changed, no_deadline);
-
- ASSERT_OK(s);
+ ASSERT_OK(clock_->WaitUntilAfter(past_ts_changed, no_deadline));
MonoTime after = MonoTime::Now();
MonoDelta delta = after - before;
@@ -229,7 +227,7 @@ TEST_F(HybridClockTest, TestWaitUntilAfter_TestCase2) {
{
MonoTime deadline = before;
Status s = clock_->WaitUntilAfter(wait_until, deadline);
- ASSERT_TRUE(s.IsTimedOut());
+ ASSERT_TRUE(s.IsTimedOut()) << s.ToString();
}
// Wait with a deadline well in the future. This should succeed.
@@ -280,16 +278,23 @@ void StresserThread(HybridClock* clock, AtomicBool* stop) {
// Add a random bit of offset to the clock, and perform an update.
Timestamp new_ts = HybridClock::AddPhysicalTimeToTimestamp(
t, MonoDelta::FromMicroseconds(rng.Uniform(10000)));
- clock->Update(new_ts);
+ CHECK_OK(clock->Update(new_ts));
}
}
// Regression test for KUDU-953: if threads are updating and polling the
// clock concurrently, the clock should still never run backwards.
TEST_F(HybridClockTest, TestClockDoesntGoBackwardsWithUpdates) {
- vector<scoped_refptr<kudu::Thread> > threads;
-
+ vector<scoped_refptr<kudu::Thread>> threads;
AtomicBool stop(false);
+
+ SCOPED_CLEANUP({
+ stop.Store(true);
+ for (const auto& t : threads) {
+ t->Join();
+ }
+ });
+
for (int i = 0; i < 4; i++) {
scoped_refptr<Thread> thread;
ASSERT_OK(Thread::Create("test", "stresser",
@@ -299,10 +304,6 @@ TEST_F(HybridClockTest, TestClockDoesntGoBackwardsWithUpdates) {
}
SleepFor(MonoDelta::FromSeconds(1));
- stop.Store(true);
- for (const scoped_refptr<Thread> t : threads) {
- t->Join();
- }
}
TEST_F(HybridClockTest, TestGetPhysicalComponentDifference) {