Skip to content

Commit

Permalink
timeutil: embed Go time zone data into CockroachDB
Browse files Browse the repository at this point in the history
* Use go's time/tzdata to embed timezone data.
* Modify timeutil package to always use this package.
* 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, which is the fallback source of time if tzdata is
not located by the default Go standard library.
  • Loading branch information
otan committed Nov 14, 2020
1 parent 6b254f2 commit 90efaaf
Show file tree
Hide file tree
Showing 11 changed files with 21 additions and 67 deletions.
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 @@ -848,7 +849,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 @@ -709,13 +709,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/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 @@ -819,7 +820,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 @@ -850,7 +851,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: 5 additions & 15 deletions pkg/util/timeutil/zoneinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,16 @@ package timeutil
import (
"strings"
"time"

"github.com/cockroachdb/errors"
// embed tzdata in case system tzdata is not available.
_ "time/tzdata"
)

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".
// We do not use Go's time.LoadLocation() directly because it maps
// "Local" to the local time zone, whereas we want UTC.
func LoadLocation(name string) (*time.Location, error) {
switch strings.ToLower(name) {
case "local", "default":
Expand All @@ -39,9 +33,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 time.LoadLocation(name)
}

0 comments on commit 90efaaf

Please sign in to comment.