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

privilege: add restricted read only #25340

Merged
merged 18 commits into from
Jul 16, 2021
Merged

privilege: add restricted read only #25340

merged 18 commits into from
Jul 16, 2021

Conversation

ichn-hu
Copy link
Contributor

@ichn-hu ichn-hu commented Jun 10, 2021 •
edited
Loading

What problem does this PR solve?

Issue Number: close #xxx

Problem Summary:

What is changed and how it works?

Proposal: xxx

What's Changed:

How it Works:

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch

Check List

Tests

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

Side effects

  • Performance regression
    • Consumes more CPU
    • Consumes more MEM
  • Breaking backward compatibility

Release note

  • Support TIDB_RESTRICTED_READ_ONLY global variable.

@ichn-hu ichn-hu requested a review from a team as a code owner June 10, 2021 10:50
@ichn-hu ichn-hu requested review from eurekaka and removed request for a team June 10, 2021 10:50
@ti-chi-bot ti-chi-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 10, 2021
@ti-srebot
Copy link
Contributor

No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md

@github-actions github-actions bot added sig/planner SIG: Planner sig/sql-infra SIG: SQL Infra labels Jun 10, 2021
@ti-srebot
Copy link
Contributor

No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md

@ti-srebot
Copy link
Contributor

No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md

@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 15, 2021
@eurekaka eurekaka removed their request for review June 21, 2021 07:59
@ti-srebot
Copy link
Contributor

No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md

@ti-srebot
Copy link
Contributor

No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md

@ichn-hu ichn-hu force-pushed the restricted-read-only branch 2 times, most recently from 10b3cde to 7f1b45c Compare June 23, 2021 08:25
@ichn-hu ichn-hu changed the title privilege: add restricted read only [WIP] privilege: add restricted read only Jun 23, 2021
@ichn-hu ichn-hu force-pushed the restricted-read-only branch 3 times, most recently from 428880c to c0b0dca Compare June 28, 2021 08:25
@ichn-hu ichn-hu requested a review from a team as a code owner June 28, 2021 08:25
@ichn-hu ichn-hu requested review from XuHuaiyu and removed request for a team June 28, 2021 08:25
@github-actions github-actions bot added the sig/execution SIG execution label Jun 28, 2021
@ichn-hu
Copy link
Contributor Author

ichn-hu commented Jun 28, 2021

/cc @morgo

@ti-chi-bot ti-chi-bot requested a review from morgo June 28, 2021 09:05
morgo
morgo approved these changes Jul 15, 2021
View reviewed changes
Copy link
Contributor

@morgo morgo left a comment

Choose a reason for hiding this comment

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

LGTM, sorry for the delay :-)

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • AilinKid
  • morgo

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 15, 2021
@ti-chi-bot ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 16, 2021
@ichn-hu
Copy link
Contributor Author

ichn-hu commented Jul 16, 2021

/merge

1 similar comment
@ichn-hu
Copy link
Contributor Author

ichn-hu commented Jul 16, 2021

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 3ab22e9

1 similar comment
@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 3ab22e9

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 16, 2021
@ti-chi-bot ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Jul 16, 2021
@ichn-hu
Copy link
Contributor Author

ichn-hu commented Jul 16, 2021

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 9835eb5

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 16, 2021
@ti-chi-bot ti-chi-bot merged commit 4f26765 into pingcap:master Jul 16, 2021
@tisonkun
Copy link
Contributor

@ichn-hu do we have a tracking issue for this effort? Or this is the whole task.

@ichn-hu
Copy link
Contributor Author

ichn-hu commented Jul 21, 2021

@ichn-hu do we have a tracking issue for this effort? Or this is the whole task.

this is the whole task done atomically :)

@tisonkun
Copy link
Contributor

tisonkun commented Jul 21, 2021 •
edited
Loading

@ichn-hu OK. For development task, we may always create an issue for it.

And for design and implementation, I'd suggest separate in two PRs.

You may ask what if they run almost simultaneously. Well, always the design PR can go into discussion first, and it refers to the feature branch so that we discuss simultaneously. After the design merged, we create a PR from the feature branch to the master.

According to this post, @wjhuang2016 should have further ideas.

@ichn-hu ichn-hu mentioned this pull request Jul 22, 2021
Merged
14 tasks
This was referenced Jan 28, 2022
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@morgo morgo morgo approved these changes

@AilinKid AilinKid AilinKid approved these changes

Assignees
No one assigned
Labels
component/config sig/execution SIG execution sig/planner SIG: Planner sig/sql-infra SIG: SQL Infra size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. 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.
6 participants
@ichn-hu @ti-srebot @ti-chi-bot @tisonkun @morgo @AilinKid