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: let flush privileges do nothing when skip-grant-table is configured #10986

Merged
merged 5 commits into from
Jul 2, 2019
Merged

executor: let flush privileges do nothing when skip-grant-table is configured #10986

merged 5 commits into from
Jul 2, 2019

Conversation

tiancaiamao
Copy link
Contributor

What problem does this PR solve?

When skip-grant-table is enabled, privilege handle is not initialized, calling flush privileges
would meet nil pointer panic

mysql> flush privileges;
ERROR 2013 (HY000): Lost connection to MySQL server during query

github.com/pingcap/tidb/server.(*clientConn).Run.func1
	/root/go/src/github.com/pingcap/tidb/server/conn.go:593
runtime.gopanic
	/usr/local/go/src/runtime/panic.go:522
runtime.panicmem
	/usr/local/go/src/runtime/panic.go:82
runtime.sigpanic
	/usr/local/go/src/runtime/signal_unix.go:390
github.com/pingcap/tidb/privilege/privileges.(*Handle).Update
	/root/go/src/github.com/pingcap/tidb/privilege/privileges/cache.go:979
github.com/pingcap/tidb/executor.(*SimpleExec).executeFlush
	/root/go/src/github.com/pingcap/tidb/executor/simple.go:822
github.com/pingcap/tidb/executor.(*SimpleExec).Next
	/root/go/src/github.com/pingcap/tidb/executor/simple.go:74
github.com/pingcap/tidb/executor.Next
	/root/go/src/github.com/pingcap/tidb/executor/executor.go:190
github.com/pingcap/tidb/executor.(*ExecStmt).handleNoDelayExecutor
	/root/go/src/github.com/pingcap/tidb/executor/adapter.go:388
github.com/pingcap/tidb/executor.(*ExecStmt).Exec
	/root/go/src/github.com/pingcap/tidb/executor/adapter.go:265
github.com/pingcap/tidb/session.runStmt
	/root/go/src/github.com/pingcap/tidb/session/tidb.go:219
github.com/pingcap/tidb/session.(*session).executeStatement
	/root/go/src/github.com/pingcap/tidb/session/session.go:939
github.com/pingcap/tidb/session.(*session).execute
	/root/go/src/github.com/pingcap/tidb/session/session.go:1037
github.com/pingcap/tidb/session.(*session).Execute
	/root/go/src/github.com/pingcap/tidb/session/session.go:966
github.com/pingcap/tidb/session.execRestrictedSQL
	/root/go/src/github.com/pingcap/tidb/session/session.go:745
github.com/pingcap/tidb/session.(*session).ExecRestrictedSQL
	/root/go/src/github.com/pingcap/tidb/session/session.go:700
github.com/pingcap/tidb/domain.(*Domain).NotifyUpdatePrivilege
	/root/go/src/github.com/pingcap/tidb/domain/domain.go:1047
github.com/pingcap/tidb/executor.(*SimpleExec).executeSetPwd
	/root/go/src/github.com/pingcap/tidb/executor/simple.go:789
github.com/pingcap/tidb/executor.(*SimpleExec).Next
	/root/go/src/github.com/pingcap/tidb/executor/simple.go:88
github.com/pingcap/tidb/executor.Next
	/root/go/src/github.com/pingcap/tidb/executor/executor.go:190
github.com/pingcap/tidb/executor.(*ExecStmt).handleNoDelayExecutor
	/root/go/src/github.com/pingcap/tidb/executor/adapter.go:388
github.com/pingcap/tidb/executor.(*ExecStmt).Exec
	/root/go/src/github.com/pingcap/tidb/executor/adapter.go:265
github.com/pingcap/tidb/session.runStmt
	/root/go/src/github.com/pingcap/tidb/session/tidb.go:219
github.com/pingcap/tidb/session.(*session).executeStatement
	/root/go/src/github.com/pingcap/tidb/session/session.go:939
