From 24d368a52164b4ce858610c7877b0d073652b59c Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Thu, 3 Aug 2023 10:50:47 +0000 Subject: [PATCH] timeutil/ptp: fix conversion from seconds to Time Epic: none Release note (bug fix): since 22.2.0, using a PTP clock device (enabled by the --clock-device flag) would generate timestamps in the far future. It now generates the correct time. This could cause nodes to crash due to incorrect timestamps, or in the worst case irreversibly advance the cluster's HLC clock into the far future. --- pkg/BUILD.bazel | 2 ++ pkg/util/timeutil/ptp/BUILD.bazel | 18 ++++++++++- pkg/util/timeutil/ptp/ptp_clock_linux.go | 7 ++++- pkg/util/timeutil/ptp/ptp_clock_linux_test.go | 30 +++++++++++++++++++ 4 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 pkg/util/timeutil/ptp/ptp_clock_linux_test.go diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 1738995ac5f8..c1c33429eb30 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -671,6 +671,7 @@ ALL_TESTS = [ "//pkg/util/timeofday:timeofday_test", "//pkg/util/timetz:timetz_test", "//pkg/util/timeutil/pgdate:pgdate_test", + "//pkg/util/timeutil/ptp:ptp_test", "//pkg/util/timeutil:timeutil_test", "//pkg/util/tochar:tochar_test", "//pkg/util/tracing/collector:collector_test", @@ -2320,6 +2321,7 @@ GO_TARGETS = [ "//pkg/util/timeutil/pgdate:pgdate", "//pkg/util/timeutil/pgdate:pgdate_test", "//pkg/util/timeutil/ptp:ptp", + "//pkg/util/timeutil/ptp:ptp_test", "//pkg/util/timeutil:timeutil", "//pkg/util/timeutil:timeutil_test", "//pkg/util/tochar:tochar", diff --git a/pkg/util/timeutil/ptp/BUILD.bazel b/pkg/util/timeutil/ptp/BUILD.bazel index 76eb9d5a1e9a..c1505a597fc1 100644 --- a/pkg/util/timeutil/ptp/BUILD.bazel +++ b/pkg/util/timeutil/ptp/BUILD.bazel @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "ptp", @@ -59,3 +59,19 @@ go_library( "//conditions:default": [], }), ) + +go_test( + name = "ptp_test", + srcs = ["ptp_clock_linux_test.go"], + args = ["-test.timeout=295s"], + embed = [":ptp"], + deps = select({ + "@io_bazel_rules_go//go/platform:android": [ + "//pkg/util/timeutil", + ], + "@io_bazel_rules_go//go/platform:linux": [ + "//pkg/util/timeutil", + ], + "//conditions:default": [], + }), +) diff --git a/pkg/util/timeutil/ptp/ptp_clock_linux.go b/pkg/util/timeutil/ptp/ptp_clock_linux.go index 3f2a880a28b5..4bbbcea6782f 100644 --- a/pkg/util/timeutil/ptp/ptp_clock_linux.go +++ b/pkg/util/timeutil/ptp/ptp_clock_linux.go @@ -79,5 +79,10 @@ func (p Clock) Now() time.Time { panic(err) } - return timeutil.Unix(int64(ts.tv_sec)*1e9, int64(ts.tv_nsec)) + return timeutil.Unix(int64(ts.tv_sec), int64(ts.tv_nsec)) +} + +// realtime returns a clock using the system CLOCK_REALTIME device. For testing. +func realtime() Clock { + return Clock{clockDeviceID: uintptr(C.CLOCK_REALTIME)} } diff --git a/pkg/util/timeutil/ptp/ptp_clock_linux_test.go b/pkg/util/timeutil/ptp/ptp_clock_linux_test.go new file mode 100644 index 000000000000..01e915bf9983 --- /dev/null +++ b/pkg/util/timeutil/ptp/ptp_clock_linux_test.go @@ -0,0 +1,30 @@ +// Copyright 2023 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +//go:build linux +// +build linux + +package ptp + +import ( + "testing" + "time" + + "github.com/cockroachdb/cockroach/pkg/util/timeutil" +) + +// TestClockNow sanity checks that Clock.Now() sourced from the CLOCK_REALTIME +// device returns time close to timeutil.Now(). This ensures that the conversion +// from the time returned by a clock device to Go's time.Time is correct. +func TestClockNow(t *testing.T) { + if got, want := realtime().Now(), timeutil.Now(); want.Sub(got).Abs() > 10*time.Second { + t.Errorf("clock mismatch: got %v; timeutil says %v", got, want) + } +}