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

executor: add SHUTDOWN command and privilege check #12804

Merged
merged 23 commits into from
Oct 21, 2019
Merged

executor: add SHUTDOWN command and privilege check #12804

merged 23 commits into from
Oct 21, 2019

Conversation

ekalinin
Copy link
Contributor

@ekalinin ekalinin commented Oct 17, 2019 •
edited
Loading

What problem does this PR solve?

Add:

  • SHUTDOWN command implementation
  • SHUTDOWN privilege check
  • Migrations for mysql.user table

Fix for #5046 (+ see pingcap/parser#580)

What is changed and how it works?

TiDB now is able to handle shutdown command. Also added privilege check for this command.

When shutdown comes to server it sends to itself SIGTERM signal.
SIGTERM signal causes graceful shutdown of the server.

At the moment ekalinin/parser is used to be able to run tests.
Will revert go.mod / go.sum when pingcap/parser#580 will be merged.

Check List

Tests

  • Unit test
  • Integration test

@sre-bot sre-bot added the contribution This PR is from a community contributor. label Oct 17, 2019
@ekalinin ekalinin changed the title add SHUTDOWN command and privilege add SHUTDOWN command and privilege check Oct 17, 2019
@ekalinin ekalinin changed the title add SHUTDOWN command and privilege check executor: add SHUTDOWN command and privilege check Oct 17, 2019
@codecov
Copy link

codecov bot commented Oct 17, 2019 •
edited
Loading

Codecov Report

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

@@            Coverage Diff            @@
##            master    #12804   +/-   ##
=========================================
  Coverage   80.154%   80.154%           
=========================================
  Files          465       465           
  Lines       107609    107609           
=========================================
  Hits         86253     86253           
  Misses       14913     14913           
  Partials      6443      6443

@zyxbest
Copy link
Contributor

zyxbest commented Oct 18, 2019

/run-all-tests

@zz-jason zz-jason added the sig/execution SIG execution label Oct 19, 2019
@tangenta
Copy link
Contributor

/run-all-tests

tangenta
tangenta reviewed Oct 19, 2019
View reviewed changes
Copy link
Contributor

@tangenta tangenta 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 the status/LGT1 Indicates that a PR has LGTM 1. label Oct 19, 2019
SunRunAway
SunRunAway reviewed Oct 21, 2019
View reviewed changes
executor/simple.go Outdated Show resolved Hide resolved
SunRunAway
SunRunAway reviewed Oct 21, 2019
View reviewed changes
Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

LGTM

@SunRunAway SunRunAway added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 21, 2019
@SunRunAway
Copy link
Contributor

Thank you @ekalinin
Please fix the CI build problem.

@ekalinin
Copy link
Contributor Author

/run-all-tests

@ekalinin
Copy link
Contributor Author

Fixed all failed tests related to pingcap/parser#581

kennytm
kennytm reviewed Oct 21, 2019
View reviewed changes
Copy link
Contributor

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

LGTM

@kennytm kennytm added status/LGT1 Indicates that a PR has LGTM 1. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Oct 21, 2019
SunRunAway
SunRunAway approved these changes Oct 21, 2019
View reviewed changes
Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

LGTM

@SunRunAway SunRunAway added status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. labels Oct 21, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Oct 21, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Oct 21, 2019

@ekalinin merge failed.

@zz-jason zz-jason removed the status/LGT1 Indicates that a PR has LGTM 1. label Oct 21, 2019
@ngaut ngaut merged commit 5bf03c8 into pingcap:master Oct 21, 2019
@ekalinin ekalinin deleted the 5046/shutdown-support branch October 21, 2019 14:12
@lysu
Copy link
Contributor

lysu commented Dec 3, 2019

/run-cherry-picker

@sre-bot
Copy link
Contributor

sre-bot commented Dec 3, 2019

cherry pick to release-3.0 failed

@lysu lysu mentioned this pull request Dec 18, 2019
Merged
sre-bot pushed a commit that referenced this pull request Dec 19, 2019
@lysu @sre-bot
executor: add SHUTDOWN command and privilege check (#12804) (#14104)
f2ae63e
XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019
@ekalinin @XiaTianliang
executor: add SHUTDOWN command and privilege check (pingcap#12804)
8c6b39e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kennytm kennytm kennytm left review comments

@SunRunAway SunRunAway SunRunAway approved these changes

@XuHuaiyu XuHuaiyu Awaiting requested review from XuHuaiyu

@tangenta tangenta Awaiting requested review from tangenta

Assignees
No one assigned
Labels
contribution This PR is from a community contributor. sig/execution SIG execution status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@ekalinin @zyxbest @tangenta @SunRunAway @sre-bot @lysu @kennytm @ngaut @zz-jason