-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
*: tiny refactor code to reduce txn conflict on 'table_cache_meta' #32387
Conversation
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
/run-unit-test |
Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/5fcb24b1e353badad2cc17b94db3352f97963612 |
Ref #25293 |
Please make sure this is expected: Previously each domain can have at most 10 goroutines for renewing lease. After this PR, each cached table in infoSchema can spawn a new goroutine to renew lease. |
This is by design. |
sync.RWMutex | ||
lockingForRead bool | ||
} | ||
lockingForRead tokenLimit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to be a semaphore, why not just use semaphore
in go lib?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with semaphore
, the channel is more simple for me here.
/merge |
This pull request has been accepted and is ready to merge. Commit hash: bb591be
|
@tiancaiamao: Your PR was out of date, I have automatically updated it for you. At the same time I will also trigger all tests for you: /run-all-tests If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository. |
/run-unit-test |
/run-mysql-test |
What problem does this PR solve?
Issue Number: close #32386
Problem Summary:
The warning log is caused by the fact that "select ... for update" can't retry when using optimistic transaction.
What is changed and how it works?
Tiny code refactor. Two changes mainly:
There is no need to worry about write skew abnormity here, we can just use the default RR (SI)
In the past, I make a goroutine along with a channel for the renew lease operation,
i.e. send the renew operation (func) teller the worker to execute it.
And this pattern to avoid duplicated message: https://github.com/pingcap/tidb/pull/31475/files
This is complex than it should be.
To avoid duplicated message, a chan(1) can be used as a token limiter, and spawn a new goroutine is handy:
Check List
Tests
Tiny refactor
Side effects
Documentation
Release note