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/09/28 00:44:46 UTC
[kudu] branch master updated (883367e -> fcbca3d)
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git.
from 883367e [thread] Fix the comment of Thread
new 5b2259a [clock] fix VLOG() misprint in MockNtp
new fcbca3d [clock] change in the semantics of TimeService::Init()
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.cc | 12 +++++++--
src/kudu/clock/hybrid_clock.h | 4 +--
src/kudu/clock/mock_ntp.cc | 7 +++---
src/kudu/clock/mock_ntp.h | 7 +++---
src/kudu/clock/system_ntp.cc | 55 ++++++++++++++++++------------------------
src/kudu/clock/system_ntp.h | 11 +++++----
src/kudu/clock/time_service.h | 18 ++++++++------
7 files changed, 58 insertions(+), 56 deletions(-)
[kudu] 01/02: [clock] fix VLOG() misprint in MockNtp
Posted by al...@apache.org.
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
commit 5b2259a79ca52a6bbbd72386c157d3b37bfec489
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Thu Feb 7 18:45:11 2019 -0800
[clock] fix VLOG() misprint in MockNtp
Moved misplaced VLOG() into appropriate place.
This changelist does not contain any functional modifications.
Change-Id: I5ebd3e97bafa2038779dcf2b14de6a0e08cc4012
Reviewed-on: http://gerrit.cloudera.org:8080/12404
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Alexey Serbin <as...@cloudera.com>
---
src/kudu/clock/mock_ntp.cc | 7 ++++---
src/kudu/clock/mock_ntp.h | 6 +++---
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/kudu/clock/mock_ntp.cc b/src/kudu/clock/mock_ntp.cc
index 5d7aa8a..7924ac6 100644
--- a/src/kudu/clock/mock_ntp.cc
+++ b/src/kudu/clock/mock_ntp.cc
@@ -29,9 +29,8 @@ namespace clock {
Status MockNtp::WalltimeWithError(uint64_t* now_usec, uint64_t* error_usec) {
std::lock_guard<simple_spinlock> lock(lock_);
- VLOG(1) << "Current clock time: " << mock_clock_time_usec_ << " error: "
- << mock_clock_max_error_usec_ << ". Updating to time: " << now_usec
- << " and error: " << error_usec;
+ VLOG(1) << "Current clock time: " << mock_clock_time_usec_
+ << " error: " << mock_clock_max_error_usec_;
*now_usec = mock_clock_time_usec_;
*error_usec = mock_clock_max_error_usec_;
return Status::OK();
@@ -39,12 +38,14 @@ Status MockNtp::WalltimeWithError(uint64_t* now_usec, uint64_t* error_usec) {
void MockNtp::SetMockClockWallTimeForTests(uint64_t now_usec) {
std::lock_guard<simple_spinlock> lock(lock_);
+ VLOG(1) << "Updating to time: " << now_usec;
CHECK_GE(now_usec, mock_clock_time_usec_);
mock_clock_time_usec_ = now_usec;
}
void MockNtp::SetMockMaxClockErrorForTests(uint64_t max_error_usec) {
std::lock_guard<simple_spinlock> lock(lock_);
+ VLOG(1) << "Updating to error: " << max_error_usec;
mock_clock_max_error_usec_ = max_error_usec;
}
diff --git a/src/kudu/clock/mock_ntp.h b/src/kudu/clock/mock_ntp.h
index 0878c5d..94d4a06 100644
--- a/src/kudu/clock/mock_ntp.h
+++ b/src/kudu/clock/mock_ntp.h
@@ -34,13 +34,13 @@ class MockNtp : public TimeService {
MockNtp() = default;
virtual ~MockNtp() = default;
- virtual Status Init() override {
+ Status Init() override {
return Status::OK();
}
- virtual Status WalltimeWithError(uint64_t* now_usec, uint64_t* error_usec) override;
+ Status WalltimeWithError(uint64_t* now_usec, uint64_t* error_usec) override;
- virtual int64_t skew_ppm() const override {
+ int64_t skew_ppm() const override {
// Just return the same constant as the default configuration for NTP:
// the local clock frequency may accumulate error at a max rate of
// 500us per second.
[kudu] 02/02: [clock] change in the semantics of TimeService::Init()
Posted by al...@apache.org.
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
commit fcbca3dd1234a2c0a41b6dcf9f1d7b6410f33c34
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Fri Sep 27 12:39:27 2019 -0700
[clock] change in the semantics of TimeService::Init()
This patch introduces a change in the semantics of the TimeService's
initialization method Init(). With this patch, TimeService::Init()
no longer waits for the clock synchronisation to happen for NTP-based
time services. Users of the TimeService interface should rely on
TimeService::WalltimeWithError() to get the status of the clock
synchronisation after calling TimeService::Init().
This is a follow-up to e110cb88b145edb6d536ae0806e02736f5bf51e5.
Change-Id: Ie7245b2cd2c3001a508235f9e539d7202d3ca994
Reviewed-on: http://gerrit.cloudera.org:8080/14317
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
src/kudu/clock/hybrid_clock.cc | 12 +++++++--
src/kudu/clock/hybrid_clock.h | 4 +--
src/kudu/clock/mock_ntp.h | 1 -
src/kudu/clock/system_ntp.cc | 55 ++++++++++++++++++------------------------
src/kudu/clock/system_ntp.h | 11 +++++----
src/kudu/clock/time_service.h | 18 ++++++++------
6 files changed, 51 insertions(+), 50 deletions(-)
diff --git a/src/kudu/clock/hybrid_clock.cc b/src/kudu/clock/hybrid_clock.cc
index 6db1990..0163271 100644
--- a/src/kudu/clock/hybrid_clock.cc
+++ b/src/kudu/clock/hybrid_clock.cc
@@ -134,13 +134,19 @@ Status HybridClock::Init() {
} else {
return Status::InvalidArgument("invalid NTP source", FLAGS_time_source);
}
+ RETURN_NOT_OK(time_service_->Init());
+ // Make sure the underlying clock service is available (e.g., for NTP-based
+ // clock make sure it's synchronized with its NTP source). If requested, wait
+ // up to the specified timeout for the clock to become ready to use.
const auto wait_s = FLAGS_ntp_initial_sync_wait_secs;
const auto deadline = MonoTime::Now() + MonoDelta::FromSeconds(wait_s);
bool need_log = true;
Status s;
+ uint64_t now_usec;
+ uint64_t error_usec;
do {
- s = time_service_->Init();
+ s = time_service_->WalltimeWithError(&now_usec, &error_usec);
if (!s.IsServiceUnavailable()) {
break;
}
@@ -164,8 +170,10 @@ Status HybridClock::Init() {
return s.CloneAndPrepend("timed out waiting for clock synchronisation");
}
+ LOG(INFO) << Substitute("HybridClock initialized: "
+ "now $0 us; error $1 us; skew $2 ppm",
+ now_usec, error_usec, time_service_->skew_ppm());
state_ = kInitialized;
-
return Status::OK();
}
diff --git a/src/kudu/clock/hybrid_clock.h b/src/kudu/clock/hybrid_clock.h
index ed23696..db4cfef 100644
--- a/src/kudu/clock/hybrid_clock.h
+++ b/src/kudu/clock/hybrid_clock.h
@@ -158,13 +158,11 @@ class HybridClock : public Clock {
}
private:
- friend class TestNtp;
-
// Obtains the current wallclock time and maximum error in microseconds,
// and checks if the clock is synchronized.
//
// On OS X, the error will always be 0.
- kudu::Status WalltimeWithError(uint64_t* now_usec, uint64_t* error_usec);
+ Status WalltimeWithError(uint64_t* now_usec, uint64_t* error_usec);
// Same as above, but exits with a FATAL if there is an error.
void WalltimeWithErrorOrDie(uint64_t* now_usec, uint64_t* error_usec);
diff --git a/src/kudu/clock/mock_ntp.h b/src/kudu/clock/mock_ntp.h
index 94d4a06..2f71295 100644
--- a/src/kudu/clock/mock_ntp.h
+++ b/src/kudu/clock/mock_ntp.h
@@ -32,7 +32,6 @@ namespace clock {
class MockNtp : public TimeService {
public:
MockNtp() = default;
- virtual ~MockNtp() = default;
Status Init() override {
return Status::OK();
diff --git a/src/kudu/clock/system_ntp.cc b/src/kudu/clock/system_ntp.cc
index 9163175..b3a278d 100644
--- a/src/kudu/clock/system_ntp.cc
+++ b/src/kudu/clock/system_ntp.cc
@@ -21,6 +21,7 @@
#include <sys/timex.h>
#include <cerrno>
+#include <limits>
#include <ostream>
#include <string>
#include <vector>
@@ -106,6 +107,29 @@ void TryRun(vector<string> cmd, vector<string>* log) {
} // anonymous namespace
+SystemNtp::SystemNtp()
+ : skew_ppm_(std::numeric_limits<int64_t>::max()) {
+}
+
+Status SystemNtp::WalltimeWithError(uint64_t* now_usec, uint64_t* error_usec) {
+ // Read the time. This will return an error if the clock is not synchronized.
+ timex tx;
+ RETURN_NOT_OK(CallAdjTime(&tx));
+
+ // Calculate the sleep skew adjustment according to the max tolerance of the clock.
+ // Tolerance comes in parts per million but needs to be applied a scaling factor.
+ skew_ppm_ = tx.tolerance / kAdjtimexScalingFactor;
+
+ if (tx.status & STA_NANO) {
+ tx.time.tv_usec /= 1000;
+ }
+ DCHECK_LE(tx.time.tv_usec, 1e6);
+
+ *now_usec = tx.time.tv_sec * kMicrosPerSec + tx.time.tv_usec;
+ *error_usec = tx.maxerror;
+ return Status::OK();
+}
+
void SystemNtp::DumpDiagnostics(vector<string>* log) const {
LOG_STRING(ERROR, log) << "Dumping NTP diagnostics";
TryRun({"ntptime"}, log);
@@ -131,36 +155,5 @@ void SystemNtp::DumpDiagnostics(vector<string>* log) const {
TryRun({"chronyc", "-n", "sources"}, log);
}
-Status SystemNtp::Init() {
- timex timex;
- RETURN_NOT_OK(CallAdjTime(&timex));
-
- // Calculate the sleep skew adjustment according to the max tolerance of the clock.
- // Tolerance comes in parts per million but needs to be applied a scaling factor.
- skew_ppm_ = timex.tolerance / kAdjtimexScalingFactor;
-
- LOG(INFO) << "NTP initialized."
- << " Skew: " << skew_ppm_ << "ppm"
- << " Current error: " << timex.maxerror << "us";
-
- return Status::OK();
-}
-
-Status SystemNtp::WalltimeWithError(uint64_t *now_usec,
- uint64_t *error_usec) {
- // Read the time. This will return an error if the clock is not synchronized.
- timex tx;
- RETURN_NOT_OK(CallAdjTime(&tx));
-
- if (tx.status & STA_NANO) {
- tx.time.tv_usec /= 1000;
- }
- DCHECK_LE(tx.time.tv_usec, 1e6);
-
- *now_usec = tx.time.tv_sec * kMicrosPerSec + tx.time.tv_usec;
- *error_usec = tx.maxerror;
- return Status::OK();
-}
-
} // namespace clock
} // namespace kudu
diff --git a/src/kudu/clock/system_ntp.h b/src/kudu/clock/system_ntp.h
index 5fd8fe7..3b3a723 100644
--- a/src/kudu/clock/system_ntp.h
+++ b/src/kudu/clock/system_ntp.h
@@ -16,6 +16,7 @@
// under the License.
#pragma once
+#include <atomic>
#include <cstdint>
#include <string>
#include <vector>
@@ -35,11 +36,11 @@ namespace clock {
// to keep the kernel's timekeeping up to date and in sync.
class SystemNtp : public TimeService {
public:
- SystemNtp() = default;
+ SystemNtp();
- // Ensure that the kernel's timekeeping status indicates that it is currently
- // in sync, and initialize various internal parameters.
- virtual Status Init() override;
+ virtual Status Init() override {
+ return Status::OK();
+ }
virtual Status WalltimeWithError(uint64_t* now_usec, uint64_t* error_usec) override;
@@ -57,7 +58,7 @@ class SystemNtp : public TimeService {
static const uint64_t kMicrosPerSec;
// The skew rate in PPM reported by the kernel.
- uint64_t skew_ppm_ = 0;
+ std::atomic<int64_t> skew_ppm_;
DISALLOW_COPY_AND_ASSIGN(SystemNtp);
};
diff --git a/src/kudu/clock/time_service.h b/src/kudu/clock/time_service.h
index cc28714..6851bbf 100644
--- a/src/kudu/clock/time_service.h
+++ b/src/kudu/clock/time_service.h
@@ -32,19 +32,21 @@ class TimeService {
TimeService() = default;
virtual ~TimeService() = default;
- // Initialize the NTP source, validating that it is available and properly
- // synchronized: Status::OK() is returned in such case. If the source
- // is not yet synchronized, then Status::ServiceUnavailable() is returned:
- // a caller may call this method again to eventually get Status::OK().
- // In case of other non-OK() return statuses, the caller should not invoke
- // this method again.
+ // Initialize the clock source. No verification is performed on the
+ // synchronisation status of the clock. To verify the clock is synchronized
+ // and the time service is ready to use, make sure WalltimeWithError() returns
+ // Status::OK() after calling Init(). The Init() method it itself should be
+ // called only once.
virtual Status Init() = 0;
// Return the current wall time in microseconds since the Unix epoch in '*now_usec'.
// The current maximum error bound in microseconds is returned in '*error_usec'.
+ // Neither of the output parameters can be null.
//
- // May return a bad Status if the NTP service has become unsynchronized or otherwise
- // unavailable.
+ // May return a bad Status if the NTP service has become unsynchronized or
+ // otherwise unavailable. In case if the method returns ServiceUnavailable()
+ // after calling Init(), the caller may call this method again and eventually
+ // get Status::OK() if anticipating the clock synchronisation to happen soon.
virtual Status WalltimeWithError(uint64_t* now_usec, uint64_t* error_usec) = 0;
// Return the estimated max amount of clock skew as configured by this NTP service.