From 2f8cd099752aadbe500e16feaaacdc8ade258cd8 Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Thu, 3 Aug 2023 11:50:47 +0100 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 ca3461b29dff..43065203844e 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -694,6 +694,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", @@ -2416,6 +2417,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) + } +}