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

config: add server level isolation read config #14440

Merged
merged 12 commits into from
Jan 19, 2020

Conversation

lzmhhh123
Copy link
Contributor

What problem does this PR solve?

For isolation read. We have the global and session variable tidb_isolation_read_engines to control. But we can't control specific tidb-server to set isolation read. So we need a server-level config to control the isolation.

What is changed and how it works?

When both the server-level isolation read and session-level isolation read exists. The access paths should be filtered by both of them.

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported variable/fields change
  • Has interface methods change
  • Has persistent data change

Related changes

  • Need to update the documentation

@lzmhhh123 lzmhhh123 requested a review from a team as a code owner January 10, 2020 08:26
@ghost ghost requested review from eurekaka and winoros and removed request for a team January 10, 2020 08:26
[isolation-read]
# engines means allow the tidb server read data from which types of engines. options: "tikv", "tiflash", "tidb".
engines = ["tikv", "tiflash", "tidb"]
# labels means allow the tidb server read data from which labels. Empty set means all labels are allowed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain more on the purpose of labels in the comment? or the use cases of the labels?

@lzmhhh123
Copy link
Contributor Author

@eurekaka @lamxTyler @winoros PTAL.

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

winoros
winoros previously approved these changes Jan 17, 2020
Copy link
Member

@winoros winoros 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
Copy link
Contributor Author

/rebuild

@lzmhhh123 lzmhhh123 force-pushed the dev/add_server_isolation_read branch from 0159d0e to 69199e9 Compare January 17, 2020 10:19
@lzmhhh123
Copy link
Contributor Author

/rebuild

@lzmhhh123
Copy link
Contributor Author

/run-unit-test

@alivxxx alivxxx added status/LGT2 Indicates that a PR has LGTM 2. status/can-merge Indicates a PR has been approved by a committer. labels Jan 19, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Jan 19, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Jan 19, 2020

@lzmhhh123 merge failed.

@lzmhhh123
Copy link
Contributor Author

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Jan 19, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Jan 19, 2020

@lzmhhh123 merge failed.

@lzmhhh123
Copy link
Contributor Author

/run-all-tests

@lzmhhh123
Copy link
Contributor Author

/rebuild

@lzmhhh123
Copy link
Contributor Author

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Jan 19, 2020

Your auto merge job has been accepted, waiting for 14530

@sre-bot
Copy link
Contributor

sre-bot commented Jan 19, 2020

/run-all-tests

@lzmhhh123
Copy link
Contributor Author

/run-unit-test

@sre-bot
Copy link
Contributor

sre-bot commented Jan 19, 2020

@lzmhhh123 merge failed.

@lzmhhh123 lzmhhh123 merged commit 0b8eea3 into pingcap:master Jan 19, 2020
@lzmhhh123 lzmhhh123 deleted the dev/add_server_isolation_read branch January 19, 2020 06:28
lzmhhh123 added a commit to lzmhhh123/tidb that referenced this pull request Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/config status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/new-feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants