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

raftstore-v2: support unsafe recovery create and destroy peer #15185

Merged
merged 6 commits into from Aug 3, 2023
Merged

raftstore-v2: support unsafe recovery create and destroy peer #15185

merged 6 commits into from Aug 3, 2023

Conversation

overvenus
Copy link
Member

@overvenus overvenus commented Jul 24, 2023 •
edited

What is changed and how it works?

Issue Number: ref #15108

What's Changed:

* raftstore-v2: support unsafe recovery create peer

  Reuse split mechanism to create peer. Because in v2 the only way to
  create and initialize to peer is via a snapshot message.

* raftstore-v2: support unsafe recovery destroy peer

Check List

Tests

  • Unit test
  • Integration test

Release note 

None

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jul 24, 2023 •
edited

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • Connor1996
  • tonyxuqqi

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.

@overvenus overvenus changed the title raftstore-v2: support unsafe recovery create peer raftstore-v2: support unsafe recovery create and destroy peer Jul 24, 2023
Connor1996
Connor1996 reviewed Jul 25, 2023
View reviewed changes
components/raftstore-v2/src/operation/query/mod.rs Outdated
@@ -141,13 +141,9 @@ impl<EK: KvEngine, ER: RaftEngine> Peer<EK, ER> {
));
}

// TODO: add flashback_state check
Copy link
Member

Choose a reason for hiding this comment

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

not added

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a clean up, flashback is supported in v2.

@overvenus
raftstore-v2: support unsafe recovery create peer
869473e 
Reuse split mechanism to create peer. Because in v2 the only way to
create and initialize to peer is via a snapshot message.

Signed-off-by: Neil Shen <overvenus@gmail.com>
@overvenus
raftstore-v2: support unsafe recovery destroy peer
4ef1d85 
Signed-off-by: Neil Shen <overvenus@gmail.com>
@overvenus
move test_unsafe_recovery_create_destroy_reentrancy to integrations
8381b45 
Signed-off-by: Neil Shen <overvenus@gmail.com>
@overvenus overvenus force-pushed the raftstorev2/unsafe-recovery-create branch from 7cf49bb to 8381b45 Compare July 26, 2023 07:31
@ti-chi-bot ti-chi-bot bot removed the needs-rebase label Jul 26, 2023
@overvenus overvenus marked this pull request as ready for review July 26, 2023 07:31
@overvenus overvenus mentioned this pull request Jul 26, 2023
Closed
6 tasks
@overvenus
fix fmt
92637a8 
Signed-off-by: Neil Shen <overvenus@gmail.com>
@ti-chi-bot ti-chi-bot bot added the status/LGT1 Status: PR - There is already 1 approval label Jul 28, 2023
Connor1996
Connor1996 approved these changes Aug 3, 2023
View reviewed changes
Copy link
Member

@Connor1996 Connor1996 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-chi-bot 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 Aug 3, 2023
@overvenus
address comments
f27b880 
Signed-off-by: Neil Shen <overvenus@gmail.com>
@overvenus
Copy link
Member Author

/merge

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Aug 3, 2023

@overvenus: 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
Contributor

ti-chi-bot bot commented Aug 3, 2023

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

Commit hash: f27b880

@ti-chi-bot ti-chi-bot bot added the status/can-merge Status: Can merge to base branch label Aug 3, 2023
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Aug 3, 2023

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

@overvenus
Copy link
Member Author

/test

@ti-chi-bot ti-chi-bot bot merged commit 7a3163f into tikv:master Aug 3, 2023
7 checks passed
@ti-chi-bot ti-chi-bot bot added this to the Pool milestone Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tonyxuqqi tonyxuqqi tonyxuqqi approved these changes

@Connor1996 Connor1996 Connor1996 approved these changes

@tabokie tabokie Awaiting requested review from tabokie

Assignees
No one assigned
Labels
release-note-none size/XXL 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
3 participants
@overvenus @tonyxuqqi @Connor1996