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

storage/lock_manager: Add metrics to the new lock waiting queue #13560

Merged
merged 25 commits into from Sep 30, 2022
Merged

storage/lock_manager: Add metrics to the new lock waiting queue #13560

merged 25 commits into from Sep 30, 2022

Conversation

MyonKeminta
Copy link
Contributor

What is changed and how it works?

Issue Number: Ref #13298

What's Changed:

Add metrics to the new lock waiting queue, including:

* The number of keys on which there is lock waiting
* The number of requests that are waiting in the queue
* The histogram of the queue length observed when enqueueing new requests

Related changes

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

Check List

Tests

  • Manual test (WIP)

Side effects

  • Performance regression
    • Consumes more CPU

Release note 

None

MyonKeminta and others added 23 commits September 19, 2022 14:58
@MyonKeminta
Add lock wait queue and lock wait context; refactor storage::lock_man…
af57580 
…ager into a directory

Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Remove commented code; remove debug level change in Cargo.toml
e810deb 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Fix lint
38be4b7 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
fix lint
bdb117d 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Add document to explain how legacy_wake_up_cnt works
1685e9d 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Remove the comparator wrapper
4dce9c1 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Move SharedError to storage/error.rs
44d3eea 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Remove Clone on PessimisticLockParameters which is not used yet
e18bf41 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Fix lint
a624ee9 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Fix lint
821d414 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Address comments
a23cab9 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Add metrics for the new lock waiting queue
51037df 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Address comments
eae8902 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Address comments
8826a30 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@ti-chi-bot
Merge branch 'master' into m/new-lock-waiting-queue-part
7338f53
@ti-chi-bot
Merge branch 'master' into m/new-lock-waiting-queue-part
9f25907
@ti-chi-bot
Merge branch 'master' into m/new-lock-waiting-queue-part
9e79ab2
@ti-chi-bot
Merge branch 'master' into m/new-lock-waiting-queue-part
cb6f1ca
@MyonKeminta
Set write conflict reason
8e1fb44 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@MyonKeminta
Merge branch 'm/new-lock-waiting-queue-part' of https://github.com/My…
439fbc8 
…onKeminta/tikv into m/new-lock-waiting-queue-part
@MyonKeminta
Merge branch 'm/new-lock-waiting-queue-part' into m/new-lock-waiting-…
9d3b76f 
…queue-metrics
@MyonKeminta
Merge commit '9321040' into m/new-lock-waiting-queue-metrics
4306dac
@MyonKeminta
Fix build
3f77213 
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Sep 29, 2022 •
edited

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • ekexium
  • sticnarf

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.

@MyonKeminta MyonKeminta mentioned this pull request Sep 29, 2022
Open
30 tasks
@MyonKeminta MyonKeminta marked this pull request as ready for review September 29, 2022 09:57
TonsnakeLin
TonsnakeLin reviewed Sep 29, 2022
View reviewed changes
src/storage/metrics.rs Outdated Show resolved Hide resolved
@MyonKeminta @TonsnakeLin
Update src/storage/metrics.rs
5c6154c 
Co-authored-by: TonsnakeLin <87681388+TonsnakeLin@users.noreply.github.com>
Signed-off-by: MyonKeminta <9948422+MyonKeminta@users.noreply.github.com>
@ti-chi-bot ti-chi-bot added the status/LGT1 Status: PR - There is already 1 approval label Sep 30, 2022
@ti-chi-bot ti-chi-bot added status/LGT2 Status: PR - There are already 2 approvals and removed status/LGT1 Status: PR - There is already 1 approval labels Sep 30, 2022
@ekexium
Copy link
Contributor

ekexium commented Sep 30, 2022

Shall we benchmark it?

@cfzjywxk
Copy link
Collaborator

Shall we benchmark it?

We could do the benchmark in the enabling PR later.

@cfzjywxk
Copy link
Collaborator

/merge

@ti-chi-bot
Copy link
Member

@cfzjywxk: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and 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.

If you have any questions about the PR merge process, please refer to pr process.

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.

@ti-chi-bot
Copy link
Member

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

Commit hash: 5c6154c

@ti-chi-bot ti-chi-bot added the status/can-merge Status: Can merge to base branch label Sep 30, 2022
@ti-chi-bot
Copy link
Member

@MyonKeminta: 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.

@ti-chi-bot ti-chi-bot merged commit 47d8c9e into tikv:master Sep 30, 2022
@ti-chi-bot ti-chi-bot added this to the Pool milestone Sep 30, 2022
@MyonKeminta MyonKeminta mentioned this pull request Oct 11, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TonsnakeLin TonsnakeLin TonsnakeLin left review comments

@sticnarf sticnarf sticnarf approved these changes

@ekexium ekexium ekexium approved these changes

Assignees
No one assigned
Labels
release-note-none size/M status/can-merge Status: Can merge to base branch status/LGT2 Status: PR - There are already 2 approvals
Projects
None yet
Milestone
Pool
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@MyonKeminta @ti-chi-bot @ekexium @cfzjywxk @sticnarf @TonsnakeLin