Skip to content
New issue

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

Sign up for GitHub

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

Jump to bottom

server: set system_time_zone from system_tz #13934

Merged
merged 2 commits into from
Dec 9, 2019
Merged

server: set system_time_zone from system_tz #13934

merged 2 commits into from
Dec 9, 2019

Conversation

bb7133
Copy link
Member

@bb7133 bb7133 commented Dec 5, 2019

What problem does this PR solve?

This PR tries to close #13894, by setting system variable system_time_zone from system_tz variable in mysql.tidb table.

What is changed and how it works?

A setSystemTimeZoneVariable() is added to server.NewServer(), which get the value of systemTZ from exported function GetSystemTZ()

Please notice that this PR doesn't change any other behavior: all time-related calculations are not affected.

Check List

Tests

  • Integration test
  • Manual test (add detailed scripts or steps below)
export TZ="Europe/London"
# Before the initial bootstrap of TiDB Server)

./bin/tidb-server
# TiDB bootstrapped and infer `systemTZ` from TZ environment variable.

tidb> select variable_value from mysql.tidb where variable_name = "system_tz"; 
+----------------+
| VARIABLE_VALUE |
+----------------+
| Europe/London  |
+----------------+
1 row in set (0.00 sec)

tidb> select @@system_time_zone;                                                  
+--------------------+
| @@system_time_zone |
+--------------------+
| Europe/London      |
+--------------------+
1 row in set (0.00 sec)

The scripts described in #13894 is also used for manual tests.

Code changes

  • Has exported function/method change

Side effects

  • NA

Related changes

  • Need to update the documentation

Release note

  • NA

@bb7133 bb7133 added type/bugfix This PR fixes a bug. type/enhancement The issue or PR belongs to an enhancement. component/server and removed type/bugfix This PR fixes a bug. labels Dec 5, 2019
@bb7133 bb7133 force-pushed the bb7133/fix_server_time_zone branch from 4e92705 to b7ca0cc Compare December 5, 2019 13:45
@codecov
Copy link

codecov bot commented Dec 5, 2019 •
edited
Loading

Codecov Report

Merging #13934 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #13934   +/-   ##
===========================================
  Coverage   80.2315%   80.2315%           
===========================================
  Files           480        480           
  Lines        120748     120748           
===========================================
  Hits          96878      96878           
  Misses        16172      16172           
  Partials       7698       7698

@bb7133
Copy link
Member Author

bb7133 commented Dec 5, 2019

PTAL @lysu @zhexuany

@bb7133
Copy link
Member Author

bb7133 commented Dec 5, 2019

/run-all-tests

1 similar comment
@Deardrops
Copy link
Contributor

/run-all-tests

@bb7133
Copy link
Member Author

bb7133 commented Dec 6, 2019

/run-common-test /run-integration-common-test

@Deardrops
Copy link
Contributor

LGTM

@Deardrops Deardrops added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 6, 2019
@bb7133 bb7133 force-pushed the bb7133/fix_server_time_zone branch from b7ca0cc to aae1742 Compare December 6, 2019 02:47
@bb7133 bb7133 requested review from lysu and zhexuany and removed request for lysu December 6, 2019 03:14
AilinKid
AilinKid reviewed Dec 9, 2019
View reviewed changes
Copy link
Contributor

@AilinKid AilinKid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rest LGTM

server/tidb_test.go
defer server.Close()

tz1 := tk.MustQuery("select variable_value from mysql.tidb where variable_name = 'system_tz'").Rows()
tk.MustQuery("select @@system_time_zone").Check(tz1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need test check equal to TZ from host?

wjhuang2016
wjhuang2016 reviewed Dec 9, 2019
View reviewed changes
Copy link
Member

@wjhuang2016 wjhuang2016 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@tangenta tangenta added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 9, 2019
lysu
lysu reviewed Dec 9, 2019
View reviewed changes
Copy link
Contributor

@lysu lysu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ngaut ngaut merged commit 41c7d7e into pingcap:master Dec 9, 2019
@bb7133 bb7133 mentioned this pull request Dec 17, 2019
Merged
bb7133 added a commit to bb7133/tidb that referenced this pull request Dec 17, 2019
@bb7133
server: set system_time_zone from systemTZ (pingcap#13934)
35771be
sre-bot pushed a commit that referenced this pull request Dec 17, 2019
@bb7133 @sre-bot
server: set system_time_zone from systemTZ (#13934) (#14086)
9a103ef
XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019
@bb7133 @XiaTianliang
server: set system_time_zone from systemTZ (pingcap#13934)
9c59541
@bb7133 bb7133 mentioned this pull request Jan 10, 2020
Open
@bb7133 bb7133 deleted the bb7133/fix_server_time_zone branch December 29, 2023 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lysu lysu lysu left review comments

@AilinKid AilinKid AilinKid left review comments

@wjhuang2016 wjhuang2016 wjhuang2016 left review comments

@ngaut ngaut ngaut approved these changes

@zhexuany zhexuany Awaiting requested review from zhexuany

Assignees
No one assigned
Labels
component/server status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

server: fixed system_time_zone value in TiDB
7 participants
@bb7133 @Deardrops @lysu @ngaut @AilinKid @wjhuang2016 @tangenta