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

ddl: concurrent safe api for using rule bundles cache #22035

Merged
merged 12 commits into from
Jan 13, 2021
Merged

ddl: concurrent safe api for using rule bundles cache #22035

merged 12 commits into from
Jan 13, 2021

Conversation

xhebox
Copy link
Contributor

@xhebox xhebox commented Dec 25, 2020

What problem does this PR solve?

Issue Number: close #21913

Problem Summary: Introduce a new concurrent-safe(by RWMutex) api for rule cache framework. Adapt all the tests that is using the old API, and code that relies on the internal map rather than using API.

Check List

Tests

  • Unit test
  • Integration test

Release note

  • No release note

@xhebox xhebox requested a review from a team as a code owner December 25, 2020 09:54
@xhebox xhebox requested review from wshwsh12 and removed request for a team December 25, 2020 09:54
@xhebox
ddl: introduce new rule cache api
fee3e61 
Signed-off-by: xhe <xw897002528@gmail.com>
@xhebox
ddl: also wrap around the internal
0264ca0 
Signed-off-by: xhe <xw897002528@gmail.com>
@xhebox
ddl: fix fmt
e5ff2ba 
Signed-off-by: xhe <xw897002528@gmail.com>
@xhebox
ddl: fix
8da6d9f 
Signed-off-by: xhe <xw897002528@gmail.com>
@xhebox
ddl: fix errcheck
d460368 
Signed-off-by: xhe <xw897002528@gmail.com>
@xhebox
ddl: fix merge
91e8c81 
Signed-off-by: xhe <xw897002528@gmail.com>
@xhebox
ddl: address comment
e9d84b1 
Signed-off-by: xhe <xw897002528@gmail.com>
@xhebox
ddl: fix rebase conflicts
ddaaf0b 
Signed-off-by: xhe <xw897002528@gmail.com>
djshow832
djshow832 reviewed Dec 31, 2020
View reviewed changes
infoschema/infoschema.go Outdated Show resolved Hide resolved
infoschema/infoschema.go Show resolved Hide resolved
@xhebox
ddl: reword
42bed6b 
Signed-off-by: xhe <xw897002528@gmail.com>
@wshwsh12 wshwsh12 removed their request for review January 5, 2021 10:06
@github-actions github-actions bot added the sig/sql-infra SIG: SQL Infra label Jan 7, 2021
@xhebox
ddl: cover Mock
09c1cbe 
Signed-off-by: xhe <xw897002528@gmail.com>
djshow832
djshow832 reviewed Jan 11, 2021
View reviewed changes
Copy link
Contributor

@djshow832 djshow832 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot
Copy link
Contributor

@djshow832, Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments. See the corresponding SIG page for more information. Related SIG: execution(slack).

@djshow832 djshow832 removed sig/DDL sig/execution SIG execution sig/sql-infra SIG: SQL Infra labels Jan 11, 2021
@djshow832
Copy link
Contributor

LGTM

@ti-srebot
Copy link
Contributor

@djshow832, Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments. See the corresponding SIG page for more information. Related SIG: execution(slack).

@djshow832 djshow832 added the sig/sql-infra SIG: SQL Infra label Jan 11, 2021
@djshow832
Copy link
Contributor

LGTM

@ti-srebot
Copy link
Contributor

@djshow832, Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments. See the corresponding SIG page for more information. Related SIG: execution(slack).

@github-actions github-actions bot added the sig/execution SIG execution label Jan 11, 2021
@djshow832
Copy link
Contributor

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 11, 2021
@xhebox
Copy link
Contributor Author

xhebox commented Jan 11, 2021

/unlabel sig/execution

@ti-srebot ti-srebot removed the sig/execution SIG execution label Jan 11, 2021
@github-actions github-actions bot added the sig/execution SIG execution label Jan 11, 2021
AilinKid
AilinKid reviewed Jan 13, 2021
View reviewed changes
Copy link
Contributor

@AilinKid AilinKid left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Jan 13, 2021
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Jan 13, 2021
@AilinKid
Copy link
Contributor

/run-all-tests

@djshow832
Copy link
Contributor

/run-unit-test

@xhebox
Copy link
Contributor Author

xhebox commented Jan 13, 2021

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 13, 2021
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 289bc7e into pingcap:master Jan 13, 2021
@xhebox xhebox deleted the lock branch January 13, 2021 05:06
@xhebox xhebox mentioned this pull request Jul 7, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AilinKid AilinKid AilinKid left review comments

@djshow832 djshow832 djshow832 left review comments

@ti-srebot ti-srebot ti-srebot approved these changes

Assignees
No one assigned
Labels
component/expression sig/execution SIG execution sig/sql-infra SIG: SQL Infra 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.

DATA RACE:github.com/pingcap/tidb/infoschema.(*infoSchema).RuleBundles() failed
4 participants
@xhebox @ti-srebot @djshow832 @AilinKid