From ec6a822cfa782c744415d5b47341ae969b0852c4 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Fri, 7 Sep 2018 12:42:48 +0800 Subject: [PATCH 01/57] get tz name from zoneinfo --- executor/distsql.go | 16 +++++++++++++++- executor/distsql_test.go | 6 ++++++ store/mockstore/mocktikv/cop_handler_dag.go | 15 +++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/executor/distsql.go b/executor/distsql.go index b16c04aad1901..e1f8c11a937ef 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -117,6 +117,16 @@ func closeAll(objs ...Closeable) error { return errors.Trace(err) } +func getTZNameFromFileName(path string) (string, error) { + // phase1 only support read /etc/localtime which is a softlink to zoneinfo file + substr := "share/zoneinfo" + if strings.Contains(path, substr) { + idx := strings.Index(path, substr) + return string(path[idx+len(substr)+1:]), nil + } + return "", errors.New("only support softlink has share/zoneinfo as a suffix" + path) +} + // zone returns the current timezone name and timezone offset in seconds. // In compatible with MySQL, we change `Local` to `System`. // TODO: Golang team plan to return system timezone name intead of @@ -127,7 +137,11 @@ func zone(sctx sessionctx.Context) (string, int64) { var name string name = loc.String() if name == "Local" { - name = "System" + path, err := filepath.EvalSymlinks("/etc/localtime") + if err != nil { + return "Sysytem", int64(offset) + } + return getTZNameFromFileName(path), int64(offset) } return name, int64(offset) diff --git a/executor/distsql_test.go b/executor/distsql_test.go index 070c10233e1be..15edf80a586a4 100644 --- a/executor/distsql_test.go +++ b/executor/distsql_test.go @@ -207,3 +207,9 @@ func (s *testSuite) TestUniqueKeyNullValueSelect(c *C) { res = tk.MustQuery("select * from t where id is null;") res.Check(testkit.Rows(" a", " b", " c")) } + +func (s *testSuite) TestgetTZNameFromFileName(c *C) { + tz, err := getTZNameFromFileName("/user/share/zoneinfo/Asia/Shanghai") + c.Assert(err, IsNil) + c.Assert(tz, Equals, "Asia/Shanghai") +} diff --git a/store/mockstore/mocktikv/cop_handler_dag.go b/store/mockstore/mocktikv/cop_handler_dag.go index 456e1feb057b2..450694703573d 100644 --- a/store/mockstore/mocktikv/cop_handler_dag.go +++ b/store/mockstore/mocktikv/cop_handler_dag.go @@ -17,6 +17,7 @@ import ( "bytes" "fmt" "io" + "strings" "sync" "time" @@ -41,6 +42,7 @@ import ( "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/metadata" + "path/filepath" ) var dummySlice = make([]byte, 0) @@ -164,13 +166,26 @@ func (h *rpcHandler) buildDAGExecutor(req *coprocessor.Request) (*dagContext, ex return ctx, e, dagReq, err } +func getTZNameFromFileName(path string) (string, error) { + // phase1 only support read /etc/localtime which is a softlink to zoneinfo file + substr := "share/zoneinfo" + if strings.Contains(path, substr) { + idx := strings.Index(path, substr) + return string(path[idx+len(substr)+1:]), nil + } + return "", errors.New("only support softlink has share/zoneinfo as a suffix" + path) +} + // constructTimeZone constructs timezone by name first. When the timezone name // is set, the daylight saving problem must be considered. Otherwise the // timezone offset in seconds east of UTC is used to constructed the timezone. func constructTimeZone(name string, offset int) (*time.Location, error) { if name != "" { + // no need to care about case since name is retreved + // from go std library call return LocCache.getLoc(name) } + return time.FixedZone("", offset), nil } From 40f1caf7904f2dfbd9cda8c12f6847cfbc439f67 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Fri, 7 Sep 2018 14:32:02 +0800 Subject: [PATCH 02/57] remove imports --- executor/distsql.go | 10 +++++++++- store/mockstore/mocktikv/cop_handler_dag.go | 1 - 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/executor/distsql.go b/executor/distsql.go index e1f8c11a937ef..9132a0f50ee46 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -15,8 +15,10 @@ package executor import ( "math" + "path/filepath" "runtime" "sort" + "strings" "sync" "sync/atomic" "time" @@ -139,9 +141,15 @@ func zone(sctx sessionctx.Context) (string, int64) { if name == "Local" { path, err := filepath.EvalSymlinks("/etc/localtime") if err != nil { + log.Errorln(err) return "Sysytem", int64(offset) } - return getTZNameFromFileName(path), int64(offset) + name, err = getTZNameFromFileName(path) + if err != nil { + log.Errorln(err) + return "System", int64(offset) + } + return name, int64(offset) } return name, int64(offset) diff --git a/store/mockstore/mocktikv/cop_handler_dag.go b/store/mockstore/mocktikv/cop_handler_dag.go index 450694703573d..a3f919a070630 100644 --- a/store/mockstore/mocktikv/cop_handler_dag.go +++ b/store/mockstore/mocktikv/cop_handler_dag.go @@ -42,7 +42,6 @@ import ( "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/metadata" - "path/filepath" ) var dummySlice = make([]byte, 0) From ad574a4c679e1ffe2cd27e460850c9391afc3466 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Fri, 7 Sep 2018 14:39:13 +0800 Subject: [PATCH 03/57] add comment --- executor/distsql.go | 6 ++++-- executor/distsql_test.go | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/executor/distsql.go b/executor/distsql.go index 9132a0f50ee46..81345838f1d99 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -119,7 +119,9 @@ func closeAll(objs ...Closeable) error { return errors.Trace(err) } -func getTZNameFromFileName(path string) (string, error) { +// GetTZNameFromFileName gets IANA timezone name from zoninfo path. +// TODO It will be refined later. This is just a quick fix. +func GetTZNameFromFileName(path string) (string, error) { // phase1 only support read /etc/localtime which is a softlink to zoneinfo file substr := "share/zoneinfo" if strings.Contains(path, substr) { @@ -144,7 +146,7 @@ func zone(sctx sessionctx.Context) (string, int64) { log.Errorln(err) return "Sysytem", int64(offset) } - name, err = getTZNameFromFileName(path) + name, err = GetTZNameFromFileName(path) if err != nil { log.Errorln(err) return "System", int64(offset) diff --git a/executor/distsql_test.go b/executor/distsql_test.go index 15edf80a586a4..d303c212e026b 100644 --- a/executor/distsql_test.go +++ b/executor/distsql_test.go @@ -209,7 +209,7 @@ func (s *testSuite) TestUniqueKeyNullValueSelect(c *C) { } func (s *testSuite) TestgetTZNameFromFileName(c *C) { - tz, err := getTZNameFromFileName("/user/share/zoneinfo/Asia/Shanghai") + tz, err := executor.GetTZNameFromFileName("/user/share/zoneinfo/Asia/Shanghai") c.Assert(err, IsNil) c.Assert(tz, Equals, "Asia/Shanghai") } From e76c22456ffe091fc5b867a2b5318decb01a3cdd Mon Sep 17 00:00:00 2001 From: zhexuany Date: Fri, 7 Sep 2018 14:40:36 +0800 Subject: [PATCH 04/57] correct a spelling --- executor/distsql.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/distsql.go b/executor/distsql.go index 81345838f1d99..c37292237af50 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -144,7 +144,7 @@ func zone(sctx sessionctx.Context) (string, int64) { path, err := filepath.EvalSymlinks("/etc/localtime") if err != nil { log.Errorln(err) - return "Sysytem", int64(offset) + return "System", int64(offset) } name, err = GetTZNameFromFileName(path) if err != nil { From a74739c857755cc7be2863f43045550c43f975d6 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Fri, 7 Sep 2018 16:25:42 +0800 Subject: [PATCH 05/57] called once for evalSys --- executor/distsql.go | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/executor/distsql.go b/executor/distsql.go index c37292237af50..372b7c8eee3f7 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -131,6 +131,9 @@ func GetTZNameFromFileName(path string) (string, error) { return "", errors.New("only support softlink has share/zoneinfo as a suffix" + path) } +var localStr string +var localOnce sync.Once + // zone returns the current timezone name and timezone offset in seconds. // In compatible with MySQL, we change `Local` to `System`. // TODO: Golang team plan to return system timezone name intead of @@ -141,17 +144,20 @@ func zone(sctx sessionctx.Context) (string, int64) { var name string name = loc.String() if name == "Local" { - path, err := filepath.EvalSymlinks("/etc/localtime") - if err != nil { - log.Errorln(err) - return "System", int64(offset) - } - name, err = GetTZNameFromFileName(path) - if err != nil { - log.Errorln(err) - return "System", int64(offset) - } - return name, int64(offset) + localOnce.Do(func() { + path, err := filepath.EvalSymlinks("/etc/localtime") + if err != nil { + log.Errorln(err) + localStr = "System" + } + localStr, err = GetTZNameFromFileName(path) + if err != nil { + log.Errorln(err) + localStr = "System" + } + }) + + return localStr, int64(offset) } return name, int64(offset) From f0502c800335a123db9fb3cf89cf8abab5c51a9c Mon Sep 17 00:00:00 2001 From: zhexuany Date: Fri, 7 Sep 2018 19:18:18 +0800 Subject: [PATCH 06/57] address comments --- executor/distsql.go | 4 ++-- store/mockstore/mocktikv/cop_handler_dag.go | 10 ---------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/executor/distsql.go b/executor/distsql.go index 372b7c8eee3f7..274d8330f7f6f 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -119,7 +119,7 @@ func closeAll(objs ...Closeable) error { return errors.Trace(err) } -// GetTZNameFromFileName gets IANA timezone name from zoninfo path. +// GetTZNameFromFileName gets IANA timezone name from zoneinfo path. // TODO It will be refined later. This is just a quick fix. func GetTZNameFromFileName(path string) (string, error) { // phase1 only support read /etc/localtime which is a softlink to zoneinfo file @@ -128,7 +128,7 @@ func GetTZNameFromFileName(path string) (string, error) { idx := strings.Index(path, substr) return string(path[idx+len(substr)+1:]), nil } - return "", errors.New("only support softlink has share/zoneinfo as a suffix" + path) + return "", errors.New("only support softlink has share/zoneinfo in the middle" + path) } var localStr string diff --git a/store/mockstore/mocktikv/cop_handler_dag.go b/store/mockstore/mocktikv/cop_handler_dag.go index a3f919a070630..ca2d3dc2e567f 100644 --- a/store/mockstore/mocktikv/cop_handler_dag.go +++ b/store/mockstore/mocktikv/cop_handler_dag.go @@ -165,16 +165,6 @@ func (h *rpcHandler) buildDAGExecutor(req *coprocessor.Request) (*dagContext, ex return ctx, e, dagReq, err } -func getTZNameFromFileName(path string) (string, error) { - // phase1 only support read /etc/localtime which is a softlink to zoneinfo file - substr := "share/zoneinfo" - if strings.Contains(path, substr) { - idx := strings.Index(path, substr) - return string(path[idx+len(substr)+1:]), nil - } - return "", errors.New("only support softlink has share/zoneinfo as a suffix" + path) -} - // constructTimeZone constructs timezone by name first. When the timezone name // is set, the daylight saving problem must be considered. Otherwise the // timezone offset in seconds east of UTC is used to constructed the timezone. From ed136b980ceba64d9593ae55bf4ec0415f6802b0 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Fri, 7 Sep 2018 22:15:55 +0800 Subject: [PATCH 07/57] remove unecessary import --- store/mockstore/mocktikv/cop_handler_dag.go | 1 - 1 file changed, 1 deletion(-) diff --git a/store/mockstore/mocktikv/cop_handler_dag.go b/store/mockstore/mocktikv/cop_handler_dag.go index ca2d3dc2e567f..dbbbef6d3f70e 100644 --- a/store/mockstore/mocktikv/cop_handler_dag.go +++ b/store/mockstore/mocktikv/cop_handler_dag.go @@ -17,7 +17,6 @@ import ( "bytes" "fmt" "io" - "strings" "sync" "time" From 8ba623f5ea47db4d9ebad51534ce62b58cf29bfb Mon Sep 17 00:00:00 2001 From: zhexuany Date: Sat, 8 Sep 2018 14:13:05 +0800 Subject: [PATCH 08/57] rewrite code --- executor/distsql.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/executor/distsql.go b/executor/distsql.go index 274d8330f7f6f..9f7e3d6e7963e 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -146,15 +146,13 @@ func zone(sctx sessionctx.Context) (string, int64) { if name == "Local" { localOnce.Do(func() { path, err := filepath.EvalSymlinks("/etc/localtime") - if err != nil { - log.Errorln(err) - localStr = "System" - } - localStr, err = GetTZNameFromFileName(path) - if err != nil { - log.Errorln(err) - localStr = "System" + if err == nil { + localStr, err = GetTZNameFromFileName(path) + if err == nil { + return + } } + localStr = "System" }) return localStr, int64(offset) From e962f8207265e1d92b4e9e2bd1e4a4a906c8fd66 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Sat, 8 Sep 2018 14:22:04 +0800 Subject: [PATCH 09/57] print localStr out --- executor/distsql.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/executor/distsql.go b/executor/distsql.go index 9f7e3d6e7963e..b9b9dfe95f4e6 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -24,6 +24,7 @@ import ( "time" "unsafe" + "fmt" "github.com/juju/errors" "github.com/pingcap/tidb/distsql" "github.com/pingcap/tidb/expression" @@ -155,6 +156,7 @@ func zone(sctx sessionctx.Context) (string, int64) { localStr = "System" }) + fmt.Println(localStr) return localStr, int64(offset) } From 26f65f7dcda0e5c770db772dd6755896dc5ce209 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Sat, 8 Sep 2018 14:57:11 +0800 Subject: [PATCH 10/57] set time_zone in test --- executor/distsql.go | 2 -- executor/executor_test.go | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/executor/distsql.go b/executor/distsql.go index b9b9dfe95f4e6..9f7e3d6e7963e 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -24,7 +24,6 @@ import ( "time" "unsafe" - "fmt" "github.com/juju/errors" "github.com/pingcap/tidb/distsql" "github.com/pingcap/tidb/expression" @@ -156,7 +155,6 @@ func zone(sctx sessionctx.Context) (string, int64) { localStr = "System" }) - fmt.Println(localStr) return localStr, int64(offset) } diff --git a/executor/executor_test.go b/executor/executor_test.go index 6ab4b4bbcb755..40b591ebedba8 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -3121,6 +3121,7 @@ func (s *testSuite) TestCurrentTimestampValueSelection(c *C) { tk.MustExec("use test") tk.MustExec("drop table if exists t,t1") + tk.MustExec("set time_zone='Asia/Shanghai'") tk.MustExec("create table t (id int, t0 timestamp null default current_timestamp, t1 timestamp(1) null default current_timestamp(1), t2 timestamp(2) null default current_timestamp(2) on update current_timestamp(2))") tk.MustExec("insert into t (id) values (1)") rs := tk.MustQuery("select t0, t1, t2 from t where id = 1") From d70818665af70fee0f3804dc97b9983e4bdfe20e Mon Sep 17 00:00:00 2001 From: zhexuany Date: Sat, 8 Sep 2018 15:38:45 +0800 Subject: [PATCH 11/57] align tz behavior with golang --- executor/distsql.go | 46 ++++++++++++++++++++++++++++++--------- executor/executor_test.go | 1 - 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/executor/distsql.go b/executor/distsql.go index 9f7e3d6e7963e..1fa7a25ed5a2b 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -21,6 +21,7 @@ import ( "strings" "sync" "sync/atomic" + "syscall" "time" "unsafe" @@ -43,6 +44,7 @@ import ( "github.com/pingcap/tipb/go-tipb" log "github.com/sirupsen/logrus" "golang.org/x/net/context" + "os" ) var ( @@ -133,6 +135,39 @@ func GetTZNameFromFileName(path string) (string, error) { var localStr string var localOnce sync.Once +var zoneSources = []string{ + "/usr/share/zoneinfo/", + "/usr/share/lib/zoneinfo/", + "/usr/lib/locale/TZ/", +} + +func initLocalStr() { + // 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") + if ok && tz != "" { + for _, source := range zoneSources { + if _, err := os.Stat(source + tz); os.IsExist(err) { + localStr = tz + break + } + } + } else if tz == "" { + localStr = "UTC" + } else { + path, err := filepath.EvalSymlinks("/etc/localtime") + if err == nil { + localStr, err = GetTZNameFromFileName(path) + if err == nil { + return + } + } + localStr = "System" + } +} // zone returns the current timezone name and timezone offset in seconds. // In compatible with MySQL, we change `Local` to `System`. @@ -144,16 +179,7 @@ func zone(sctx sessionctx.Context) (string, int64) { var name string name = loc.String() if name == "Local" { - localOnce.Do(func() { - path, err := filepath.EvalSymlinks("/etc/localtime") - if err == nil { - localStr, err = GetTZNameFromFileName(path) - if err == nil { - return - } - } - localStr = "System" - }) + localOnce.Do(initLocalStr) return localStr, int64(offset) } diff --git a/executor/executor_test.go b/executor/executor_test.go index 40b591ebedba8..6ab4b4bbcb755 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -3121,7 +3121,6 @@ func (s *testSuite) TestCurrentTimestampValueSelection(c *C) { tk.MustExec("use test") tk.MustExec("drop table if exists t,t1") - tk.MustExec("set time_zone='Asia/Shanghai'") tk.MustExec("create table t (id int, t0 timestamp null default current_timestamp, t1 timestamp(1) null default current_timestamp(1), t2 timestamp(2) null default current_timestamp(2) on update current_timestamp(2))") tk.MustExec("insert into t (id) values (1)") rs := tk.MustQuery("select t0, t1, t2 from t where id = 1") From ceedc34e86eefa9a1285bf604a25bba49ae3f3e1 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Sat, 8 Sep 2018 16:36:44 +0800 Subject: [PATCH 12/57] when tz is empty use localtime --- executor/distsql.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/executor/distsql.go b/executor/distsql.go index 1fa7a25ed5a2b..be2e500a673b8 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -155,8 +155,6 @@ func initLocalStr() { break } } - } else if tz == "" { - localStr = "UTC" } else { path, err := filepath.EvalSymlinks("/etc/localtime") if err == nil { @@ -167,6 +165,7 @@ func initLocalStr() { } localStr = "System" } + println(localStr) } // zone returns the current timezone name and timezone offset in seconds. @@ -180,7 +179,6 @@ func zone(sctx sessionctx.Context) (string, int64) { name = loc.String() if name == "Local" { localOnce.Do(initLocalStr) - return localStr, int64(offset) } From 2adc1e8de0cb5f83ce56d9629ff4ce4567317a75 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Sat, 8 Sep 2018 22:24:59 +0800 Subject: [PATCH 13/57] adding timeutil package --- executor/distsql.go | 56 +------- executor/distsql_test.go | 6 - sessionctx/variable/session.go | 3 +- store/mockstore/mocktikv/cop_handler_dag.go | 49 +------ util/logutil/trace.go | 27 ++++ util/timeutil/time.go | 134 ++++++++++++++++++++ util/timeutil/time_test.go | 13 ++ 7 files changed, 181 insertions(+), 107 deletions(-) create mode 100644 util/logutil/trace.go create mode 100644 util/timeutil/time.go create mode 100644 util/timeutil/time_test.go diff --git a/executor/distsql.go b/executor/distsql.go index be2e500a673b8..a13db418d87eb 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -15,13 +15,10 @@ package executor import ( "math" - "path/filepath" "runtime" "sort" - "strings" "sync" "sync/atomic" - "syscall" "time" "unsafe" @@ -44,7 +41,6 @@ import ( "github.com/pingcap/tipb/go-tipb" log "github.com/sirupsen/logrus" "golang.org/x/net/context" - "os" ) var ( @@ -121,53 +117,6 @@ func closeAll(objs ...Closeable) error { return errors.Trace(err) } -// GetTZNameFromFileName gets IANA timezone name from zoneinfo path. -// TODO It will be refined later. This is just a quick fix. -func GetTZNameFromFileName(path string) (string, error) { - // phase1 only support read /etc/localtime which is a softlink to zoneinfo file - substr := "share/zoneinfo" - if strings.Contains(path, substr) { - idx := strings.Index(path, substr) - return string(path[idx+len(substr)+1:]), nil - } - return "", errors.New("only support softlink has share/zoneinfo in the middle" + path) -} - -var localStr string -var localOnce sync.Once -var zoneSources = []string{ - "/usr/share/zoneinfo/", - "/usr/share/lib/zoneinfo/", - "/usr/lib/locale/TZ/", -} - -func initLocalStr() { - // 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") - if ok && tz != "" { - for _, source := range zoneSources { - if _, err := os.Stat(source + tz); os.IsExist(err) { - localStr = tz - break - } - } - } else { - path, err := filepath.EvalSymlinks("/etc/localtime") - if err == nil { - localStr, err = GetTZNameFromFileName(path) - if err == nil { - return - } - } - localStr = "System" - } - println(localStr) -} - // zone returns the current timezone name and timezone offset in seconds. // In compatible with MySQL, we change `Local` to `System`. // TODO: Golang team plan to return system timezone name intead of @@ -177,9 +126,10 @@ func zone(sctx sessionctx.Context) (string, int64) { _, offset := time.Now().In(loc).Zone() var name string name = loc.String() + // when we found name is "Local", we have no chice but push down + // "System" to tikv side. if name == "Local" { - localOnce.Do(initLocalStr) - return localStr, int64(offset) + name = "System" } return name, int64(offset) diff --git a/executor/distsql_test.go b/executor/distsql_test.go index d303c212e026b..070c10233e1be 100644 --- a/executor/distsql_test.go +++ b/executor/distsql_test.go @@ -207,9 +207,3 @@ func (s *testSuite) TestUniqueKeyNullValueSelect(c *C) { res = tk.MustQuery("select * from t where id is null;") res.Check(testkit.Rows(" a", " b", " c")) } - -func (s *testSuite) TestgetTZNameFromFileName(c *C) { - tz, err := executor.GetTZNameFromFileName("/user/share/zoneinfo/Asia/Shanghai") - c.Assert(err, IsNil) - c.Assert(tz, Equals, "Asia/Shanghai") -} diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index f8c6e1183c43b..0bc70d1e94930 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -30,6 +30,7 @@ import ( "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/auth" "github.com/pingcap/tidb/util/chunk" + "github.com/pingcap/tidb/util/timeutil" ) const ( @@ -426,7 +427,7 @@ func (s *SessionVars) GetNextPreparedStmtID() uint32 { func (s *SessionVars) Location() *time.Location { loc := s.TimeZone if loc == nil { - loc = time.Local + loc = timeutil.Local() } return loc } diff --git a/store/mockstore/mocktikv/cop_handler_dag.go b/store/mockstore/mocktikv/cop_handler_dag.go index dbbbef6d3f70e..727a1f753ce80 100644 --- a/store/mockstore/mocktikv/cop_handler_dag.go +++ b/store/mockstore/mocktikv/cop_handler_dag.go @@ -15,9 +15,7 @@ package mocktikv import ( "bytes" - "fmt" "io" - "sync" "time" "github.com/golang/protobuf/proto" @@ -37,6 +35,7 @@ import ( "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/codec" mockpkg "github.com/pingcap/tidb/util/mock" + "github.com/pingcap/tidb/util/timeutil" "github.com/pingcap/tipb/go-tipb" "golang.org/x/net/context" "google.golang.org/grpc" @@ -45,50 +44,6 @@ import ( var dummySlice = make([]byte, 0) -// 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, -// they suggests that only programmers knows which one is best for their use case. -// For detail, please refer to: https://github.com/golang/go/issues/26106 -type locCache struct { - sync.RWMutex - // locMap stores locations used in past and can be retrieved by a timezone's name. - locMap map[string]*time.Location -} - -// init initializes `locCache`. -func init() { - LocCache = &locCache{} - LocCache.locMap = make(map[string]*time.Location) -} - -// LocCache is a simple cache policy to improve the performance of 'time.LoadLocation'. -var LocCache *locCache - -// getLoc first trying to load location from a cache map. If nothing found in such map, then call -// `time.LocadLocation` to get a timezone location. After trying both way, an error wil be returned -// if valid Location is not found. -func (lm *locCache) getLoc(name string) (*time.Location, error) { - if name == "System" { - name = "Local" - } - lm.RLock() - if v, ok := lm.locMap[name]; ok { - lm.RUnlock() - return v, nil - } - - if loc, err := time.LoadLocation(name); err == nil { - lm.RUnlock() - lm.Lock() - lm.locMap[name] = loc - lm.Unlock() - return loc, nil - } - - lm.RUnlock() - return nil, fmt.Errorf("invalid name for timezone %s", name) -} - type dagContext struct { dagReq *tipb.DAGRequest keyRanges []*coprocessor.KeyRange @@ -171,7 +126,7 @@ func constructTimeZone(name string, offset int) (*time.Location, error) { if name != "" { // no need to care about case since name is retreved // from go std library call - return LocCache.getLoc(name) + return timeutil.LoadLocation(name) } return time.FixedZone("", offset), nil diff --git a/util/logutil/trace.go b/util/logutil/trace.go new file mode 100644 index 0000000000000..af703feb789bb --- /dev/null +++ b/util/logutil/trace.go @@ -0,0 +1,27 @@ +// Copyright 2018 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package logutil + +import ( + "github.com/opentracing/opentracing-go" + "golang.org/x/net/context" +) + +// HasSpan return true if context has an active span +func HasSpan(ctx context.Context) bool { + if sp := opentracing.SpanFromContext(ctx); sp != nil { + return true + } + return false +} diff --git a/util/timeutil/time.go b/util/timeutil/time.go new file mode 100644 index 0000000000000..ef29766c7d324 --- /dev/null +++ b/util/timeutil/time.go @@ -0,0 +1,134 @@ +// Copyright 2017 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package timeutil + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "sync" + "syscall" + "time" + + log "github.com/sirupsen/logrus" +) + +// init initializes `locCache`. +func init() { + LocCache = &locCache{} + LocCache.locMap = make(map[string]*time.Location) +} + +// LocCache is a simple cache policy to improve the performance of 'time.LoadLocation'. +var LocCache *locCache +var localStr string +var localOnce sync.Once +var zoneSources = []string{ + "/usr/share/zoneinfo/", + "/usr/share/lib/zoneinfo/", + "/usr/lib/locale/TZ/", +} + +// 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, +// they suggests that only programmers knows which one is best for their use case. +// For detail, please refer to: https://github.com/golang/go/issues/26106 +type locCache struct { + sync.RWMutex + // locMap stores locations used in past and can be retrieved by a timezone's name. + locMap map[string]*time.Location +} + +func initLocalStr() { + // 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") + if ok && tz != "" { + for _, source := range zoneSources { + if _, err := os.Stat(source + tz); os.IsExist(err) { + localStr = tz + break + } + } + // } else if tz == "" { + // localStr = "UTC" + } else { + path, err := filepath.EvalSymlinks("/etc/localtime") + if err == nil { + localStr, err = getTZNameFromFileName(path) + if err == nil { + return + } + log.Errorln(err) + } + log.Errorln(err) + } + localStr = "System" +} + +// getTZNameFromFileName gets IANA timezone name from zoneinfo path. +// TODO It will be refined later. This is just a quick fix. +func getTZNameFromFileName(path string) (string, error) { + // phase1 only support read /etc/localtime which is a softlink to zoneinfo file + substr := "zoneinfo" + if strings.Contains(path, substr) { + idx := strings.Index(path, substr) + return string(path[idx+len(substr)+1:]), nil + } + return "", errors.New(fmt.Sprintf("path %s is not supported", path)) +} + +// Local returns time.Local's IANA timezone name. +func Local() *time.Location { + localOnce.Do(initLocalStr) + loc, err := LoadLocation(localStr) + if err != nil { + return time.Local + } + return loc +} + +// getLoc first trying to load location from a cache map. If nothing found in such map, then call +// `time.LocadLocation` to get a timezone location. After trying both way, an error wil be returned +// if valid Location is not found. +func (lm *locCache) getLoc(name string) (*time.Location, error) { + if name == "System" { + name = "Local" + } + lm.RLock() + if v, ok := lm.locMap[name]; ok { + lm.RUnlock() + return v, nil + } + + if loc, err := time.LoadLocation(name); err == nil { + lm.RUnlock() + lm.Lock() + lm.locMap[name] = loc + lm.Unlock() + return loc, nil + } + + lm.RUnlock() + return nil, fmt.Errorf("invalid name for timezone %s", name) +} + +// LoadLocation loads time.Location by IANA timezone time. +func LoadLocation(name string) (*time.Location, error) { + return LocCache.getLoc(name) +} diff --git a/util/timeutil/time_test.go b/util/timeutil/time_test.go new file mode 100644 index 0000000000000..0098cd463da86 --- /dev/null +++ b/util/timeutil/time_test.go @@ -0,0 +1,13 @@ +package timeutil + +import ( + . "github.com/pingcap/check" +) + +type testSuite struct{} + +func (s *testSuite) TestgetTZNameFromFileName(c *C) { + tz, err := getTZNameFromFileName("/user/share/zoneinfo/Asia/Shanghai") + c.Assert(err, IsNil) + c.Assert(tz, Equals, "Asia/Shanghai") +} From acc026283f1b9d8bc2af8b58454c0c93af8a223f Mon Sep 17 00:00:00 2001 From: zhexuany Date: Sun, 9 Sep 2018 00:06:26 +0800 Subject: [PATCH 14/57] make the use of time.Now in expression correct --- expression/builtin_time_test.go | 9 +++++---- expression/helper.go | 2 +- expression/helper_test.go | 2 +- store/mockstore/mocktikv/cop_handler_dag.go | 2 -- util/timeutil/time.go | 2 +- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/expression/builtin_time_test.go b/expression/builtin_time_test.go index e516c6da08d25..cb5c76fd01fd3 100644 --- a/expression/builtin_time_test.go +++ b/expression/builtin_time_test.go @@ -32,6 +32,7 @@ import ( "github.com/pingcap/tidb/util/mock" "github.com/pingcap/tidb/util/testleak" "github.com/pingcap/tidb/util/testutil" + "github.com/pingcap/tidb/util/timeutil" ) func (s *testEvaluatorSuite) TestDate(c *C) { @@ -774,7 +775,7 @@ func (s *testEvaluatorSuite) TestNowAndUTCTimestamp(c *C) { fc functionClass now func() time.Time }{ - {funcs[ast.Now], func() time.Time { return time.Now() }}, + {funcs[ast.Now], func() time.Time { return time.Now().In(s.ctx.GetSessionVars().Location()) }}, {funcs[ast.UTCTimestamp], func() time.Time { return time.Now().UTC() }}, } { f, err := x.fc.getFunction(s.ctx, s.datumsToConstants(nil)) @@ -997,7 +998,7 @@ func (s *testEvaluatorSuite) TestSysDate(c *C) { fc := funcs[ast.Sysdate] ctx := mock.NewContext() - ctx.GetSessionVars().StmtCtx.TimeZone = time.Local + ctx.GetSessionVars().StmtCtx.TimeZone = timeutil.Local() timezones := []types.Datum{types.NewDatum(1234), types.NewDatum(0)} for _, timezone := range timezones { // sysdate() result is not affected by "timestamp" session variable. @@ -1011,7 +1012,7 @@ func (s *testEvaluatorSuite) TestSysDate(c *C) { c.Assert(n.String(), GreaterEqual, last.Format(types.TimeFormat)) } - last := time.Now() + last := time.Now().In(s.ctx.GetSessionVars().Location()) f, err := fc.getFunction(ctx, s.datumsToConstants(types.MakeDatums(6))) c.Assert(err, IsNil) v, err := evalBuiltinFunc(f, chunk.Row{}) @@ -1156,7 +1157,7 @@ func (s *testEvaluatorSuite) TestCurrentTime(c *C) { defer testleak.AfterTest(c)() tfStr := "15:04:05" - last := time.Now() + last := time.Now().In(s.ctx.GetSessionVars().Location()) fc := funcs[ast.CurrentTime] f, err := fc.getFunction(s.ctx, s.datumsToConstants(types.MakeDatums(nil))) c.Assert(err, IsNil) diff --git a/expression/helper.go b/expression/helper.go index 813f45e903c85..657df5fed1092 100644 --- a/expression/helper.go +++ b/expression/helper.go @@ -121,7 +121,7 @@ func GetTimeValue(ctx sessionctx.Context, v interface{}, tp byte, fsp int) (d ty } func getSystemTimestamp(ctx sessionctx.Context) (time.Time, error) { - now := time.Now() + now := time.Now().In(ctx.GetSessionVars().Location()) if ctx == nil { return now, nil diff --git a/expression/helper_test.go b/expression/helper_test.go index f63109a5a28e6..dd1f9fa7840bd 100644 --- a/expression/helper_test.go +++ b/expression/helper_test.go @@ -68,7 +68,7 @@ func (s *testExpressionSuite) TestGetTimeValue(c *C) { Ret interface{} }{ {"2012-12-12 00:00:00", "2012-12-12 00:00:00"}, - {ast.CurrentTimestamp, time.Unix(1234, 0).Format(types.TimeFormat)}, + {ast.CurrentTimestamp, time.Unix(1234, 0).In(ctx.GetSessionVars().Location()).Format(types.TimeFormat)}, {types.ZeroDatetimeStr, "0000-00-00 00:00:00"}, {ast.NewValueExpr("2012-12-12 00:00:00"), "2012-12-12 00:00:00"}, {ast.NewValueExpr(int64(0)), "0000-00-00 00:00:00"}, diff --git a/store/mockstore/mocktikv/cop_handler_dag.go b/store/mockstore/mocktikv/cop_handler_dag.go index 727a1f753ce80..c82712b89fa7a 100644 --- a/store/mockstore/mocktikv/cop_handler_dag.go +++ b/store/mockstore/mocktikv/cop_handler_dag.go @@ -124,8 +124,6 @@ func (h *rpcHandler) buildDAGExecutor(req *coprocessor.Request) (*dagContext, ex // timezone offset in seconds east of UTC is used to constructed the timezone. func constructTimeZone(name string, offset int) (*time.Location, error) { if name != "" { - // no need to care about case since name is retreved - // from go std library call return timeutil.LoadLocation(name) } diff --git a/util/timeutil/time.go b/util/timeutil/time.go index ef29766c7d324..0dc01bd5bcdfd 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -108,7 +108,7 @@ func Local() *time.Location { // if valid Location is not found. func (lm *locCache) getLoc(name string) (*time.Location, error) { if name == "System" { - name = "Local" + return time.Local, nil } lm.RLock() if v, ok := lm.locMap[name]; ok { From 709c32c506cddb1465f5a515d45835f0f0bc8708 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Sun, 9 Sep 2018 00:14:51 +0800 Subject: [PATCH 15/57] remoeve unecessay file --- util/logutil/trace.go | 27 --------------------------- 1 file changed, 27 deletions(-) delete mode 100644 util/logutil/trace.go diff --git a/util/logutil/trace.go b/util/logutil/trace.go deleted file mode 100644 index af703feb789bb..0000000000000 --- a/util/logutil/trace.go +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2018 PingCAP, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// See the License for the specific language governing permissions and -// limitations under the License. - -package logutil - -import ( - "github.com/opentracing/opentracing-go" - "golang.org/x/net/context" -) - -// HasSpan return true if context has an active span -func HasSpan(ctx context.Context) bool { - if sp := opentracing.SpanFromContext(ctx); sp != nil { - return true - } - return false -} From 350d27d75d0ac9446967ad99d767531569ab8e68 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Sun, 9 Sep 2018 11:18:05 +0800 Subject: [PATCH 16/57] adding test and debug print --- util/timeutil/time.go | 1 + util/timeutil/time_test.go | 20 ++++++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index 0dc01bd5bcdfd..a5c6ae00ec504 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -96,6 +96,7 @@ func getTZNameFromFileName(path string) (string, error) { // Local returns time.Local's IANA timezone name. func Local() *time.Location { localOnce.Do(initLocalStr) + fmt.Printf("local time zone name is %s\n", localStr) loc, err := LoadLocation(localStr) if err != nil { return time.Local diff --git a/util/timeutil/time_test.go b/util/timeutil/time_test.go index 0098cd463da86..c3b6e98501f6a 100644 --- a/util/timeutil/time_test.go +++ b/util/timeutil/time_test.go @@ -1,13 +1,29 @@ package timeutil import ( + "os" + "testing" + . "github.com/pingcap/check" ) -type testSuite struct{} +var _ = Suite(&testTimeSuite{}) + +func TestT(t *testing.T) { + TestingT(t) +} + +type testTimeSuite struct{} -func (s *testSuite) TestgetTZNameFromFileName(c *C) { +func (s *testTimeSuite) TestgetTZNameFromFileName(c *C) { tz, err := getTZNameFromFileName("/user/share/zoneinfo/Asia/Shanghai") c.Assert(err, IsNil) c.Assert(tz, Equals, "Asia/Shanghai") } + +func (s *testTimeSuite) TestLocal(c *C) { + os.Setenv("TZ", "UTC") + loc := Local() + c.Assert(loc.String(), Equals, "UTC") + os.Unsetenv("TZ") +} From c95e6c354350e5851f4b5d663af4c8242439852b Mon Sep 17 00:00:00 2001 From: zhexuany Date: Sun, 9 Sep 2018 11:33:02 +0800 Subject: [PATCH 17/57] print tz in initLocalStr --- 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 a5c6ae00ec504..3337aa2aba908 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -59,6 +59,7 @@ func initLocalStr() { // $TZ="foo" means use /usr/share/zoneinfo/foo. tz, ok := syscall.Getenv("TZ") if ok && tz != "" { + fmt.Printf("initLocalStr tz %s", tz) for _, source := range zoneSources { if _, err := os.Stat(source + tz); os.IsExist(err) { localStr = tz @@ -96,7 +97,6 @@ func getTZNameFromFileName(path string) (string, error) { // Local returns time.Local's IANA timezone name. func Local() *time.Location { localOnce.Do(initLocalStr) - fmt.Printf("local time zone name is %s\n", localStr) loc, err := LoadLocation(localStr) if err != nil { return time.Local From 21e9e2290f6343e407f8b566cfe74025f6c837ad Mon Sep 17 00:00:00 2001 From: zhexuany Date: Sun, 9 Sep 2018 12:18:55 +0800 Subject: [PATCH 18/57] move zone to timeutil --- executor/admin.go | 7 ++++--- executor/builder.go | 7 ++++--- executor/distsql.go | 19 ------------------- util/timeutil/time.go | 26 +++++++++++++++++++++++--- 4 files changed, 31 insertions(+), 28 deletions(-) diff --git a/executor/admin.go b/executor/admin.go index 2faf6278eeaee..b4a766477f29d 100644 --- a/executor/admin.go +++ b/executor/admin.go @@ -31,6 +31,7 @@ import ( "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/chunk" "github.com/pingcap/tidb/util/ranger" + "github.com/pingcap/tidb/util/timeutil" "github.com/pingcap/tipb/go-tipb" log "github.com/sirupsen/logrus" "golang.org/x/net/context" @@ -125,7 +126,7 @@ func (e *CheckIndexRangeExec) Open(ctx context.Context) error { func (e *CheckIndexRangeExec) buildDAGPB() (*tipb.DAGRequest, error) { dagReq := &tipb.DAGRequest{} dagReq.StartTs = e.ctx.Txn().StartTS() - dagReq.TimeZoneName, dagReq.TimeZoneOffset = zone(e.ctx) + dagReq.TimeZoneName, dagReq.TimeZoneOffset = timeutil.Zone(e.ctx.GetSessionVars().Location()) sc := e.ctx.GetSessionVars().StmtCtx dagReq.Flags = statementContextToFlags(sc) for i := range e.schema.Columns { @@ -223,7 +224,7 @@ func (e *RecoverIndexExec) constructLimitPB(count uint64) *tipb.Executor { func (e *RecoverIndexExec) buildDAGPB(txn kv.Transaction, limitCnt uint64) (*tipb.DAGRequest, error) { dagReq := &tipb.DAGRequest{} dagReq.StartTs = txn.StartTS() - dagReq.TimeZoneName, dagReq.TimeZoneOffset = zone(e.ctx) + dagReq.TimeZoneName, dagReq.TimeZoneOffset = timeutil.Zone(e.ctx.GetSessionVars().Location()) sc := e.ctx.GetSessionVars().StmtCtx dagReq.Flags = statementContextToFlags(sc) for i := range e.columns { @@ -651,7 +652,7 @@ func (e *CleanupIndexExec) Open(ctx context.Context) error { func (e *CleanupIndexExec) buildIdxDAGPB(txn kv.Transaction) (*tipb.DAGRequest, error) { dagReq := &tipb.DAGRequest{} dagReq.StartTs = txn.StartTS() - dagReq.TimeZoneName, dagReq.TimeZoneOffset = zone(e.ctx) + dagReq.TimeZoneName, dagReq.TimeZoneOffset = timeutil.Zone(e.ctx.GetSessionVars().Location()) sc := e.ctx.GetSessionVars().StmtCtx dagReq.Flags = statementContextToFlags(sc) for i := range e.idxCols { diff --git a/executor/builder.go b/executor/builder.go index ceb0245c8a17f..7965d5fe2ffff 100644 --- a/executor/builder.go +++ b/executor/builder.go @@ -44,6 +44,7 @@ import ( "github.com/pingcap/tidb/util/admin" "github.com/pingcap/tidb/util/chunk" "github.com/pingcap/tidb/util/ranger" + "github.com/pingcap/tidb/util/timeutil" "github.com/pingcap/tipb/go-tipb" "golang.org/x/net/context" ) @@ -1320,7 +1321,7 @@ func (b *executorBuilder) buildDelete(v *plan.Delete) Executor { } func (b *executorBuilder) buildAnalyzeIndexPushdown(task plan.AnalyzeIndexTask, maxNumBuckets uint64) *AnalyzeIndexExec { - _, offset := zone(b.ctx) + _, offset := timeutil.Zone(b.ctx.GetSessionVars().Location()) e := &AnalyzeIndexExec{ ctx: b.ctx, physicalTableID: task.PhysicalTableID, @@ -1353,7 +1354,7 @@ func (b *executorBuilder) buildAnalyzeColumnsPushdown(task plan.AnalyzeColumnsTa cols = append([]*model.ColumnInfo{task.PKInfo}, cols...) } - _, offset := zone(b.ctx) + _, offset := timeutil.Zone(b.ctx.GetSessionVars().Location()) e := &AnalyzeColumnsExec{ ctx: b.ctx, physicalTableID: task.PhysicalTableID, @@ -1430,7 +1431,7 @@ func constructDistExec(sctx sessionctx.Context, plans []plan.PhysicalPlan) ([]*t func (b *executorBuilder) constructDAGReq(plans []plan.PhysicalPlan) (dagReq *tipb.DAGRequest, streaming bool, err error) { dagReq = &tipb.DAGRequest{} dagReq.StartTs = b.getStartTS() - dagReq.TimeZoneName, dagReq.TimeZoneOffset = zone(b.ctx) + dagReq.TimeZoneName, dagReq.TimeZoneOffset = timeutil.Zone(b.ctx.GetSessionVars().Location()) sc := b.ctx.GetSessionVars().StmtCtx dagReq.Flags = statementContextToFlags(sc) dagReq.Executors, streaming, err = constructDistExec(b.ctx, plans) diff --git a/executor/distsql.go b/executor/distsql.go index a13db418d87eb..32b3dc6c1259f 100644 --- a/executor/distsql.go +++ b/executor/distsql.go @@ -19,7 +19,6 @@ import ( "sort" "sync" "sync/atomic" - "time" "unsafe" "github.com/juju/errors" @@ -117,24 +116,6 @@ func closeAll(objs ...Closeable) error { return errors.Trace(err) } -// zone returns the current timezone name and timezone offset in seconds. -// In compatible with MySQL, we change `Local` to `System`. -// TODO: Golang team plan to return system timezone name intead of -// returning `Local` when `loc` is `time.Local`. We need keep an eye on this. -func zone(sctx sessionctx.Context) (string, int64) { - loc := sctx.GetSessionVars().Location() - _, offset := time.Now().In(loc).Zone() - var name string - name = loc.String() - // when we found name is "Local", we have no chice but push down - // "System" to tikv side. - if name == "Local" { - name = "System" - } - - return name, int64(offset) -} - // statementContextToFlags converts StatementContext to tipb.SelectRequest.Flags. func statementContextToFlags(sc *stmtctx.StatementContext) uint64 { var flags uint64 diff --git a/util/timeutil/time.go b/util/timeutil/time.go index 3337aa2aba908..a171633e530a8 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -59,16 +59,18 @@ func initLocalStr() { // $TZ="foo" means use /usr/share/zoneinfo/foo. tz, ok := syscall.Getenv("TZ") if ok && tz != "" { - fmt.Printf("initLocalStr tz %s", tz) + fmt.Println("first if") for _, source := range zoneSources { if _, err := os.Stat(source + tz); os.IsExist(err) { localStr = tz break } } - // } else if tz == "" { - // localStr = "UTC" + } else if tz == "" { + fmt.Println("first else if") + localStr = "UTC" } else { + fmt.Println("else") path, err := filepath.EvalSymlinks("/etc/localtime") if err == nil { localStr, err = getTZNameFromFileName(path) @@ -79,6 +81,7 @@ func initLocalStr() { } log.Errorln(err) } + fmt.Printf("localStr is %s", localStr) localStr = "System" } @@ -133,3 +136,20 @@ func (lm *locCache) getLoc(name string) (*time.Location, error) { func LoadLocation(name string) (*time.Location, error) { return LocCache.getLoc(name) } + +// Zone returns the current timezone name and timezone offset in seconds. +// In compatible with MySQL, we change `Local` to `System`. +// TODO: Golang team plan to return system timezone name intead of +// returning `Local` when `loc` is `time.Local`. We need keep an eye on this. +func Zone(loc *time.Location) (string, int64) { + _, offset := time.Now().In(loc).Zone() + var name string + name = loc.String() + // when we found name is "Local", we have no chice but push down + // "System" to tikv side. + if name == "Local" { + name = "System" + } + + return name, int64(offset) +} From a84c028b18a13d4f907aac8b6a792a67c0b7b540 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Sun, 9 Sep 2018 13:42:06 +0800 Subject: [PATCH 19/57] remove prev changes --- expression/builtin_time_test.go | 6 +++--- expression/helper.go | 2 +- expression/helper_test.go | 2 +- util/timeutil/time.go | 29 +++++++++++++++-------------- util/timeutil/time_test.go | 14 +++++++++++++- 5 files changed, 33 insertions(+), 20 deletions(-) diff --git a/expression/builtin_time_test.go b/expression/builtin_time_test.go index cb5c76fd01fd3..9b456b5760f5e 100644 --- a/expression/builtin_time_test.go +++ b/expression/builtin_time_test.go @@ -775,7 +775,7 @@ func (s *testEvaluatorSuite) TestNowAndUTCTimestamp(c *C) { fc functionClass now func() time.Time }{ - {funcs[ast.Now], func() time.Time { return time.Now().In(s.ctx.GetSessionVars().Location()) }}, + {funcs[ast.Now], func() time.Time { return time.Now() }}, {funcs[ast.UTCTimestamp], func() time.Time { return time.Now().UTC() }}, } { f, err := x.fc.getFunction(s.ctx, s.datumsToConstants(nil)) @@ -1012,7 +1012,7 @@ func (s *testEvaluatorSuite) TestSysDate(c *C) { c.Assert(n.String(), GreaterEqual, last.Format(types.TimeFormat)) } - last := time.Now().In(s.ctx.GetSessionVars().Location()) + last := time.Now() f, err := fc.getFunction(ctx, s.datumsToConstants(types.MakeDatums(6))) c.Assert(err, IsNil) v, err := evalBuiltinFunc(f, chunk.Row{}) @@ -1157,7 +1157,7 @@ func (s *testEvaluatorSuite) TestCurrentTime(c *C) { defer testleak.AfterTest(c)() tfStr := "15:04:05" - last := time.Now().In(s.ctx.GetSessionVars().Location()) + last := time.Now() fc := funcs[ast.CurrentTime] f, err := fc.getFunction(s.ctx, s.datumsToConstants(types.MakeDatums(nil))) c.Assert(err, IsNil) diff --git a/expression/helper.go b/expression/helper.go index 657df5fed1092..813f45e903c85 100644 --- a/expression/helper.go +++ b/expression/helper.go @@ -121,7 +121,7 @@ func GetTimeValue(ctx sessionctx.Context, v interface{}, tp byte, fsp int) (d ty } func getSystemTimestamp(ctx sessionctx.Context) (time.Time, error) { - now := time.Now().In(ctx.GetSessionVars().Location()) + now := time.Now() if ctx == nil { return now, nil diff --git a/expression/helper_test.go b/expression/helper_test.go index dd1f9fa7840bd..f63109a5a28e6 100644 --- a/expression/helper_test.go +++ b/expression/helper_test.go @@ -68,7 +68,7 @@ func (s *testExpressionSuite) TestGetTimeValue(c *C) { Ret interface{} }{ {"2012-12-12 00:00:00", "2012-12-12 00:00:00"}, - {ast.CurrentTimestamp, time.Unix(1234, 0).In(ctx.GetSessionVars().Location()).Format(types.TimeFormat)}, + {ast.CurrentTimestamp, time.Unix(1234, 0).Format(types.TimeFormat)}, {types.ZeroDatetimeStr, "0000-00-00 00:00:00"}, {ast.NewValueExpr("2012-12-12 00:00:00"), "2012-12-12 00:00:00"}, {ast.NewValueExpr(int64(0)), "0000-00-00 00:00:00"}, diff --git a/util/timeutil/time.go b/util/timeutil/time.go index a171633e530a8..500ea2b60b42d 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -40,6 +40,8 @@ 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. @@ -57,20 +59,10 @@ func initLocalStr() { // 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") - if ok && tz != "" { - fmt.Println("first if") - for _, source := range zoneSources { - if _, err := os.Stat(source + tz); os.IsExist(err) { - localStr = tz - break - } - } - } else if tz == "" { - fmt.Println("first else if") - localStr = "UTC" - } else { - fmt.Println("else") + switch { + case !ok: path, err := filepath.EvalSymlinks("/etc/localtime") if err == nil { localStr, err = getTZNameFromFileName(path) @@ -80,8 +72,17 @@ func initLocalStr() { log.Errorln(err) } log.Errorln(err) + case tz == "": + case tz == "UTC": + localStr = "UTC" + case tz != "" && tz != "UTC": + for _, source := range zoneSources { + if _, err := os.Stat(source + tz); err == nil { + localStr = tz + return + } + } } - fmt.Printf("localStr is %s", localStr) localStr = "System" } diff --git a/util/timeutil/time_test.go b/util/timeutil/time_test.go index c3b6e98501f6a..fdb62f8132c83 100644 --- a/util/timeutil/time_test.go +++ b/util/timeutil/time_test.go @@ -22,8 +22,20 @@ func (s *testTimeSuite) TestgetTZNameFromFileName(c *C) { } func (s *testTimeSuite) TestLocal(c *C) { - os.Setenv("TZ", "UTC") + os.Setenv("TZ", "Asia/Shanghai") loc := Local() + c.Assert(loc.String(), Equals, "Asia/Shanghai") + + os.Setenv("TZ", "UTC") + // reset localStr + initLocalStr() + loc = Local() + c.Assert(loc.String(), Equals, "UTC") + + os.Setenv("TZ", "") + // reset localStr + initLocalStr() + loc = Local() c.Assert(loc.String(), Equals, "UTC") os.Unsetenv("TZ") } From c1658d8395fa8b95e90825a45ea9730cfe3861ef Mon Sep 17 00:00:00 2001 From: zhexuany Date: Tue, 11 Sep 2018 10:04:43 +0000 Subject: [PATCH 20/57] init localstr in bootstrap stage --- session/bootstrap.go | 18 +++++++++++++++++- session/session.go | 2 +- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/session/bootstrap.go b/session/bootstrap.go index c4298907d66b3..6a1c39c3174c3 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -33,6 +33,7 @@ import ( "github.com/pingcap/tidb/terror" "github.com/pingcap/tidb/util/auth" "github.com/pingcap/tidb/util/chunk" + "github.com/pingcap/tidb/util/timeutil" log "github.com/sirupsen/logrus" "golang.org/x/net/context" ) @@ -233,7 +234,10 @@ const ( bootstrappedVarTrue = "True" // The variable name in mysql.TiDB table. // It is used for getting the version of the TiDB server which bootstrapped the store. - tidbServerVersionVar = "tidb_server_version" // + tidbServerVersionVar = "tidb_server_version" + // The variable name in mysql.tidb table and it will be used when we want to know + // system timezone. + systemTZ = "system_tz" // Const for TiDB server version 2. version2 = 2 version3 = 3 @@ -257,6 +261,7 @@ const ( version21 = 21 version22 = 22 version23 = 23 + version24 = 24 ) func checkBootstrapped(s Session) (bool, error) { @@ -407,6 +412,10 @@ func upgrade(s Session) { upgradeToVer23(s) } + if ver < version24 { + upgradeToVer24(s) + } + updateBootstrapVer(s) _, err = s.Execute(context.Background(), "COMMIT") @@ -649,6 +658,13 @@ func upgradeToVer23(s Session) { doReentrantDDL(s, "ALTER TABLE mysql.stats_histograms ADD COLUMN `flag` bigint(64) NOT NULL DEFAULT 0", infoschema.ErrColumnExists) } +// upgradeToVer24 initializes `Sysstem` timezone according to docs/design/2018-09-10-adding-tz-env.md +func upgradeToVer24(s Session) { + sql := fmt.Sprintf(`INSERT HIGH_PRIORITY INTO %s.%s VALUES ("%s", "%s", "TiDB Global System Timezone).")`, + mysql.SystemDB, mysql.TiDBTable, tidbServerVersionVar, timeutil.Local().String()) + mustExecute(s, sql) +} + // updateBootstrapVer updates bootstrap version variable in mysql.TiDB table. func updateBootstrapVer(s Session) { // Update bootstrap version. diff --git a/session/session.go b/session/session.go index 9f5e662edcfdf..dac563c1d9edc 100644 --- a/session/session.go +++ b/session/session.go @@ -1180,7 +1180,7 @@ func createSessionWithDomain(store kv.Storage, dom *domain.Domain) (*session, er const ( notBootstrapped = 0 - currentBootstrapVersion = 23 + currentBootstrapVersion = 24 ) func getStoreBootstrapVersion(store kv.Storage) int64 { From a517a4361a7251a95c39396361b655d6de012a68 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Tue, 11 Sep 2018 10:17:52 +0000 Subject: [PATCH 21/57] init localstr in bootstrap stage --- session/bootstrap.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/session/bootstrap.go b/session/bootstrap.go index 6a1c39c3174c3..733e69a7d5889 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -237,7 +237,7 @@ const ( tidbServerVersionVar = "tidb_server_version" // The variable name in mysql.tidb table and it will be used when we want to know // system timezone. - systemTZ = "system_tz" + tidbSystemTZ = "system_tz" // Const for TiDB server version 2. version2 = 2 version3 = 3 @@ -660,8 +660,8 @@ func upgradeToVer23(s Session) { // upgradeToVer24 initializes `Sysstem` timezone according to docs/design/2018-09-10-adding-tz-env.md func upgradeToVer24(s Session) { - sql := fmt.Sprintf(`INSERT HIGH_PRIORITY INTO %s.%s VALUES ("%s", "%s", "TiDB Global System Timezone).")`, - mysql.SystemDB, mysql.TiDBTable, tidbServerVersionVar, timeutil.Local().String()) + sql := fmt.Sprintf(`INSERT HIGH_PRIORITY INTO %s.%s VALUES ("%s", "%s", "TiDB Global System Timezone.")`, + mysql.SystemDB, mysql.TiDBTable, tidbSystemTZ, timeutil.Local().String()) mustExecute(s, sql) } From b5e39b6fed7ebb7acd4f6d47973c3cb526eb5483 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Wed, 12 Sep 2018 19:36:56 +0800 Subject: [PATCH 22/57] load system tz by select --- session/bootstrap.go | 27 +++++++++++++++++++++++---- util/timeutil/time.go | 29 ++++++++++++++--------------- util/timeutil/time_test.go | 17 +++++++++++++++++ 3 files changed, 54 insertions(+), 19 deletions(-) diff --git a/session/bootstrap.go b/session/bootstrap.go index 733e69a7d5889..d32807f602235 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -416,6 +416,19 @@ func upgrade(s Session) { upgradeToVer24(s) } + timeutil.LoadSystemTZ = func() string { + sql := `select variable_value from mysql.tidb where variable_name = "system_tz"` + rss, err := s.Execute(context.Background(), sql) + if err != nil { + log.Fatal(err) + } + chk := rss[0].NewChunk() + rss[0].Next(context.Background(), chk) + return chk.GetRow(0).GetString(0) + } + + timeutil.LocalStr = timeutil.LoadSystemTZ() + updateBootstrapVer(s) _, err = s.Execute(context.Background(), "COMMIT") @@ -658,13 +671,18 @@ func upgradeToVer23(s Session) { doReentrantDDL(s, "ALTER TABLE mysql.stats_histograms ADD COLUMN `flag` bigint(64) NOT NULL DEFAULT 0", infoschema.ErrColumnExists) } -// upgradeToVer24 initializes `Sysstem` timezone according to docs/design/2018-09-10-adding-tz-env.md -func upgradeToVer24(s Session) { - sql := fmt.Sprintf(`INSERT HIGH_PRIORITY INTO %s.%s VALUES ("%s", "%s", "TiDB Global System Timezone.")`, - mysql.SystemDB, mysql.TiDBTable, tidbSystemTZ, timeutil.Local().String()) +//writeSystemTZ writes system timezone info into mysql.tidb +func writeSystemTZ(s Session) { + sql := fmt.Sprintf(`INSERT HIGH_PRIORITY INTO %s.%s VALUES ("%s", "%s", "TiDB Global System Timezone.") ON DUPLICATE KEY UPDATE VARIABLE_VALUE="%s"`, + mysql.SystemDB, mysql.TiDBTable, tidbSystemTZ, timeutil.GetSystemTZ(), timeutil.GetSystemTZ()) mustExecute(s, sql) } +// upgradeToVer24 initializes `System` timezone according to docs/design/2018-09-10-adding-tz-env.md +func upgradeToVer24(s Session) { + writeSystemTZ(s) +} + // updateBootstrapVer updates bootstrap version variable in mysql.TiDB table. func updateBootstrapVer(s Session) { // Update bootstrap version. @@ -748,6 +766,7 @@ func doDMLWorks(s Session) { mysql.SystemDB, mysql.TiDBTable, tidbServerVersionVar, currentBootstrapVersion) mustExecute(s, sql) + writeSystemTZ(s) _, err := s.Execute(context.Background(), "COMMIT") if err != nil { time.Sleep(1 * time.Second) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index 500ea2b60b42d..68f22fe301cea 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -34,13 +34,17 @@ func init() { // LocCache is a simple cache policy to improve the performance of 'time.LoadLocation'. var LocCache *locCache -var localStr string -var localOnce sync.Once + +// LoadSystemTZ loads system timezone from mysql.tidb and returns the value. +var LoadSystemTZ func() string + +// LocalStr is current TiDB's system timezone name. +var LocalStr string var zoneSources = []string{ "/usr/share/zoneinfo/", "/usr/share/lib/zoneinfo/", "/usr/lib/locale/TZ/", - // this is for macos + // this is for macOS "/var/db/timezone/zoneinfo", } @@ -54,36 +58,32 @@ type locCache struct { locMap map[string]*time.Location } -func initLocalStr() { +// GetSystemTZ 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`. +func GetSystemTZ() 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, err := filepath.EvalSymlinks("/etc/localtime") if err == nil { - localStr, err = getTZNameFromFileName(path) + name, err := getTZNameFromFileName(path) if err == nil { - return + return name } log.Errorln(err) } log.Errorln(err) - case tz == "": - case tz == "UTC": - localStr = "UTC" case tz != "" && tz != "UTC": for _, source := range zoneSources { if _, err := os.Stat(source + tz); err == nil { - localStr = tz - return + return tz } } } - localStr = "System" + return "UTC" } // getTZNameFromFileName gets IANA timezone name from zoneinfo path. @@ -100,8 +100,7 @@ func getTZNameFromFileName(path string) (string, error) { // Local returns time.Local's IANA timezone name. func Local() *time.Location { - localOnce.Do(initLocalStr) - loc, err := LoadLocation(localStr) + loc, err := LoadLocation(LocalStr) if err != nil { return time.Local } diff --git a/util/timeutil/time_test.go b/util/timeutil/time_test.go index fdb62f8132c83..c168620051363 100644 --- a/util/timeutil/time_test.go +++ b/util/timeutil/time_test.go @@ -1,3 +1,20 @@ +// Copyright 2018 The ql Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSES/QL-LICENSE file. + +// Copyright 2015 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + package timeutil import ( From 84b6e8fa53134e468e11cbf9573d76b7031260f3 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Wed, 12 Sep 2018 20:02:22 +0800 Subject: [PATCH 23/57] address comment --- session/bootstrap.go | 6 +++--- util/timeutil/time.go | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/session/bootstrap.go b/session/bootstrap.go index d32807f602235..929d12f6faa90 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -416,6 +416,9 @@ func upgrade(s Session) { upgradeToVer24(s) } + updateBootstrapVer(s) + _, err = s.Execute(context.Background(), "COMMIT") + timeutil.LoadSystemTZ = func() string { sql := `select variable_value from mysql.tidb where variable_name = "system_tz"` rss, err := s.Execute(context.Background(), sql) @@ -429,9 +432,6 @@ func upgrade(s Session) { timeutil.LocalStr = timeutil.LoadSystemTZ() - updateBootstrapVer(s) - _, err = s.Execute(context.Background(), "COMMIT") - if err != nil { time.Sleep(1 * time.Second) // Check if TiDB is already upgraded. diff --git a/util/timeutil/time.go b/util/timeutil/time.go index 68f22fe301cea..8071bcb96b685 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -59,6 +59,7 @@ type locCache struct { } // GetSystemTZ 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. func GetSystemTZ() string { // consult $TZ to find the time zone to use. // no $TZ means use the system default /etc/localtime. From dae8a5a4489355d9e019eb2622e5a24eac395f14 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Wed, 12 Sep 2018 20:14:06 +0800 Subject: [PATCH 24/57] correct test case --- util/timeutil/time_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/timeutil/time_test.go b/util/timeutil/time_test.go index c168620051363..56d110ea07f17 100644 --- a/util/timeutil/time_test.go +++ b/util/timeutil/time_test.go @@ -45,13 +45,13 @@ func (s *testTimeSuite) TestLocal(c *C) { os.Setenv("TZ", "UTC") // reset localStr - initLocalStr() + GetSystemTZ() loc = Local() c.Assert(loc.String(), Equals, "UTC") os.Setenv("TZ", "") // reset localStr - initLocalStr() + GetSystemTZ() loc = Local() c.Assert(loc.String(), Equals, "UTC") os.Unsetenv("TZ") From 2d0aca2b3acdce38fd37d536e9ea25a45686bec6 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Wed, 12 Sep 2018 20:31:14 +0800 Subject: [PATCH 25/57] fix ci --- util/timeutil/time_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/util/timeutil/time_test.go b/util/timeutil/time_test.go index 7e08975e51a17..234783d54fc27 100644 --- a/util/timeutil/time_test.go +++ b/util/timeutil/time_test.go @@ -21,6 +21,7 @@ import ( "os" "testing" + "fmt" . "github.com/pingcap/check" ) @@ -40,19 +41,21 @@ func (s *testTimeSuite) TestgetTZNameFromFileName(c *C) { func (s *testTimeSuite) TestLocal(c *C) { os.Setenv("TZ", "Asia/Shanghai") - GetSystemTZ() + LocalStr = GetSystemTZ() loc := Local() + fmt.Println(LocalStr) + c.Assert(LocalStr, Equals, "Asia/Shanghai") c.Assert(loc.String(), Equals, "Asia/Shanghai") os.Setenv("TZ", "UTC") // reset localStr - GetSystemTZ() + LocalStr = GetSystemTZ() loc = Local() c.Assert(loc.String(), Equals, "UTC") os.Setenv("TZ", "") // reset localStr - GetSystemTZ() + LocalStr = GetSystemTZ() loc = Local() c.Assert(loc.String(), Equals, "UTC") os.Unsetenv("TZ") From e624443cc1ad3d1f1370af48d38d1a8da22fa303 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Wed, 12 Sep 2018 20:39:54 +0800 Subject: [PATCH 26/57] remove shadow err --- util/timeutil/time.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index 8071bcb96b685..dde55b2f9e0ad 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -68,15 +68,15 @@ func GetSystemTZ() string { tz, ok := syscall.Getenv("TZ") switch { case !ok: - path, err := filepath.EvalSymlinks("/etc/localtime") - if err == nil { - name, err := getTZNameFromFileName(path) - if err == nil { + path, err1 := filepath.EvalSymlinks("/etc/localtime") + if err1 == nil { + name, err2 := getTZNameFromFileName(path) + if err2 == nil { return name } - log.Errorln(err) + log.Errorln(err2) } - log.Errorln(err) + log.Errorln(err1) case tz != "" && tz != "UTC": for _, source := range zoneSources { if _, err := os.Stat(source + tz); err == nil { From f3ad3bc9c7e9fa708d5aafa294b7fd746b98cffc Mon Sep 17 00:00:00 2001 From: zhexuany Date: Wed, 12 Sep 2018 20:48:58 +0800 Subject: [PATCH 27/57] remove err shadow --- session/bootstrap.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/session/bootstrap.go b/session/bootstrap.go index 3f24afd5fcdc6..10cccdbc251b2 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -421,9 +421,9 @@ func upgrade(s Session) { timeutil.LoadSystemTZ = func() string { sql := `select variable_value from mysql.tidb where variable_name = "system_tz"` - rss, err := s.Execute(context.Background(), sql) - if err != nil { - log.Fatal(err) + rss, errLoad := s.Execute(context.Background(), sql) + if errLoad != nil { + log.Fatal(errLoad) } chk := rss[0].NewChunk() rss[0].Next(context.Background(), chk) From 692d4d5a17520ff35018328a9f787eec4d5b391c Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 14:00:01 +0800 Subject: [PATCH 28/57] address comment --- util/timeutil/time.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index dde55b2f9e0ad..744a9a4baef06 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -28,6 +28,9 @@ import ( // init initializes `locCache`. func init() { + if LocalStr == "" { + LocalStr = "System" + } LocCache = &locCache{} LocCache.locMap = make(map[string]*time.Location) } From b32bdd2180dac856d06a88ef899565f20018ce9a Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 14:01:20 +0800 Subject: [PATCH 29/57] remove extra print --- util/timeutil/time_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/util/timeutil/time_test.go b/util/timeutil/time_test.go index 234783d54fc27..5755d6ecdf860 100644 --- a/util/timeutil/time_test.go +++ b/util/timeutil/time_test.go @@ -43,7 +43,6 @@ func (s *testTimeSuite) TestLocal(c *C) { os.Setenv("TZ", "Asia/Shanghai") LocalStr = GetSystemTZ() loc := Local() - fmt.Println(LocalStr) c.Assert(LocalStr, Equals, "Asia/Shanghai") c.Assert(loc.String(), Equals, "Asia/Shanghai") From 49454d55fbfebe59895f7cd14f18abe6143ce49d Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 14:03:07 +0800 Subject: [PATCH 30/57] add more comment --- util/timeutil/time.go | 1 + 1 file changed, 1 insertion(+) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index 744a9a4baef06..f40ae52338803 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -28,6 +28,7 @@ import ( // init initializes `locCache`. func init() { + // We need set LocalStr when it is in testing process. if LocalStr == "" { LocalStr = "System" } From 318c12b1470037d76b57382a6f040d4ce35712f4 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 17:54:04 +0800 Subject: [PATCH 31/57] address comments --- session/bootstrap.go | 4 ++-- util/timeutil/time.go | 28 +++++++++++++++------------- util/timeutil/time_test.go | 1 - 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/session/bootstrap.go b/session/bootstrap.go index 10cccdbc251b2..aa6c7bed7cc4f 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -419,7 +419,7 @@ func upgrade(s Session) { updateBootstrapVer(s) _, err = s.Execute(context.Background(), "COMMIT") - timeutil.LoadSystemTZ = func() string { + callback := func() string { sql := `select variable_value from mysql.tidb where variable_name = "system_tz"` rss, errLoad := s.Execute(context.Background(), sql) if errLoad != nil { @@ -430,7 +430,7 @@ func upgrade(s Session) { return chk.GetRow(0).GetString(0) } - timeutil.LocalStr = timeutil.LoadSystemTZ() + timeutil.LoadLocalStrFromTB(callback()) if err != nil { time.Sleep(1 * time.Second) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index f40ae52338803..09b30712df14d 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -28,22 +28,19 @@ import ( // init initializes `locCache`. func init() { - // We need set LocalStr when it is in testing process. - if LocalStr == "" { - LocalStr = "System" + // We need set localStr when it is in testing process. + if localStr == "" { + localStr = "System" } - LocCache = &locCache{} - LocCache.locMap = make(map[string]*time.Location) + locCa = &locCache{} + locCa.locMap = make(map[string]*time.Location) } -// LocCache is a simple cache policy to improve the performance of 'time.LoadLocation'. -var LocCache *locCache - -// LoadSystemTZ loads system timezone from mysql.tidb and returns the value. -var LoadSystemTZ func() string +// locCa is a simple cache policy to improve the performance of 'time.LoadLocation'. +var locCa *locCache // LocalStr is current TiDB's system timezone name. -var LocalStr string +var localStr string var zoneSources = []string{ "/usr/share/zoneinfo/", "/usr/share/lib/zoneinfo/", @@ -105,13 +102,18 @@ func getTZNameFromFileName(path string) (string, error) { // Local returns time.Local's IANA timezone name. func Local() *time.Location { - loc, err := LoadLocation(LocalStr) + loc, err := LoadLocation(localStr) if err != nil { return time.Local } return loc } +// LoadLocalStrFromTB loads system timezone from mysql.tidb and returns the value. +func LoadLocalStrFromTB(name string) { + localStr = name +} + // getLoc first trying to load location from a cache map. If nothing found in such map, then call // `time.LocadLocation` to get a timezone location. After trying both way, an error wil be returned // if valid Location is not found. @@ -139,7 +141,7 @@ func (lm *locCache) getLoc(name string) (*time.Location, error) { // LoadLocation loads time.Location by IANA timezone time. func LoadLocation(name string) (*time.Location, error) { - return LocCache.getLoc(name) + return locCa.getLoc(name) } // Zone returns the current timezone name and timezone offset in seconds. diff --git a/util/timeutil/time_test.go b/util/timeutil/time_test.go index 5755d6ecdf860..4a3bd46ca0164 100644 --- a/util/timeutil/time_test.go +++ b/util/timeutil/time_test.go @@ -21,7 +21,6 @@ import ( "os" "testing" - "fmt" . "github.com/pingcap/check" ) From cb5fd7d82b66e3978f81b828e8fe6653261e4a79 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 17:55:51 +0800 Subject: [PATCH 32/57] address comments --- util/timeutil/time.go | 12 ++++++------ util/timeutil/time_test.go | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index 09b30712df14d..91af74c5c4781 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -59,9 +59,9 @@ type locCache struct { locMap map[string]*time.Location } -// GetSystemTZ 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`, 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. -func GetSystemTZ() string { +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. @@ -71,7 +71,7 @@ func GetSystemTZ() string { case !ok: path, err1 := filepath.EvalSymlinks("/etc/localtime") if err1 == nil { - name, err2 := getTZNameFromFileName(path) + name, err2 := inferTZNameFromFileName(path) if err2 == nil { return name } @@ -88,9 +88,9 @@ func GetSystemTZ() string { return "UTC" } -// getTZNameFromFileName gets IANA timezone name from zoneinfo path. -// TODO It will be refined later. This is just a quick fix. -func getTZNameFromFileName(path string) (string, error) { +// 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" if strings.Contains(path, substr) { diff --git a/util/timeutil/time_test.go b/util/timeutil/time_test.go index 4a3bd46ca0164..849c7b925e6e9 100644 --- a/util/timeutil/time_test.go +++ b/util/timeutil/time_test.go @@ -33,27 +33,27 @@ func TestT(t *testing.T) { type testTimeSuite struct{} func (s *testTimeSuite) TestgetTZNameFromFileName(c *C) { - tz, err := getTZNameFromFileName("/user/share/zoneinfo/Asia/Shanghai") + tz, err := inferTZNameFromFileName("/user/share/zoneinfo/Asia/Shanghai") c.Assert(err, IsNil) c.Assert(tz, Equals, "Asia/Shanghai") } func (s *testTimeSuite) TestLocal(c *C) { os.Setenv("TZ", "Asia/Shanghai") - LocalStr = GetSystemTZ() + LocalStr = InferSystemTZ() loc := Local() c.Assert(LocalStr, Equals, "Asia/Shanghai") c.Assert(loc.String(), Equals, "Asia/Shanghai") os.Setenv("TZ", "UTC") // reset localStr - LocalStr = GetSystemTZ() + LocalStr = InferSystemTZ() loc = Local() c.Assert(loc.String(), Equals, "UTC") os.Setenv("TZ", "") // reset localStr - LocalStr = GetSystemTZ() + LocalStr = InferSystemTZ() loc = Local() c.Assert(loc.String(), Equals, "UTC") os.Unsetenv("TZ") From ec66759aec7e9b80263909b4de343657db9f3e58 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 18:04:54 +0800 Subject: [PATCH 33/57] fix build error --- session/bootstrap.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/session/bootstrap.go b/session/bootstrap.go index aa6c7bed7cc4f..7c05b6d3b606c 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -674,7 +674,7 @@ func upgradeToVer23(s Session) { //writeSystemTZ writes system timezone info into mysql.tidb func writeSystemTZ(s Session) { sql := fmt.Sprintf(`INSERT HIGH_PRIORITY INTO %s.%s VALUES ("%s", "%s", "TiDB Global System Timezone.") ON DUPLICATE KEY UPDATE VARIABLE_VALUE="%s"`, - mysql.SystemDB, mysql.TiDBTable, tidbSystemTZ, timeutil.GetSystemTZ(), timeutil.GetSystemTZ()) + mysql.SystemDB, mysql.TiDBTable, tidbSystemTZ, timeutil.InferSystemTZ(), timeutil.InferSystemTZ()) mustExecute(s, sql) } From a3c3cc470dd24951333e4f37f27b91de3ae633c8 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 18:11:41 +0800 Subject: [PATCH 34/57] fix test error --- util/timeutil/time_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/util/timeutil/time_test.go b/util/timeutil/time_test.go index 849c7b925e6e9..9e770c7141226 100644 --- a/util/timeutil/time_test.go +++ b/util/timeutil/time_test.go @@ -40,20 +40,20 @@ func (s *testTimeSuite) TestgetTZNameFromFileName(c *C) { func (s *testTimeSuite) TestLocal(c *C) { os.Setenv("TZ", "Asia/Shanghai") - LocalStr = InferSystemTZ() + localStr = InferSystemTZ() loc := Local() - c.Assert(LocalStr, Equals, "Asia/Shanghai") + c.Assert(localStr, Equals, "Asia/Shanghai") c.Assert(loc.String(), Equals, "Asia/Shanghai") os.Setenv("TZ", "UTC") // reset localStr - LocalStr = InferSystemTZ() + localStr = InferSystemTZ() loc = Local() c.Assert(loc.String(), Equals, "UTC") os.Setenv("TZ", "") // reset localStr - LocalStr = InferSystemTZ() + localStr = InferSystemTZ() loc = Local() c.Assert(loc.String(), Equals, "UTC") os.Unsetenv("TZ") From 3976eda668bca245711c1a8d70cfeb8aa46951bd Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 12:14:42 +0000 Subject: [PATCH 35/57] load system tz after bootstrap --- session/bootstrap.go | 13 ------------- session/session.go | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/session/bootstrap.go b/session/bootstrap.go index 7c05b6d3b606c..fa3712ac07aae 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -419,19 +419,6 @@ func upgrade(s Session) { updateBootstrapVer(s) _, err = s.Execute(context.Background(), "COMMIT") - callback := func() string { - sql := `select variable_value from mysql.tidb where variable_name = "system_tz"` - rss, errLoad := s.Execute(context.Background(), sql) - if errLoad != nil { - log.Fatal(errLoad) - } - chk := rss[0].NewChunk() - rss[0].Next(context.Background(), chk) - return chk.GetRow(0).GetString(0) - } - - timeutil.LoadLocalStrFromTB(callback()) - if err != nil { time.Sleep(1 * time.Second) // Check if TiDB is already upgraded. diff --git a/session/session.go b/session/session.go index bc13959d37724..5f3612abf8b9a 100644 --- a/session/session.go +++ b/session/session.go @@ -53,6 +53,7 @@ import ( "github.com/pingcap/tidb/util/charset" "github.com/pingcap/tidb/util/chunk" "github.com/pingcap/tidb/util/kvcache" + "github.com/pingcap/tidb/util/timeutil" binlog "github.com/pingcap/tipb/go-binlog" "github.com/pkg/errors" log "github.com/sirupsen/logrus" @@ -1086,6 +1087,21 @@ func BootstrapSession(store kv.Storage) (*domain.Domain, error) { if err != nil { return nil, errors.Trace(err) } + + // get system tz from mysql.tidb + callback := func() string { + sql := `select variable_value from mysql.tidb where variable_name = "system_tz"` + rss, errLoad := se.Execute(context.Background(), sql) + if errLoad != nil { + log.Fatal(errLoad) + } + chk := rss[0].NewChunk() + rss[0].Next(context.Background(), chk) + return chk.GetRow(0).GetString(0) + } + + timeutil.LoadLocalStrFromTB(callback()) + se1, err := createSession(store) if err != nil { return nil, errors.Trace(err) From d52298261f4479be8d30ee9de028bbac66f172bc Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 12:20:01 +0000 Subject: [PATCH 36/57] address comments --- session/session.go | 2 +- util/timeutil/time.go | 16 ++++++++-------- util/timeutil/time_test.go | 12 ++++++------ 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/session/session.go b/session/session.go index 5f3612abf8b9a..6286db53f72e3 100644 --- a/session/session.go +++ b/session/session.go @@ -1100,7 +1100,7 @@ func BootstrapSession(store kv.Storage) (*domain.Domain, error) { return chk.GetRow(0).GetString(0) } - timeutil.LoadLocalStrFromTB(callback()) + timeutil.SetSystemTZ(callback()) se1, err := createSession(store) if err != nil { diff --git a/util/timeutil/time.go b/util/timeutil/time.go index 91af74c5c4781..6d7b0f3cf6036 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -28,9 +28,9 @@ import ( // init initializes `locCache`. func init() { - // We need set localStr when it is in testing process. - if localStr == "" { - localStr = "System" + // We need set systemTZ when it is in testing process. + if systemTZ == "" { + systemTZ = "System" } locCa = &locCache{} locCa.locMap = make(map[string]*time.Location) @@ -40,7 +40,7 @@ func init() { var locCa *locCache // LocalStr is current TiDB's system timezone name. -var localStr string +var systemTZ string var zoneSources = []string{ "/usr/share/zoneinfo/", "/usr/share/lib/zoneinfo/", @@ -102,16 +102,16 @@ func inferTZNameFromFileName(path string) (string, error) { // Local returns time.Local's IANA timezone name. func Local() *time.Location { - loc, err := LoadLocation(localStr) + loc, err := LoadLocation(systemTZ) if err != nil { return time.Local } return loc } -// LoadLocalStrFromTB loads system timezone from mysql.tidb and returns the value. -func LoadLocalStrFromTB(name string) { - localStr = name +// SetSystemTZ loads system timezone from mysql.tidb and returns the value. +func SetSystemTZ(name string) { + systemTZ = name } // getLoc first trying to load location from a cache map. If nothing found in such map, then call diff --git a/util/timeutil/time_test.go b/util/timeutil/time_test.go index 9e770c7141226..23d52a9da3b36 100644 --- a/util/timeutil/time_test.go +++ b/util/timeutil/time_test.go @@ -40,20 +40,20 @@ func (s *testTimeSuite) TestgetTZNameFromFileName(c *C) { func (s *testTimeSuite) TestLocal(c *C) { os.Setenv("TZ", "Asia/Shanghai") - localStr = InferSystemTZ() + systemTZ = InferSystemTZ() loc := Local() - c.Assert(localStr, Equals, "Asia/Shanghai") + c.Assert(systemTZ, Equals, "Asia/Shanghai") c.Assert(loc.String(), Equals, "Asia/Shanghai") os.Setenv("TZ", "UTC") - // reset localStr - localStr = InferSystemTZ() + // reset systemTZ + systemTZ = InferSystemTZ() loc = Local() c.Assert(loc.String(), Equals, "UTC") os.Setenv("TZ", "") - // reset localStr - localStr = InferSystemTZ() + // reset systemTZ + systemTZ = InferSystemTZ() loc = Local() c.Assert(loc.String(), Equals, "UTC") os.Unsetenv("TZ") From dd2d0c939a4aa8fc1ec7929009e7fa8c944ad4f3 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 13:05:43 +0000 Subject: [PATCH 37/57] move code around to avoid critical section --- session/session.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/session/session.go b/session/session.go index 6286db53f72e3..a741fc908768e 100644 --- a/session/session.go +++ b/session/session.go @@ -1082,12 +1082,6 @@ func BootstrapSession(store kv.Storage) (*domain.Domain, error) { if err != nil { return nil, errors.Trace(err) } - dom := domain.GetDomain(se) - err = dom.LoadPrivilegeLoop(se) - if err != nil { - return nil, errors.Trace(err) - } - // get system tz from mysql.tidb callback := func() string { sql := `select variable_value from mysql.tidb where variable_name = "system_tz"` @@ -1102,6 +1096,14 @@ func BootstrapSession(store kv.Storage) (*domain.Domain, error) { timeutil.SetSystemTZ(callback()) + dom := domain.GetDomain(se) + // after this se is not thread-safe. It could be used by another + // goroutine + err = dom.LoadPrivilegeLoop(se) + if err != nil { + return nil, errors.Trace(err) + } + se1, err := createSession(store) if err != nil { return nil, errors.Trace(err) From 29679ab5f2f3af2d4264d794b86a7d4e70137cb4 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 13:13:14 +0000 Subject: [PATCH 38/57] remove callback --- session/session.go | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/session/session.go b/session/session.go index a741fc908768e..3c85b7b0ed55e 100644 --- a/session/session.go +++ b/session/session.go @@ -1069,6 +1069,18 @@ func CreateSession(store kv.Storage) (Session, error) { return s, nil } +func loadSystemTZ(se *session) (string, error) { + sql := `select variable_value from mysql.tidb where variable_name = "system_tz"` + rss, errLoad := se.Execute(context.Background(), sql) + if errLoad != nil { + return "", errLoad + } + chk := rss[0].NewChunk() + rss[0].Next(context.Background(), chk) + return chk.GetRow(0).GetString(0), nil + +} + // BootstrapSession runs the first time when the TiDB server start. func BootstrapSession(store kv.Storage) (*domain.Domain, error) { ver := getStoreBootstrapVersion(store) @@ -1083,19 +1095,12 @@ func BootstrapSession(store kv.Storage) (*domain.Domain, error) { return nil, errors.Trace(err) } // get system tz from mysql.tidb - callback := func() string { - sql := `select variable_value from mysql.tidb where variable_name = "system_tz"` - rss, errLoad := se.Execute(context.Background(), sql) - if errLoad != nil { - log.Fatal(errLoad) - } - chk := rss[0].NewChunk() - rss[0].Next(context.Background(), chk) - return chk.GetRow(0).GetString(0) + if tz, err := loadSystemTZ(se); err != nil { + return nil, errors.Trace(err) + } else { + timeutil.SetSystemTZ(tz) } - timeutil.SetSystemTZ(callback()) - dom := domain.GetDomain(se) // after this se is not thread-safe. It could be used by another // goroutine From 733331eed2f236c5f9e5512f13e1d31701fe921c Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 21:15:17 +0800 Subject: [PATCH 39/57] remove extra comment --- session/session.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/session/session.go b/session/session.go index 3c85b7b0ed55e..9535e04f5928d 100644 --- a/session/session.go +++ b/session/session.go @@ -1102,8 +1102,6 @@ func BootstrapSession(store kv.Storage) (*domain.Domain, error) { } dom := domain.GetDomain(se) - // after this se is not thread-safe. It could be used by another - // goroutine err = dom.LoadPrivilegeLoop(se) if err != nil { return nil, errors.Trace(err) From e9bb73b57dd0d43aec4d146da566132431bf95f9 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 13:21:35 +0000 Subject: [PATCH 40/57] polish comment of fn --- 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 6d7b0f3cf6036..c8ab3492bef8f 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -109,7 +109,7 @@ func Local() *time.Location { return loc } -// SetSystemTZ loads system timezone from mysql.tidb and returns the value. +// SetSystemTZ sets systemTZ by the value loaded from mysql.tidb. func SetSystemTZ(name string) { systemTZ = name } From 1d8ddfa6622c7d44f7360995da8da73a05116ea6 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 13:23:35 +0000 Subject: [PATCH 41/57] fix ci --- session/session.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/session/session.go b/session/session.go index 9535e04f5928d..51502e0c6e6c5 100644 --- a/session/session.go +++ b/session/session.go @@ -1095,12 +1095,13 @@ func BootstrapSession(store kv.Storage) (*domain.Domain, error) { return nil, errors.Trace(err) } // get system tz from mysql.tidb - if tz, err := loadSystemTZ(se); err != nil { + tz, err := loadSystemTZ(se) + if err != nil { return nil, errors.Trace(err) - } else { - timeutil.SetSystemTZ(tz) } + timeutil.SetSystemTZ(tz) + dom := domain.GetDomain(se) err = dom.LoadPrivilegeLoop(se) if err != nil { From bd24bee38cdf978a5ddeb67122c9fcc3eb788b51 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 13:26:35 +0000 Subject: [PATCH 42/57] make code cleanner --- util/timeutil/time.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index c8ab3492bef8f..1949cd6bc8297 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -122,20 +122,20 @@ func (lm *locCache) getLoc(name string) (*time.Location, error) { return time.Local, nil } lm.RLock() - if v, ok := lm.locMap[name]; ok { - lm.RUnlock() + v, ok := lm.locMap[name] + lm.RUnlock() + if ok { return v, nil } if loc, err := time.LoadLocation(name); err == nil { - lm.RUnlock() + // assign value back to map lm.Lock() lm.locMap[name] = loc lm.Unlock() return loc, nil } - lm.RUnlock() return nil, fmt.Errorf("invalid name for timezone %s", name) } From 2f8018ab8852e3940f44e1dc1fec84179bc3cacd Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 13:36:06 +0000 Subject: [PATCH 43/57] correct set time_zone behavior --- sessionctx/variable/varsutil.go | 4 ++-- util/timeutil/time.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index 176e99ebacaa2..805a9e0d91ecb 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -25,6 +25,7 @@ import ( "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/mysql" "github.com/pingcap/tidb/types" + "github.com/pingcap/tidb/util/timeutil" "github.com/pkg/errors" ) @@ -350,8 +351,7 @@ func tidbOptInt64(opt string, defaultVal int64) int64 { func parseTimeZone(s string) (*time.Location, error) { if strings.EqualFold(s, "SYSTEM") { - // TODO: Support global time_zone variable, it should be set to global time_zone value. - return time.Local, nil + return timeutil.Local(), nil } loc, err := time.LoadLocation(s) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index 1949cd6bc8297..a0aa117e1af2a 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -100,7 +100,7 @@ func inferTZNameFromFileName(path string) (string, error) { return "", errors.New(fmt.Sprintf("path %s is not supported", path)) } -// Local returns time.Local's IANA timezone name. +// Local returns time.Local's IANA timezone name. It is TiDB's global timezone name. func Local() *time.Location { loc, err := LoadLocation(systemTZ) if err != nil { From 7d6ae04cb949d88fdec49b01b60cc7f7892f1899 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 22:32:19 +0800 Subject: [PATCH 44/57] add test --- distsql/distsql.go | 4 ++++ executor/executor_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/distsql/distsql.go b/distsql/distsql.go index b681fbc816bdf..37567b35a80f8 100644 --- a/distsql/distsql.go +++ b/distsql/distsql.go @@ -36,6 +36,10 @@ func Select(ctx context.Context, sctx sessionctx.Context, kvReq *kv.Request, fie hook.(func(*kv.Request))(kvReq) } + if hook := ctx.Value("CheckTimezonePushDownHook"); hook != nil { + hook.(func(*kv.Request))(kvReq) + } + if !sctx.GetSessionVars().EnableStreaming { kvReq.Streaming = false } diff --git a/executor/executor_test.go b/executor/executor_test.go index e45effd90f412..ebf3ee9afc1f4 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -26,6 +26,7 @@ import ( "time" gofail "github.com/etcd-io/gofail/runtime" + "github.com/golang/protobuf/proto" . "github.com/pingcap/check" pb "github.com/pingcap/kvproto/pkg/kvrpcpb" "github.com/pingcap/tidb/config" @@ -58,6 +59,8 @@ import ( "github.com/pingcap/tidb/util/testkit" "github.com/pingcap/tidb/util/testleak" "github.com/pingcap/tidb/util/testutil" + "github.com/pingcap/tidb/util/timeutil" + "github.com/pingcap/tipb/go-tipb" "github.com/pkg/errors" "golang.org/x/net/context" ) @@ -2357,6 +2360,33 @@ func (s *testContextOptionSuite) TestCoprocessorPriority(c *C) { cli.mu.Unlock() } +func (s *testSuite) TestTimezonePushDown(c *C) { + session.BootstrapSession(s.store) + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec("create table t (ts timestamp)") + defer tk.MustExec("drop table t") + tk.MustExec(`insert into t values ("2018-09-13 10:02:06")`) + + systemTZ := timeutil.Local() + c.Assert(systemTZ.String(), Not(Equals), "System") + ctx := context.Background() + count := 0 + ctx1 := context.WithValue(ctx, "CheckTimezonePushDownHook", func(req *kv.Request) { + count += 1 + dagReq := new(tipb.DAGRequest) + err := proto.Unmarshal(req.Data, dagReq) + c.Assert(err, IsNil) + c.Assert(dagReq.GetTimeZoneName(), Equals, systemTZ.String()) + }) + tk.Se.Execute(ctx1, `select * from t where ts = "2018-09-13 10:02:06"`) + + tk.MustExec(`set time_zone="System"`) + tk.Se.Execute(ctx1, `select * from t where ts = "2018-09-13 10:02:06"`) + + c.Assert(count, Equals, 2) // Make sure the hook function is called. +} + func (s *testSuite) TestNotFillCacheFlag(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") From d315b254b758e9a9065d7debd4d1d5c66b23d1e4 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 22:36:15 +0800 Subject: [PATCH 45/57] address comment --- util/timeutil/time.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index a0aa117e1af2a..3a2a6b47838c5 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -93,8 +93,7 @@ func InferSystemTZ() string { func inferTZNameFromFileName(path string) (string, error) { // phase1 only support read /etc/localtime which is a softlink to zoneinfo file substr := "zoneinfo" - if strings.Contains(path, substr) { - idx := strings.Index(path, substr) + if idx := strings.Index(path, substr); idx != -1 { return string(path[idx+len(substr)+1:]), nil } return "", errors.New(fmt.Sprintf("path %s is not supported", path)) From 5dc988be45210128ab8ed18600fa6d4ce0f50104 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 22:36:49 +0800 Subject: [PATCH 46/57] fix comment spelling issue --- 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 3a2a6b47838c5..3b06e153a8412 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -114,7 +114,7 @@ func SetSystemTZ(name string) { } // getLoc first trying to load location from a cache map. If nothing found in such map, then call -// `time.LocadLocation` to get a timezone location. After trying both way, an error wil be returned +// `time.LoadLocation` to get a timezone location. After trying both way, an error wil be returned // if valid Location is not found. func (lm *locCache) getLoc(name string) (*time.Location, error) { if name == "System" { From 1f99ca1eff4a82a531c36c0067a1090a597a7e3c Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 22:40:59 +0800 Subject: [PATCH 47/57] use one hook --- distsql/distsql.go | 4 ---- executor/executor_test.go | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/distsql/distsql.go b/distsql/distsql.go index 37567b35a80f8..b681fbc816bdf 100644 --- a/distsql/distsql.go +++ b/distsql/distsql.go @@ -36,10 +36,6 @@ func Select(ctx context.Context, sctx sessionctx.Context, kvReq *kv.Request, fie hook.(func(*kv.Request))(kvReq) } - if hook := ctx.Value("CheckTimezonePushDownHook"); hook != nil { - hook.(func(*kv.Request))(kvReq) - } - if !sctx.GetSessionVars().EnableStreaming { kvReq.Streaming = false } diff --git a/executor/executor_test.go b/executor/executor_test.go index ebf3ee9afc1f4..04ecb8625117d 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -2372,7 +2372,7 @@ func (s *testSuite) TestTimezonePushDown(c *C) { c.Assert(systemTZ.String(), Not(Equals), "System") ctx := context.Background() count := 0 - ctx1 := context.WithValue(ctx, "CheckTimezonePushDownHook", func(req *kv.Request) { + ctx1 := context.WithValue(ctx, "CheckSelectRequestHook", func(req *kv.Request) { count += 1 dagReq := new(tipb.DAGRequest) err := proto.Unmarshal(req.Data, dagReq) From e4418cfc7daff697f83aa96194fc0a6dba74924b Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 22:59:38 +0800 Subject: [PATCH 48/57] remove boot --- executor/executor_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index 04ecb8625117d..629e0a4367e02 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -2361,7 +2361,6 @@ func (s *testContextOptionSuite) TestCoprocessorPriority(c *C) { } func (s *testSuite) TestTimezonePushDown(c *C) { - session.BootstrapSession(s.store) tk := testkit.NewTestKit(c, s.store) tk.MustExec("use test") tk.MustExec("create table t (ts timestamp)") From 44996c28a3c13fc4bf2ef879789f899e6a8b2194 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Thu, 13 Sep 2018 23:57:38 +0800 Subject: [PATCH 49/57] add extra space --- session/bootstrap.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/session/bootstrap.go b/session/bootstrap.go index fa3712ac07aae..62986e734e58f 100644 --- a/session/bootstrap.go +++ b/session/bootstrap.go @@ -658,7 +658,7 @@ func upgradeToVer23(s Session) { doReentrantDDL(s, "ALTER TABLE mysql.stats_histograms ADD COLUMN `flag` bigint(64) NOT NULL DEFAULT 0", infoschema.ErrColumnExists) } -//writeSystemTZ writes system timezone info into mysql.tidb +// writeSystemTZ writes system timezone info into mysql.tidb func writeSystemTZ(s Session) { sql := fmt.Sprintf(`INSERT HIGH_PRIORITY INTO %s.%s VALUES ("%s", "%s", "TiDB Global System Timezone.") ON DUPLICATE KEY UPDATE VARIABLE_VALUE="%s"`, mysql.SystemDB, mysql.TiDBTable, tidbSystemTZ, timeutil.InferSystemTZ(), timeutil.InferSystemTZ()) From bd1693e606a92a66c79b9cb6feab39442273a6c1 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Fri, 14 Sep 2018 11:30:30 +0800 Subject: [PATCH 50/57] address comment --- session/session.go | 3 ++- util/timeutil/time.go | 6 ++---- util/timeutil/time_test.go | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/session/session.go b/session/session.go index 51502e0c6e6c5..adae5323ddb48 100644 --- a/session/session.go +++ b/session/session.go @@ -1072,13 +1072,14 @@ func CreateSession(store kv.Storage) (Session, error) { func loadSystemTZ(se *session) (string, error) { sql := `select variable_value from mysql.tidb where variable_name = "system_tz"` rss, errLoad := se.Execute(context.Background(), sql) + defer rss.Close() if errLoad != nil { return "", errLoad } chk := rss[0].NewChunk() + defer chk.Close() rss[0].Next(context.Background(), chk) return chk.GetRow(0).GetString(0), nil - } // BootstrapSession runs the first time when the TiDB server start. diff --git a/util/timeutil/time.go b/util/timeutil/time.go index 3b06e153a8412..7dce6f818fd2a 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -1,4 +1,4 @@ -// Copyright 2017 PingCAP, Inc. +// Copyright 2018 PingCAP, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -39,7 +39,7 @@ func init() { // locCa is a simple cache policy to improve the performance of 'time.LoadLocation'. var locCa *locCache -// LocalStr is current TiDB's system timezone name. +// systemTZ is current TiDB's system timezone name. var systemTZ string var zoneSources = []string{ "/usr/share/zoneinfo/", @@ -145,8 +145,6 @@ func LoadLocation(name string) (*time.Location, error) { // Zone returns the current timezone name and timezone offset in seconds. // In compatible with MySQL, we change `Local` to `System`. -// TODO: Golang team plan to return system timezone name intead of -// returning `Local` when `loc` is `time.Local`. We need keep an eye on this. func Zone(loc *time.Location) (string, int64) { _, offset := time.Now().In(loc).Zone() var name string diff --git a/util/timeutil/time_test.go b/util/timeutil/time_test.go index 23d52a9da3b36..17cbdf80f1c0c 100644 --- a/util/timeutil/time_test.go +++ b/util/timeutil/time_test.go @@ -1,4 +1,4 @@ -// Copyright 2018 The ql Authors. All rights reserved. +// Copyright 2018 PingCAP, Inc. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSES/QL-LICENSE file. From fda48134c121b4984233d3591e2776b3bd15abcc Mon Sep 17 00:00:00 2001 From: zhexuany Date: Fri, 14 Sep 2018 11:37:17 +0800 Subject: [PATCH 51/57] fix ci --- session/session.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/session/session.go b/session/session.go index adae5323ddb48..1ff3fadd07e63 100644 --- a/session/session.go +++ b/session/session.go @@ -1072,12 +1072,12 @@ func CreateSession(store kv.Storage) (Session, error) { func loadSystemTZ(se *session) (string, error) { sql := `select variable_value from mysql.tidb where variable_name = "system_tz"` rss, errLoad := se.Execute(context.Background(), sql) - defer rss.Close() + defer rss[0].Close() if errLoad != nil { return "", errLoad } chk := rss[0].NewChunk() - defer chk.Close() + defer chk.Reset() rss[0].Next(context.Background(), chk) return chk.GetRow(0).GetString(0), nil } From 1acd49189ccdfb4bdb848218d2096a5b2008e206 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Fri, 14 Sep 2018 13:59:42 +0800 Subject: [PATCH 52/57] move code after checking errLoad --- session/session.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/session/session.go b/session/session.go index 1ff3fadd07e63..759870232e427 100644 --- a/session/session.go +++ b/session/session.go @@ -1072,10 +1072,10 @@ func CreateSession(store kv.Storage) (Session, error) { func loadSystemTZ(se *session) (string, error) { sql := `select variable_value from mysql.tidb where variable_name = "system_tz"` rss, errLoad := se.Execute(context.Background(), sql) - defer rss[0].Close() if errLoad != nil { return "", errLoad } + defer rss[0].Close() chk := rss[0].NewChunk() defer chk.Reset() rss[0].Next(context.Background(), chk) From a37aeeb9c83a9cef2069236874ad95dad2981d3b Mon Sep 17 00:00:00 2001 From: zhexuany Date: Fri, 14 Sep 2018 14:02:11 +0800 Subject: [PATCH 53/57] add comment --- session/session.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/session/session.go b/session/session.go index 759870232e427..be3c7c0aa105c 100644 --- a/session/session.go +++ b/session/session.go @@ -1069,12 +1069,14 @@ func CreateSession(store kv.Storage) (Session, error) { return s, nil } +// loadSystemTZ loads systemTZ from mysql.tidb func loadSystemTZ(se *session) (string, error) { sql := `select variable_value from mysql.tidb where variable_name = "system_tz"` rss, errLoad := se.Execute(context.Background(), sql) if errLoad != nil { return "", errLoad } + // the record of mysql.tidb under where condition: variable_name = "system_tz" should shall only be one. defer rss[0].Close() chk := rss[0].NewChunk() defer chk.Reset() From e20dc07755f68cebb3f19f8477d97dc5ff5ed99b Mon Sep 17 00:00:00 2001 From: zhexuany Date: Fri, 14 Sep 2018 14:23:46 +0800 Subject: [PATCH 54/57] remove reset chk --- session/session.go | 1 - 1 file changed, 1 deletion(-) diff --git a/session/session.go b/session/session.go index be3c7c0aa105c..6ced52fe474cc 100644 --- a/session/session.go +++ b/session/session.go @@ -1079,7 +1079,6 @@ func loadSystemTZ(se *session) (string, error) { // the record of mysql.tidb under where condition: variable_name = "system_tz" should shall only be one. defer rss[0].Close() chk := rss[0].NewChunk() - defer chk.Reset() rss[0].Next(context.Background(), chk) return chk.GetRow(0).GetString(0), nil } From 74d394a9177f2b1ab3882759cc8a0102de736a0a Mon Sep 17 00:00:00 2001 From: zhexuany Date: Fri, 14 Sep 2018 16:31:19 +0800 Subject: [PATCH 55/57] rename local fn --- util/timeutil/time.go | 8 ++++---- util/timeutil/time_test.go | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/util/timeutil/time.go b/util/timeutil/time.go index 7dce6f818fd2a..4c00eaf601646 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -99,8 +99,8 @@ func inferTZNameFromFileName(path string) (string, error) { return "", errors.New(fmt.Sprintf("path %s is not supported", path)) } -// Local returns time.Local's IANA timezone name. It is TiDB's global timezone name. -func Local() *time.Location { +// SystemLocation returns time.SystemLocation's IANA timezone location. It is TiDB's global timezone location. +func SystemLocation() *time.Location { loc, err := LoadLocation(systemTZ) if err != nil { return time.Local @@ -144,12 +144,12 @@ func LoadLocation(name string) (*time.Location, error) { } // Zone returns the current timezone name and timezone offset in seconds. -// In compatible with MySQL, we change `Local` to `System`. +// In compatible with MySQL, we change `SystemLocation` to `System`. func Zone(loc *time.Location) (string, int64) { _, offset := time.Now().In(loc).Zone() var name string name = loc.String() - // when we found name is "Local", we have no chice but push down + // when we found name is "SystemLocation", we have no chice 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 17cbdf80f1c0c..e23e42f73503d 100644 --- a/util/timeutil/time_test.go +++ b/util/timeutil/time_test.go @@ -41,20 +41,20 @@ func (s *testTimeSuite) TestgetTZNameFromFileName(c *C) { func (s *testTimeSuite) TestLocal(c *C) { os.Setenv("TZ", "Asia/Shanghai") systemTZ = InferSystemTZ() - loc := Local() + loc := SystemLocation() c.Assert(systemTZ, Equals, "Asia/Shanghai") c.Assert(loc.String(), Equals, "Asia/Shanghai") os.Setenv("TZ", "UTC") // reset systemTZ systemTZ = InferSystemTZ() - loc = Local() + loc = SystemLocation() c.Assert(loc.String(), Equals, "UTC") os.Setenv("TZ", "") // reset systemTZ systemTZ = InferSystemTZ() - loc = Local() + loc = SystemLocation() c.Assert(loc.String(), Equals, "UTC") os.Unsetenv("TZ") } From 64a5b2e994e642537a63d22a56d2faab562cb483 Mon Sep 17 00:00:00 2001 From: zhexuany Date: Fri, 14 Sep 2018 16:42:16 +0800 Subject: [PATCH 56/57] fix ci --- executor/executor_test.go | 2 +- expression/builtin_time_test.go | 2 +- sessionctx/variable/session.go | 2 +- sessionctx/variable/varsutil.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index 629e0a4367e02..48e5f7935493d 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -2367,7 +2367,7 @@ func (s *testSuite) TestTimezonePushDown(c *C) { defer tk.MustExec("drop table t") tk.MustExec(`insert into t values ("2018-09-13 10:02:06")`) - systemTZ := timeutil.Local() + systemTZ := timeutil.SystemLocation() c.Assert(systemTZ.String(), Not(Equals), "System") ctx := context.Background() count := 0 diff --git a/expression/builtin_time_test.go b/expression/builtin_time_test.go index d3dd1e80c591b..f2a46abb302eb 100644 --- a/expression/builtin_time_test.go +++ b/expression/builtin_time_test.go @@ -998,7 +998,7 @@ func (s *testEvaluatorSuite) TestSysDate(c *C) { fc := funcs[ast.Sysdate] ctx := mock.NewContext() - ctx.GetSessionVars().StmtCtx.TimeZone = timeutil.Local() + ctx.GetSessionVars().StmtCtx.TimeZone = timeutil.SystemLocation() timezones := []types.Datum{types.NewDatum(1234), types.NewDatum(0)} for _, timezone := range timezones { // sysdate() result is not affected by "timestamp" session variable. diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 28b3d0b1aa5c5..b7896929d2484 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -427,7 +427,7 @@ func (s *SessionVars) GetNextPreparedStmtID() uint32 { func (s *SessionVars) Location() *time.Location { loc := s.TimeZone if loc == nil { - loc = timeutil.Local() + loc = timeutil.SystemLocation() } return loc } diff --git a/sessionctx/variable/varsutil.go b/sessionctx/variable/varsutil.go index 805a9e0d91ecb..2cc8a1cf3b682 100644 --- a/sessionctx/variable/varsutil.go +++ b/sessionctx/variable/varsutil.go @@ -351,7 +351,7 @@ func tidbOptInt64(opt string, defaultVal int64) int64 { func parseTimeZone(s string) (*time.Location, error) { if strings.EqualFold(s, "SYSTEM") { - return timeutil.Local(), nil + return timeutil.SystemLocation(), nil } loc, err := time.LoadLocation(s) From e77ee43ab085540ac06b7533c05a8e09da6af69d Mon Sep 17 00:00:00 2001 From: zhexuany Date: Fri, 14 Sep 2018 17:48:20 +0800 Subject: [PATCH 57/57] fix ci --- 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 4c00eaf601646..4aa212e232172 100644 --- a/util/timeutil/time.go +++ b/util/timeutil/time.go @@ -114,7 +114,7 @@ func SetSystemTZ(name string) { } // getLoc first trying to load location from a cache map. If nothing found in such map, then call -// `time.LoadLocation` to get a timezone location. After trying both way, an error wil be returned +// `time.LoadLocation` to get a timezone location. After trying both way, an error will be returned // if valid Location is not found. func (lm *locCache) getLoc(name string) (*time.Location, error) { if name == "System" {