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.

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

planner: do not filter access paths by isolation read for mysql.SystemDB #14309

Conversation

lzmhhh123
Copy link
Contributor

What problem does this PR solve?

After set global tidb_isolation_read_engines="tiflash", then restart tidb-server. The bootstrap can't finish because of some system DB's queries. It only can access TiKV.

What is changed and how it works?

Check the DB name whether is mysql.SystemDB before filter access paths by isolation read.

Check List

Tests

  • Unit test
  • Integration test

@lzmhhh123 lzmhhh123 added type/bugfix This PR fixes a bug. sig/planner SIG: Planner labels Jan 2, 2020
@lzmhhh123 lzmhhh123 requested a review from a team as a code owner January 2, 2020 03:46
@ghost ghost requested review from francis0407 and alivxxx and removed request for a team January 2, 2020 03:46
@@ -2536,7 +2536,9 @@ func (b *PlanBuilder) buildDataSource(ctx context.Context, tn *ast.TableName, as
if err != nil {
return nil, err
}
possiblePaths, err = b.filterPathByIsolationRead(possiblePaths)
if dbName.L != mysql.SystemDB {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we put the check into the function? In case that someone else called the function but forgot the check.

Copy link
Contributor

@alivxxx alivxxx left a comment

Choose a reason for hiding this comment

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

LGTM

@lzmhhh123 lzmhhh123 requested review from AstroProfundis and zz-jason and removed request for AstroProfundis January 6, 2020 05:46
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

@lzmhhh123 lzmhhh123 added the status/can-merge Indicates a PR has been approved by a committer. label Jan 6, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Jan 6, 2020

/run-all-tests

@sre-bot sre-bot merged commit a7e98ac into pingcap:master Jan 6, 2020
@lzmhhh123 lzmhhh123 deleted the bug-fix/do_not_filter_paths_by_isolation_for_system_db branch January 6, 2020 06:21
lidezhu pushed a commit to lidezhu/tidb that referenced this pull request Jan 7, 2020
lzmhhh123 added a commit to lzmhhh123/tidb that referenced this pull request Jan 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/planner SIG: Planner status/can-merge Indicates a PR has been approved by a committer. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants