raftstore: Avoid snapshot IO in raftstore thread #16682

Connor1996 · 2024-03-21T10:43:01Z

What is changed and how it works?

Issue Number: Close #16564

What's Changed:

Avoid snapshot IO in raftstore thread

This PR does:

Only check snapshot meta file existence when receiving snapshot message so that don't need to read meta file content
Don't calculate checksum when loading snapshot meta, because it would check in validate

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
Performance regression: Consumes more Memory
Breaking backward compatibility

Release note

Avoid snapshot IO in raftstore thread

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

ti-chi-bot · 2024-03-21T10:43:03Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

SpadeA-Tang
glorv

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.

LykxSassinator

Rest LGTM

LykxSassinator · 2024-03-21T11:03:59Z

components/raftstore/src/store/snap.rs

@@ -1710,6 +1700,20 @@ impl SnapManager {
        Ok(Box::new(s))
    }

+    pub fn check_snapshot_meta_file_exist(&self, key: &SnapKey) -> RaftStoreResult<()> {


Suggested change

pub fn check_snapshot_meta_file_exist(&self, key: &SnapKey) -> RaftStoreResult<()> {

pub fn meta_file_exists(&self, key: &SnapKey) -> RaftStoreResult<()> {

SpadeA-Tang

LGTM. There's one unresolved comment, though.

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

Connor1996 · 2024-03-22T07:15:56Z

/merge

ti-chi-bot · 2024-03-22T07:15:58Z

@Connor1996: 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 · 2024-03-22T07:16:00Z

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

Commit hash: 63908cc

ti-chi-bot · 2024-03-23T02:57:53Z

@Connor1996: Your PR was out of date, I have automatically updated it for you.

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.

components/raftstore/src/store/snap.rs

Connor1996 · 2024-03-25T04:02:31Z

/merge

ti-chi-bot · 2024-03-25T04:02:32Z

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

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

Connor1996 · 2024-03-25T08:06:22Z

/merge

ti-chi-bot · 2024-03-25T08:06:25Z

@Connor1996: 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 · 2024-03-25T08:06:27Z

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

Commit hash: ae877e1

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

Connor1996 · 2024-03-25T09:07:24Z

/merge

ti-chi-bot · 2024-03-25T09:07:26Z

@Connor1996: 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 · 2024-03-25T09:07:29Z

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

Commit hash: 3200f92

close tikv#16564 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-03-27T02:47:51Z

In response to a cherrypick label: new pull request created to branch release-6.5: #16702.

close #16564 Avoid snapshot IO in raftstore thread Signed-off-by: Connor <zbk602423539@gmail.com> Co-authored-by: Connor <zbk602423539@gmail.com>

close tikv#16564 Avoid snapshot IO in raftstore thread Signed-off-by: Connor1996 <zbk602423539@gmail.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

ti-chi-bot · 2024-04-15T06:17:38Z

In response to a cherrypick label: new pull request created to branch release-7.1: #16833.

close #16564 Avoid snapshot IO in raftstore thread Signed-off-by: Connor1996 <zbk602423539@gmail.com> Co-authored-by: Connor1996 <zbk602423539@gmail.com> Co-authored-by: tonyxuqqi <tonyxuqi@outlook.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

ti-chi-bot · 2024-05-07T05:49:34Z

In response to a cherrypick label: new pull request created to branch release-7.5: #16952.

Connor1996 added 2 commits March 21, 2024 18:35

check meta file only

0605c62

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

avoid scan

830164a

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

ti-chi-bot bot added do-not-merge/needs-triage-completed release-note size/S labels Mar 21, 2024

Connor1996 requested review from tonyxuqqi, SpadeA-Tang, LykxSassinator, glorv and overvenus March 21, 2024 10:43

ti-chi-bot bot removed the do-not-merge/needs-triage-completed label Mar 21, 2024

LykxSassinator reviewed Mar 21, 2024

View reviewed changes

cfzjywxk mentioned this pull request Mar 22, 2024

Mitigate the performance impact on tikv slow pingcap/tidb#51585

Open

glorv approved these changes Mar 22, 2024

View reviewed changes

ti-chi-bot bot added the status/LGT1 Status: PR - There is already 1 approval label Mar 22, 2024

SpadeA-Tang approved these changes Mar 22, 2024

View reviewed changes

ti-chi-bot bot added status/LGT2 Status: PR - There are already 2 approvals and removed status/LGT1 Status: PR - There is already 1 approval labels Mar 22, 2024

address comment

63908cc

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

ti-chi-bot bot added the status/can-merge Status: Can merge to base branch label Mar 22, 2024

ti-chi-bot bot added 2 commits March 23, 2024 02:45

Merge branch 'master' into snap-io

30a9769

Merge branch 'master' into snap-io

58738a4

LykxSassinator reviewed Mar 23, 2024

View reviewed changes

components/raftstore/src/store/snap.rs Outdated Show resolved Hide resolved

fix lint

ae877e1

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

Connor1996 force-pushed the snap-io branch from 9792f23 to ae877e1 Compare March 25, 2024 08:06

ti-chi-bot bot removed the status/can-merge Status: Can merge to base branch label Mar 25, 2024

ti-chi-bot bot added the status/can-merge Status: Can merge to base branch label Mar 25, 2024

change test

3200f92

Signed-off-by: Connor1996 <zbk602423539@gmail.com>

ti-chi-bot bot added size/M and removed status/can-merge Status: Can merge to base branch size/S labels Mar 25, 2024

ti-chi-bot bot added the status/can-merge Status: Can merge to base branch label Mar 25, 2024

ti-chi-bot bot merged commit 1c7b344 into tikv:master Mar 25, 2024
7 checks passed

ti-chi-bot bot added this to the Pool milestone Mar 25, 2024

ti-chi-bot added the needs-cherry-pick-release-6.5 label Mar 27, 2024

ti-chi-bot pushed a commit to ti-chi-bot/tikv that referenced this pull request Mar 27, 2024

This is an automated cherry-pick of tikv#16682

c8ffb3e

close tikv#16564 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Mar 27, 2024

raftstore: Avoid snapshot IO in raftstore thread (#16682) #16702

Merged

9 tasks

ti-chi-bot added the needs-cherry-pick-release-7.1 label Apr 15, 2024

ti-chi-bot mentioned this pull request Apr 15, 2024

raftstore: Avoid snapshot IO in raftstore thread (#16682) #16833

Merged

9 tasks

ti-chi-bot added the needs-cherry-pick-release-7.5 label May 7, 2024

ti-chi-bot mentioned this pull request May 7, 2024

raftstore: Avoid snapshot IO in raftstore thread (#16682) #16952

Open

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

raftstore: Avoid snapshot IO in raftstore thread #16682

raftstore: Avoid snapshot IO in raftstore thread #16682

Connor1996 commented Mar 21, 2024

ti-chi-bot bot commented Mar 21, 2024 •

edited

LykxSassinator left a comment

LykxSassinator Mar 21, 2024

SpadeA-Tang left a comment

Connor1996 commented Mar 22, 2024

ti-chi-bot bot commented Mar 22, 2024

ti-chi-bot bot commented Mar 22, 2024

ti-chi-bot bot commented Mar 23, 2024

Connor1996 commented Mar 25, 2024

ti-chi-bot bot commented Mar 25, 2024

Connor1996 commented Mar 25, 2024

ti-chi-bot bot commented Mar 25, 2024

ti-chi-bot bot commented Mar 25, 2024

Connor1996 commented Mar 25, 2024

ti-chi-bot bot commented Mar 25, 2024

ti-chi-bot bot commented Mar 25, 2024

ti-chi-bot commented Mar 27, 2024

ti-chi-bot commented Apr 15, 2024

ti-chi-bot commented May 7, 2024

	pub fn check_snapshot_meta_file_exist(&self, key: &SnapKey) -> RaftStoreResult<()> {
	pub fn meta_file_exists(&self, key: &SnapKey) -> RaftStoreResult<()> {

raftstore: Avoid snapshot IO in raftstore thread #16682

raftstore: Avoid snapshot IO in raftstore thread #16682

Conversation

Connor1996 commented Mar 21, 2024

What is changed and how it works?

Related changes

Check List

Release note

ti-chi-bot bot commented Mar 21, 2024 • edited

LykxSassinator left a comment

Choose a reason for hiding this comment

LykxSassinator Mar 21, 2024

Choose a reason for hiding this comment

SpadeA-Tang left a comment

Choose a reason for hiding this comment

Connor1996 commented Mar 22, 2024

ti-chi-bot bot commented Mar 22, 2024

ti-chi-bot bot commented Mar 22, 2024

ti-chi-bot bot commented Mar 23, 2024

Connor1996 commented Mar 25, 2024

ti-chi-bot bot commented Mar 25, 2024

Connor1996 commented Mar 25, 2024

ti-chi-bot bot commented Mar 25, 2024

ti-chi-bot bot commented Mar 25, 2024

Connor1996 commented Mar 25, 2024

ti-chi-bot bot commented Mar 25, 2024

ti-chi-bot bot commented Mar 25, 2024

ti-chi-bot commented Mar 27, 2024

ti-chi-bot commented Apr 15, 2024

ti-chi-bot commented May 7, 2024

ti-chi-bot bot commented Mar 21, 2024 •

edited