From 13693daca76623326974d0705fdbc16f790cec58 Mon Sep 17 00:00:00 2001 From: Oliver Tan Date: Fri, 9 Oct 2020 09:55:54 -0700 Subject: [PATCH] timeutil: embed time zone data into CockroachDB * Use 4d63.com/tz to embed timezone data. * Modify timeutil.LoadLocation to use this data. * Remove cli checks for tzdata. * Add lint rule to use timeutil.LoadLocation The binary size changes by ~1MB here. Release note (general change): The timezone data is now built in to the CockroachDB binary, thus no longer relying on the system tzdata for locations. --- go.mod | 2 ++ go.sum | 6 ++++ pkg/ccl/serverccl/role_authentication_test.go | 2 +- pkg/cli/demo.go | 4 --- pkg/cli/start.go | 36 ------------------- pkg/server/authentication_test.go | 2 +- pkg/sql/sem/tree/datum_test.go | 3 +- pkg/testutils/lint/lint_test.go | 5 +-- pkg/util/timetz/timetz_test.go | 2 +- pkg/util/timeutil/pgdate/parsing_test.go | 5 +-- pkg/util/timeutil/pgdate/zone_cache.go | 7 ++-- pkg/util/timeutil/time_zone_util_test.go | 2 +- pkg/util/timeutil/zoneinfo.go | 20 ++++------- vendor | 2 +- 14 files changed, 32 insertions(+), 66 deletions(-) diff --git a/go.mod b/go.mod index fc548936c62e..640c39755209 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,8 @@ module github.com/cockroachdb/cockroach go 1.13 require ( + 4d63.com/embedfiles v1.0.0 // indirect + 4d63.com/tz v1.2.0 cloud.google.com/go v0.34.0 github.com/Azure/azure-sdk-for-go v33.4.0+incompatible github.com/Azure/azure-storage-blob-go v0.0.0-20190104215108-45d0c5e3638e diff --git a/go.sum b/go.sum index c24cad742cab..9ad10fad12cf 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,9 @@ +4d63.com/embedfiles v0.0.0-20190311033909-995e0740726f h1:oyYjGRBNq1TxAIG8aHqtxlvqUfzdZf+MbcRb/oweNfY= +4d63.com/embedfiles v0.0.0-20190311033909-995e0740726f/go.mod h1:HxEsUxoVZyRxsZML/S6e2xAuieFMlGO0756ncWx1aXE= +4d63.com/embedfiles v1.0.0 h1:AR4j5WItSJwBX9SapkvmQUGLPlgCHQZaCDQ52zLXzZM= +4d63.com/embedfiles v1.0.0/go.mod h1:U0e+fedkrGPVJiU29PWZQ7pHHZRPiQAzwDJocZ4d3PE= +4d63.com/tz v1.2.0 h1:EpJt060xY+M+M0Wj8btz+THdOJbSxj4i8jhVQP3Wr0U= +4d63.com/tz v1.2.0/go.mod h1:SHGqVdL7hd2ZaX2T9uEiOZ/OFAUfCCLURdLPJsd8ZNs= cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.34.0 h1:eOI3/cP2VTU6uZLDYAoic+eyzzB9YyGmJ7eIjl8rOPg= cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= diff --git a/pkg/ccl/serverccl/role_authentication_test.go b/pkg/ccl/serverccl/role_authentication_test.go index 7fe62378e2ba..ec6404123d13 100644 --- a/pkg/ccl/serverccl/role_authentication_test.go +++ b/pkg/ccl/serverccl/role_authentication_test.go @@ -47,7 +47,7 @@ func TestVerifyPassword(t *testing.T) { } //location is used for timezone testing. - shanghaiLoc, err := time.LoadLocation("Asia/Shanghai") + shanghaiLoc, err := timeutil.LoadLocation("Asia/Shanghai") if err != nil { t.Fatal(err) } diff --git a/pkg/cli/demo.go b/pkg/cli/demo.go index e4b0969f3e02..ca4847da9899 100644 --- a/pkg/cli/demo.go +++ b/pkg/cli/demo.go @@ -269,10 +269,6 @@ func runDemo(cmd *cobra.Command, gen workload.Generator) (err error) { } defer c.cleanup(ctx) - if err := checkTzDatabaseAvailability(ctx); err != nil { - return err - } - loc, err := geos.EnsureInit(geos.EnsureInitErrorDisplayPrivate, startCtx.geoLibsDir) if err != nil { log.Infof(ctx, "could not initialize GEOS - spatial functions may not be available: %v", err) diff --git a/pkg/cli/start.go b/pkg/cli/start.go index 1faeaf2e8ead..ee41f33f0b97 100644 --- a/pkg/cli/start.go +++ b/pkg/cli/start.go @@ -403,13 +403,6 @@ func runStart(cmd *cobra.Command, args []string, startSingleNode bool) error { // registered. reportConfiguration(ctx) - // Until/unless CockroachDB embeds its own tz database, we want - // an early sanity check. It's better to inform the user early - // than to get surprising errors during SQL queries. - if err := checkTzDatabaseAvailability(ctx); err != nil { - return errors.Wrap(err, "failed to initialize node") - } - // ReadyFn will be called when the server has started listening on // its network sockets, but perhaps before it has done bootstrapping // and thus before Start() completes. @@ -923,35 +916,6 @@ func clientFlagsRPC() string { return strings.Join(flags, " ") } -func checkTzDatabaseAvailability(ctx context.Context) error { - if _, err := timeutil.LoadLocation("America/New_York"); err != nil { - log.Errorf(ctx, "timeutil.LoadLocation: %v", err) - reportedErr := errors.WithHint( - errors.WithIssueLink( - errors.New("unable to load named timezones"), - errors.IssueLink{IssueURL: unimplemented.MakeURL(36864)}), - "Check that the time zone database is installed on your system, or\n"+ - "set the ZONEINFO environment variable to a Go time zone .zip archive.") - - if envutil.EnvOrDefaultBool("COCKROACH_INCONSISTENT_TIME_ZONES", false) { - // The user tells us they really know what they want. - reportedErr := &formattedError{err: reportedErr} - log.Shoutf(ctx, log.Severity_WARNING, "%v", reportedErr) - } else { - // Prevent a successful start. - // - // In the past, we were simply using log.Shout to emit an error, - // informing the user that startup could continue with degraded - // behavior. However, usage demonstrated that users typically do - // not see the error and instead run into silently incorrect SQL - // results. To avoid this situation altogether, it's better to - // stop early. - return reportedErr - } - } - return nil -} - func reportConfiguration(ctx context.Context) { serverCfg.Report(ctx) if envVarsUsed := envutil.GetEnvVarsUsed(); len(envVarsUsed) > 0 { diff --git a/pkg/server/authentication_test.go b/pkg/server/authentication_test.go index 0b139af4d6ab..0c760081519c 100644 --- a/pkg/server/authentication_test.go +++ b/pkg/server/authentication_test.go @@ -205,7 +205,7 @@ func TestVerifyPassword(t *testing.T) { } //location is used for timezone testing. - shanghaiLoc, err := time.LoadLocation("Asia/Shanghai") + shanghaiLoc, err := timeutil.LoadLocation("Asia/Shanghai") if err != nil { t.Fatal(err) } diff --git a/pkg/sql/sem/tree/datum_test.go b/pkg/sql/sem/tree/datum_test.go index 5f1d7f7cb86d..2fc283f40054 100644 --- a/pkg/sql/sem/tree/datum_test.go +++ b/pkg/sql/sem/tree/datum_test.go @@ -27,6 +27,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/timeofday" "github.com/cockroachdb/cockroach/pkg/util/timetz" + "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -850,7 +851,7 @@ func TestDTimeTZ(t *testing.T) { require.False(t, depOnCtx) // No daylight savings in Hawaii! - hawaiiZone, err := time.LoadLocation("Pacific/Honolulu") + hawaiiZone, err := timeutil.LoadLocation("Pacific/Honolulu") require.NoError(t, err) hawaiiTime := tree.NewDTimeTZFromLocation(timeofday.New(1, 14, 15, 0), hawaiiZone) diff --git a/pkg/testutils/lint/lint_test.go b/pkg/testutils/lint/lint_test.go index 1570a39d22a7..d4d2ec962b22 100644 --- a/pkg/testutils/lint/lint_test.go +++ b/pkg/testutils/lint/lint_test.go @@ -708,13 +708,14 @@ func TestLint(t *testing.T) { "git", "grep", "-nE", - `\btime\.(Now|Since|Unix)\(`, + `\btime\.(Now|Since|Unix|LoadLocation)\(`, "--", "*.go", ":!**/embedded.go", - ":!util/timeutil/time.go", ":!util/timeutil/now_unix.go", ":!util/timeutil/now_windows.go", + ":!util/timeutil/time.go", + ":!util/timeutil/zoneinfo.go", ":!util/tracing/tracer_span.go", ":!util/tracing/tracer.go", ) diff --git a/pkg/util/timetz/timetz_test.go b/pkg/util/timetz/timetz_test.go index 488cb564dca6..5be5e3bc6977 100644 --- a/pkg/util/timetz/timetz_test.go +++ b/pkg/util/timetz/timetz_test.go @@ -83,7 +83,7 @@ func TestTimeTZ(t *testing.T) { require.False(t, depOnCtx) // No daylight savings in Hawaii! - hawaiiZone, err := time.LoadLocation("Pacific/Honolulu") + hawaiiZone, err := timeutil.LoadLocation("Pacific/Honolulu") require.NoError(t, err) hawaiiTime := MakeTimeTZFromLocation(timeofday.New(1, 14, 15, 0), hawaiiZone) diff --git a/pkg/util/timeutil/pgdate/parsing_test.go b/pkg/util/timeutil/pgdate/parsing_test.go index 81fd480a0937..773698940f08 100644 --- a/pkg/util/timeutil/pgdate/parsing_test.go +++ b/pkg/util/timeutil/pgdate/parsing_test.go @@ -19,6 +19,7 @@ import ( "testing" "time" + "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/cockroachdb/cockroach/pkg/util/timeutil/pgdate" _ "github.com/lib/pq" ) @@ -811,7 +812,7 @@ func BenchmarkParseTimestampComparison(b *testing.B) { } // bench compares our ParseTimestamp to ParseInLocation, optionally -// chained with a time.LoadLocation() for resolving named zones. +// chained with a timeutil.LoadLocation() for resolving named zones. // The layout parameter is only used for time.ParseInLocation(). // When a named timezone is used, it must be passed via locationName // so that it may be resolved to a time.Location. It will be @@ -842,7 +843,7 @@ func bench(b *testing.B, layout string, s string, locationName string) { loc := time.UTC if locationName != "" { var err error - loc, err = time.LoadLocation(locationName) + loc, err = timeutil.LoadLocation(locationName) if err != nil { b.Fatal(err) } diff --git a/pkg/util/timeutil/pgdate/zone_cache.go b/pkg/util/timeutil/pgdate/zone_cache.go index 37853d4d6ca8..cd7f1cffe9de 100644 --- a/pkg/util/timeutil/pgdate/zone_cache.go +++ b/pkg/util/timeutil/pgdate/zone_cache.go @@ -15,11 +15,12 @@ import ( "time" "github.com/cockroachdb/cockroach/pkg/util/syncutil" + "github.com/cockroachdb/cockroach/pkg/util/timeutil" ) // zoneCache stores the results of resolving time.Location instances. // -// time.LoadLocation does not perform caching internally and requires +// timeutil.LoadLocation does not perform caching internally and requires // many disk accesses to locate the named zoneinfo file. // // We also save time.FixedZone calls to avoid needing to regenerate @@ -46,7 +47,7 @@ type zoneCacheEntry struct { err error } -// LoadLocation wraps time.LoadLocation which does not perform +// LoadLocation wraps timeutil.LoadLocation which does not perform // caching internally and which requires many disk accesses to // locate the named zoneinfo file. func (z *zoneCache) LoadLocation(zone string) (*time.Location, error) { @@ -55,7 +56,7 @@ func (z *zoneCache) LoadLocation(zone string) (*time.Location, error) { z.mu.Unlock() if !ok { - loc, err := time.LoadLocation(zone) + loc, err := timeutil.LoadLocation(zone) entry = &zoneCacheEntry{loc: loc, err: err} z.mu.Lock() diff --git a/pkg/util/timeutil/time_zone_util_test.go b/pkg/util/timeutil/time_zone_util_test.go index 1bc5136fc5dd..a7d3d2bf9f15 100644 --- a/pkg/util/timeutil/time_zone_util_test.go +++ b/pkg/util/timeutil/time_zone_util_test.go @@ -20,7 +20,7 @@ import ( ) func TestTimeZoneStringToLocation(t *testing.T) { - aus, err := time.LoadLocation("Australia/Sydney") + aus, err := LoadLocation("Australia/Sydney") require.NoError(t, err) testCases := []struct { diff --git a/pkg/util/timeutil/zoneinfo.go b/pkg/util/timeutil/zoneinfo.go index e25070f30b78..762f7c66fe9c 100644 --- a/pkg/util/timeutil/zoneinfo.go +++ b/pkg/util/timeutil/zoneinfo.go @@ -14,21 +14,19 @@ import ( "strings" "time" - "github.com/cockroachdb/errors" + "4d63.com/tz" ) -var errTZDataNotFound = errors.New("timezone data cannot be found") - // LoadLocation returns the time.Location with the given name. // The name is taken to be a location name corresponding to a file // in the IANA Time Zone database, such as "America/New_York". // // We do not use Go's time.LoadLocation() directly because: -// 1) it maps "Local" to the local time zone, whereas we want UTC. -// 2) when a tz is not found, it reports some garbage message -// related to zoneinfo.zip, which we don't ship, instead -// of a more useful message like "the tz file with such name -// is not present in one of the standard tz locations". +// 1) It maps "Local" to the local time zone, whereas we want UTC. +// 2) It depends on the operating system's installation of zoneinfo.zip. +// Instead, we use 4d63.com/tz's embedded time. +// In Go1.15+, we can re-use time.LoadLocation if we include +// https://tip.golang.org/doc/go1.15#time/tzdata. func LoadLocation(name string) (*time.Location, error) { switch strings.ToLower(name) { case "local", "default": @@ -39,9 +37,5 @@ func LoadLocation(name string) (*time.Location, error) { // case-insensitive lookup. name = "UTC" } - l, err := time.LoadLocation(name) - if err != nil && strings.Contains(err.Error(), "zoneinfo.zip") { - err = errTZDataNotFound - } - return l, err + return tz.LoadLocation(name) } diff --git a/vendor b/vendor index 5236a2aa2716..e17e1129c29f 160000 --- a/vendor +++ b/vendor @@ -1 +1 @@ -Subproject commit 5236a2aa2716e000f1a6cfb2a7e355e58562c13f +Subproject commit e17e1129c29fbf7ca70fd59fbd9ffdc2ac7e74aa