From 3400584b7ee5e18b2e1a777d089261fae6f69d9f Mon Sep 17 00:00:00 2001 From: Shuaipeng Yu Date: Tue, 19 Nov 2019 12:07:16 +0000 Subject: [PATCH 1/5] timeutil: do not use system file to verify timezone Signed-off-by: Shuaipeng Yu --- util/timeutil/time.go | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index 846f272ce9cc4..6fada42fc71f5 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -15,7 +15,6 @@ package timeutil import ( "fmt" - "os" "path/filepath" "strings" "sync" @@ -41,13 +40,6 @@ var locCa *locCache // systemTZ is current TiDB's system timezone name. var systemTZ string -var zoneSources = []string{ - "/usr/share/zoneinfo/", - "/usr/share/lib/zoneinfo/", - "/usr/lib/locale/TZ/", - // this is for macOS - "/var/db/timezone/zoneinfo/", -} // locCache is a simple map with lock. It stores all used timezone during the lifetime of tidb instance. // Talked with Golang team about whether they can have some forms of cache policy available for programmer, @@ -60,7 +52,7 @@ type locCache struct { } // InferSystemTZ reads system timezone from `TZ`, the path of the soft link of `/etc/localtime`. If both of them are failed, system timezone will be set to `UTC`. -// It is exported because we need to use it during bootstap stage. And it should be only used at that stage. +// It is exported because we need to use it during bootstrap stage. And it should be only used at that stage. func InferSystemTZ() string { // consult $TZ to find the time zone to use. // no $TZ means use the system default /etc/localtime. @@ -79,10 +71,9 @@ func InferSystemTZ() string { } logutil.BgLogger().Error("locate timezone files failed", zap.Error(err1)) case tz != "" && tz != "UTC": - for _, source := range zoneSources { - if _, err := os.Stat(source + tz); err == nil { - return tz - } + _, err := time.LoadLocation(tz) + if err == nil { + return tz } } return "UTC" From c9e7a7d4cd1548b18719ed325cbe5522f7ed74a8 Mon Sep 17 00:00:00 2001 From: Shuaipeng Yu Date: Fri, 22 Nov 2019 17:09:50 +0800 Subject: [PATCH 2/5] address comments Signed-off-by: Shuaipeng Yu --- util/timeutil/time.go | 54 ++++---------------------------------- util/timeutil/time_test.go | 12 --------- 2 files changed, 5 insertions(+), 61 deletions(-) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index 6fada42fc71f5..2dacd502b740b 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -15,14 +15,8 @@ package timeutil import ( "fmt" - "path/filepath" - "strings" "sync" - "syscall" "time" - - "github.com/pingcap/tidb/util/logutil" - "go.uber.org/zap" ) // init initializes `locCache`. @@ -54,51 +48,13 @@ type locCache struct { // InferSystemTZ reads system timezone from `TZ`, the path of the soft link of `/etc/localtime`. If both of them are failed, system timezone will be set to `UTC`. // It is exported because we need to use it during bootstrap stage. And it should be only used at that stage. func InferSystemTZ() string { - // consult $TZ to find the time zone to use. - // no $TZ means use the system default /etc/localtime. - // $TZ="" means use UTC. - // $TZ="foo" means use /usr/share/zoneinfo/foo. - tz, ok := syscall.Getenv("TZ") - switch { - case !ok: - path, err1 := filepath.EvalSymlinks("/etc/localtime") - if err1 == nil { - name, err2 := inferTZNameFromFileName(path) - if err2 == nil { - return name - } - logutil.BgLogger().Error("infer timezone failed", zap.Error(err2)) - } - logutil.BgLogger().Error("locate timezone files failed", zap.Error(err1)) - case tz != "" && tz != "UTC": - _, err := time.LoadLocation(tz) - if err == nil { - return tz - } + tz := time.Local.String() + if tz != "Local" { + return tz } return "UTC" } -// inferTZNameFromFileName gets IANA timezone name from zoneinfo path. -// TODO: It will be refined later. This is just a quick fix. -func inferTZNameFromFileName(path string) (string, error) { - // phase1 only support read /etc/localtime which is a softlink to zoneinfo file - substr := "zoneinfo" - // macOs MoJave changes the sofe link of /etc/localtime from - // "/var/db/timezone/tz/2018e.1.0/zoneinfo/Asia/Shanghai" - // to "/usr/share/zoneinfo.default/Asia/Shanghai" - substrMojave := "zoneinfo.default" - - if idx := strings.Index(path, substrMojave); idx != -1 { - return string(path[idx+len(substrMojave)+1:]), nil - } - - if idx := strings.Index(path, substr); idx != -1 { - return string(path[idx+len(substr)+1:]), nil - } - return "", fmt.Errorf("path %s is not supported", path) -} - // SystemLocation returns time.SystemLocation's IANA timezone location. It is TiDB's global timezone location. func SystemLocation() *time.Location { loc, err := LoadLocation(systemTZ) @@ -152,8 +108,8 @@ func LoadLocation(name string) (*time.Location, error) { func Zone(loc *time.Location) (string, int64) { _, offset := time.Now().In(loc).Zone() name := loc.String() - // when we found name is "System", we have no chice but push down - // "System" to tikv side. + // when we found name is "System", we have no choice but push down + // "System" to TiKV side. if name == "Local" { name = "System" } diff --git a/util/timeutil/time_test.go b/util/timeutil/time_test.go index c354b889e14d3..1a9f26671e5c4 100644 --- a/util/timeutil/time_test.go +++ b/util/timeutil/time_test.go @@ -32,18 +32,6 @@ func TestT(t *testing.T) { type testTimeSuite struct{} -func (s *testTimeSuite) TestgetTZNameFromFileName(c *C) { - tz, err := inferTZNameFromFileName("/user/share/zoneinfo/Asia/Shanghai") - - c.Assert(err, IsNil) - c.Assert(tz, Equals, "Asia/Shanghai") - - tz, err = inferTZNameFromFileName("/usr/share/zoneinfo.default/Asia/Shanghai") - - c.Assert(err, IsNil) - c.Assert(tz, Equals, "Asia/Shanghai") -} - func (s *testTimeSuite) TestLocal(c *C) { os.Setenv("TZ", "Asia/Shanghai") systemTZ = InferSystemTZ() From ad6be453212eed7795ee3e6de84d534861cfdfb6 Mon Sep 17 00:00:00 2001 From: Shuaipeng Yu Date: Fri, 22 Nov 2019 18:29:33 +0800 Subject: [PATCH 3/5] fix test Signed-off-by: Shuaipeng Yu --- util/timeutil/time.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index 3f8e6b144c480..96711d72ee51c 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -15,6 +15,7 @@ package timeutil import ( "fmt" + "os" "sync" "time" ) @@ -45,10 +46,15 @@ type locCache struct { locMap map[string]*time.Location } -// InferSystemTZ reads system timezone from `TZ`, the path of the soft link of `/etc/localtime`. If both of them are failed, system timezone will be set to `UTC`. +// InferSystemTZ reads system timezone from `TZ` and time.Local. If both of them are failed, system timezone will be set to `UTC`. // It is exported because we need to use it during bootstrap stage. And it should be only used at that stage. func InferSystemTZ() string { - tz := time.Local.String() + tz := os.Getenv("TZ") + _, err := time.LoadLocation(tz) + if err == nil { + return tz + } + tz = time.Local.String() if tz != "Local" { return tz } From 2dbca08281a9e3d30f51ddd02867566c0ea5b3de Mon Sep 17 00:00:00 2001 From: Shuaipeng Yu Date: Mon, 25 Nov 2019 14:06:41 +0800 Subject: [PATCH 4/5] revert to the first version Signed-off-by: Shuaipeng Yu --- util/timeutil/time.go | 56 ++++++++++++++++++++++++++++++++------ util/timeutil/time_test.go | 12 ++++++++ 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index 96711d72ee51c..be65386f3a384 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -15,9 +15,14 @@ package timeutil import ( "fmt" - "os" + "path/filepath" + "strings" "sync" + "syscall" "time" + + "github.com/pingcap/tidb/util/logutil" + "go.uber.org/zap" ) // init initializes `locCache`. @@ -49,18 +54,51 @@ type locCache struct { // InferSystemTZ reads system timezone from `TZ` and time.Local. If both of them are failed, system timezone will be set to `UTC`. // It is exported because we need to use it during bootstrap stage. And it should be only used at that stage. func InferSystemTZ() string { - tz := os.Getenv("TZ") - _, err := time.LoadLocation(tz) - if err == nil { - return tz - } - tz = time.Local.String() - if tz != "Local" { - return tz + // consult $TZ to find the time zone to use. + // no $TZ means use the system default /etc/localtime. + // $TZ="" means use UTC. + // $TZ="foo" means use /usr/share/zoneinfo/foo. + tz, ok := syscall.Getenv("TZ") + switch { + case !ok: + path, err1 := filepath.EvalSymlinks("/etc/localtime") + if err1 == nil { + name, err2 := inferTZNameFromFileName(path) + if err2 == nil { + return name + } + logutil.BgLogger().Error("infer timezone failed", zap.Error(err2)) + } + logutil.BgLogger().Error("locate timezone files failed", zap.Error(err1)) + case tz != "" && tz != "UTC": + _, err := time.LoadLocation(tz) + if err == nil { + return tz + } } return "UTC" } +// inferTZNameFromFileName gets IANA timezone name from zoneinfo path. +// TODO: It will be refined later. This is just a quick fix. +func inferTZNameFromFileName(path string) (string, error) { + // phase1 only support read /etc/localtime which is a softlink to zoneinfo file + substr := "zoneinfo" + // macOs MoJave changes the sofe link of /etc/localtime from + // "/var/db/timezone/tz/2018e.1.0/zoneinfo/Asia/Shanghai" + // to "/usr/share/zoneinfo.default/Asia/Shanghai" + substrMojave := "zoneinfo.default" + + if idx := strings.Index(path, substrMojave); idx != -1 { + return string(path[idx+len(substrMojave)+1:]), nil + } + + if idx := strings.Index(path, substr); idx != -1 { + return string(path[idx+len(substr)+1:]), nil + } + return "", fmt.Errorf("path %s is not supported", path) +} + // SystemLocation returns time.SystemLocation's IANA timezone location. It is TiDB's global timezone location. func SystemLocation() *time.Location { loc, err := LoadLocation(systemTZ) diff --git a/util/timeutil/time_test.go b/util/timeutil/time_test.go index 1a9f26671e5c4..c354b889e14d3 100644 --- a/util/timeutil/time_test.go +++ b/util/timeutil/time_test.go @@ -32,6 +32,18 @@ func TestT(t *testing.T) { type testTimeSuite struct{} +func (s *testTimeSuite) TestgetTZNameFromFileName(c *C) { + tz, err := inferTZNameFromFileName("/user/share/zoneinfo/Asia/Shanghai") + + c.Assert(err, IsNil) + c.Assert(tz, Equals, "Asia/Shanghai") + + tz, err = inferTZNameFromFileName("/usr/share/zoneinfo.default/Asia/Shanghai") + + c.Assert(err, IsNil) + c.Assert(tz, Equals, "Asia/Shanghai") +} + func (s *testTimeSuite) TestLocal(c *C) { os.Setenv("TZ", "Asia/Shanghai") systemTZ = InferSystemTZ() From 70002353433d1365c3ad21ff9d039b789e5cf3bc Mon Sep 17 00:00:00 2001 From: Shuaipeng Yu Date: Mon, 25 Nov 2019 14:07:40 +0800 Subject: [PATCH 5/5] fix comments Signed-off-by: Shuaipeng Yu --- util/timeutil/time.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index be65386f3a384..b11b1dd5d127f 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -51,7 +51,7 @@ type locCache struct { locMap map[string]*time.Location } -// InferSystemTZ reads system timezone from `TZ` and time.Local. If both of them are failed, system timezone will be set to `UTC`. +// InferSystemTZ reads system timezone from `TZ`, the path of the soft link of `/etc/localtime`. If both of them are failed, system timezone will be set to `UTC`. // It is exported because we need to use it during bootstrap stage. And it should be only used at that stage. func InferSystemTZ() string { // consult $TZ to find the time zone to use.