Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

timeutil: embed time zone data into CockroachDB #55377

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -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=
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/serverccl/role_authentication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 0 additions & 4 deletions pkg/cli/demo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
36 changes: 0 additions & 36 deletions pkg/cli/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/authentication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/sem/tree/datum_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)

Expand Down
5 changes: 3 additions & 2 deletions pkg/testutils/lint/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/timetz/timetz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
5 changes: 3 additions & 2 deletions pkg/util/timeutil/pgdate/parsing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/util/timeutil/pgdate/zone_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/timeutil/time_zone_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
20 changes: 7 additions & 13 deletions pkg/util/timeutil/zoneinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand All @@ -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)
}