-
Notifications
You must be signed in to change notification settings - Fork 5.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
*: write system timezone into mysql.tidb in bootstrap stage. #7638
Changes from 4 commits
ec6a822
40f1caf
ad574a4
e76c224
a74739c
f0502c8
ed136b9
085dc79
8ba623f
13c7c1b
e962f82
26f65f7
d708186
ceedc34
2adc1e8
acc0262
709c32c
350d27d
c95e6c3
21e9e22
a84c028
c1658d8
a517a43
b5e39b6
84b6e8f
dae8a5a
5c373bf
2d0aca2
e624443
f3ad3bc
692d4d5
b32bdd2
49454d5
318c12b
cb5fd7d
ec66759
a3c3cc4
3976eda
d522982
dd2d0c9
29679ab
733331e
e9bb73b
1d8ddfa
bd24bee
2f8018a
7d6ae04
d315b25
5dc988b
1f99ca1
e4418cf
44996c2
bd1693e
fda4813
9986492
1acd491
65051bb
a37aeeb
e20dc07
74d394a
64a5b2e
e77ee43
1b53dff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,8 +15,10 @@ package executor | |
|
||
import ( | ||
"math" | ||
"path/filepath" | ||
"runtime" | ||
"sort" | ||
"strings" | ||
"sync" | ||
"sync/atomic" | ||
"time" | ||
|
@@ -117,6 +119,18 @@ func closeAll(objs ...Closeable) error { | |
return errors.Trace(err) | ||
} | ||
|
||
// 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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is better to move this function to util/xxx package. |
||
// 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe this is not kind of a suffix? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be "as a prefix"? |
||
} | ||
|
||
// 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 +141,17 @@ func zone(sctx sessionctx.Context) (string, int64) { | |
var name string | ||
name = loc.String() | ||
if name == "Local" { | ||
name = "System" | ||
path, err := filepath.EvalSymlinks("/etc/localtime") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this function evaluated? If it is evaluated frequently, we need to cache the zone. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree. |
||
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) | ||
} | ||
|
||
return name, int64(offset) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ import ( | |
"bytes" | ||
"fmt" | ||
"io" | ||
"strings" | ||
"sync" | ||
"time" | ||
|
||
|
@@ -164,13 +165,26 @@ func (h *rpcHandler) buildDAGExecutor(req *coprocessor.Request) (*dagContext, ex | |
return ctx, e, dagReq, err | ||
} | ||
|
||
func getTZNameFromFileName(path string) (string, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is it implemented again here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems be duplicated to |
||
// 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) | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove this empty line |
||
return time.FixedZone("", offset), nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When will the code run to this branch and what happen then ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. user runs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But the time zone name is not "UTC" ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this case, the name should be "empty". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will TiKV handle that ? @breeswish |
||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/zoninfo/zoneinfo