github.com/pingcap/tidb/session.(*session).execute
	/root/go/src/github.com/pingcap/tidb/session/session.go:1037
github.com/pingcap/tidb/session.(*session).Execute
	/root/go/src/github.com/pingcap/tidb/session/session.go:966
github.com/pingcap/tidb/server.(*TiDBContext).Execute
	/root/go/src/github.com/pingcap/tidb/server/driver_tidb.go:246
github.com/pingcap/tidb/server.(*clientConn).handleQuery
	/root/go/src/github.com/pingcap/tidb/server/conn.go:1158
github.com/pingcap/tidb/server.(*clientConn).dispatch
	/root/go/src/github.com/pingcap/tidb/server/conn.go:885
github.com/pingcap/tidb/server.(*clientConn).Run
	/root/go/src/github.com/pingcap/tidb/server/conn.go:644
github.com/pingcap/tidb/server.(*Server).onConn
	/root/go/src/github.com/pingcap/tidb/server/server.go:438

What is changed and how it works?

Make flush privileges return directly if skip-grant-table is enabled.

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)

Related changes

  • Need to cherry-pick to the release branch

@tiancaiamao
executor: let flush privileges do nothing when skip-grant-table is …
04576a0 
…configured

When skip-grant-table is enabled, privilege handle is not initialized, calling flush privileges
would meet nil pointer panic
@tiancaiamao
Copy link
Contributor Author

/run-all-tests

@codecov
Copy link

codecov bot commented Jun 28, 2019 •
edited
Loading

Codecov Report

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

@@             Coverage Diff             @@
##             master     #10986   +/-   ##
===========================================
  Coverage   81.0189%   81.0189%           
===========================================
  Files           418        418           
  Lines         89331      89331           
===========================================
  Hits          72375      72375           
  Misses        11730      11730           
  Partials       5226       5226

@tiancaiamao
Copy link
Contributor Author

PTAL @zz-jason @jackysp

jackysp
jackysp reviewed Jun 29, 2019
View reviewed changes
Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@jackysp jackysp added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 30, 2019
zz-jason
zz-jason approved these changes Jul 1, 2019
View reviewed changes
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 1, 2019
@zz-jason zz-jason added the priority/release-blocker This issue blocks a release. Please solve it ASAP. label Jul 1, 2019
@tiancaiamao
Copy link
Contributor Author

/run-check_dev_2

@tiancaiamao
Copy link
Contributor Author

/rebuild

@tiancaiamao tiancaiamao merged commit 5886bb9 into pingcap:master Jul 2, 2019
@tiancaiamao tiancaiamao deleted the skip-flush branch July 2, 2019 02:44
tiancaiamao added a commit to tiancaiamao/tidb that referenced this pull request Jul 2, 2019
@tiancaiamao
executor: let flush privileges do nothing when skip-grant-table is …
9235578 
…configured (pingcap#10986)

When skip-grant-table is enabled, privilege handle is not initialized, calling flush privileges
would meet nil pointer panic
@tiancaiamao tiancaiamao mentioned this pull request Jul 2, 2019
Merged
tiancaiamao added a commit to tiancaiamao/tidb that referenced this pull request Oct 18, 2019
@tiancaiamao
executor: let flush privileges do nothing when skip-grant-table is …
afb63b0 
…configured pingcap#10986
@tiancaiamao tiancaiamao mentioned this pull request Oct 18, 2019
Merged
sre-bot pushed a commit that referenced this pull request Oct 21, 2019
@tiancaiamao @sre-bot
executor: let flush privileges do nothing when skip-grant-table is …
2098cfe 
…configured #10986 (#12816)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jackysp jackysp jackysp left review comments

@zz-jason zz-jason zz-jason approved these changes

Assignees
No one assigned
Labels
component/privilege priority/release-blocker This issue blocks a release. Please solve it ASAP. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tiancaiamao @jackysp @zz-jason