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

*: Add a Raft admin command to put the region into a locking flashback state. #13541

Merged
merged 32 commits into from Oct 12, 2022
Merged

*: Add a Raft admin command to put the region into a locking flashback state. #13541

merged 32 commits into from Oct 12, 2022

Conversation

HuSharp
Copy link
Member

@HuSharp HuSharp commented Sep 27, 2022 •
edited

Signed-off-by: husharp jinhao.hu@pingcap.com

What is changed and how it works?

Issue Number: ref #13519 ref #13303

What's Changed:

Add a Raft admin command to put the region into a lock state to prevent any reading, writing, and scheduling and persist the state in the RegionLocalState.

Check List

Tests

  • Unit test
  • Integration test

Release note 

None

@HuSharp
roughly modify flashback
0c5aaa6 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
@ti-chi-bot
Copy link
Member

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

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • AndreMouche
  • Connor1996

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.

@HuSharp
Copy link
Member Author

HuSharp commented Sep 27, 2022

/cc @JmPotato

@ti-chi-bot
Copy link
Member

@HuSharp: GitHub didn't allow me to request PR reviews from the following users: JmPotato.

Note that only tikv members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @JmPotato

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 kubernetes/test-infra repository.

@HuSharp
remove redundant code
88f6f19 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
@HuSharp
remove redundant code
61c4585 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
JmPotato
JmPotato reviewed Sep 27, 2022
View reviewed changes
components/raftstore/src/store/fsm/apply.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/apply.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/peer.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/peer.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/peer.rs Outdated Show resolved Hide resolved
src/server/service/kv.rs Outdated Show resolved Hide resolved
@ti-chi-bot
Copy link
Member

@JmPotato: Request changes is only allowed for the reviewers in list.

In response to this:

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.

@HuSharp
address comment
d5ff1c2 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
JmPotato
JmPotato reviewed Sep 27, 2022
View reviewed changes
components/raftstore/src/store/fsm/apply.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/apply.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/peer.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/peer.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/peer.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/peer.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/peer.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/peer.rs Outdated Show resolved Hide resolved
src/server/service/kv.rs Outdated Show resolved Hide resolved
@JmPotato JmPotato mentioned this pull request Sep 28, 2022
Closed
3 tasks
@HuSharp
modify to channel
87f1db0 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
JmPotato
JmPotato reviewed Sep 28, 2022
View reviewed changes
components/raftstore/src/store/fsm/apply.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/apply.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/peer.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/peer.rs Outdated Show resolved Hide resolved
@HuSharp
address comment
6674ac9 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
@HuSharp
remove redundant code
c29d604 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
@HuSharp
modify parsist state to bool
34a281d 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
@HuSharp
pass test_sync_log
486de94 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
JmPotato
JmPotato reviewed Sep 29, 2022
View reviewed changes
Copy link
Member

@JmPotato JmPotato left a comment

Choose a reason for hiding this comment

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

The rest LGTM.

components/raftstore/src/store/fsm/apply.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/apply.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/fsm/peer.rs Outdated Show resolved Hide resolved
components/raftstore/src/store/peer.rs Outdated Show resolved Hide resolved
@JmPotato JmPotato mentioned this pull request Sep 29, 2022
Merged
@HuSharp
remove reply for apply
245998d 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
@HuSharp
Copy link
Member Author

HuSharp commented Oct 11, 2022

Finished addressing all the comments, PTAL! @Connor1996 @JmPotato

@HuSharp
propagate channel error
c6bc7ea 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
@HuSharp
add flag for prepare and finish flashback admin req
da25573 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
JmPotato added a commit to JmPotato/tikv that referenced this pull request Oct 11, 2022
@JmPotato
Merge the tikv#13541
e679a3f 
Signed-off-by: JmPotato <ghzpotato@gmail.com>
@HuSharp
address comment
fab24bb 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
Connor1996
Connor1996 approved these changes Oct 11, 2022
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 added the status/LGT1 Status: PR - There is already 1 approval label Oct 11, 2022
@ti-chi-bot
Copy link
Member

@JmPotato: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

In response to this:

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.

@HuSharp
merge upstream
e488437 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
@HuSharp
update kvproto
a6f05f0 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
@HuSharp
Make the first letter of the comment capitalized
42ca68e 
Signed-off-by: husharp <jinhao.hu@pingcap.com>
AndreMouche
AndreMouche approved these changes Oct 12, 2022
View reviewed changes
Copy link
Member

@AndreMouche AndreMouche 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 added status/LGT2 Status: PR - There are already 2 approvals and removed status/LGT1 Status: PR - There is already 1 approval labels Oct 12, 2022
@AndreMouche
Copy link
Member

/merge

@ti-chi-bot
Copy link
Member

@AndreMouche: 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: 42ca68e

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

/test

@ti-chi-bot ti-chi-bot merged commit c286b8a into tikv:master Oct 12, 2022
@ti-chi-bot ti-chi-bot added this to the Pool milestone Oct 12, 2022
@HuSharp HuSharp deleted the flashback_persist_in branch October 13, 2022 12:21
ti-chi-bot added a commit that referenced this pull request Oct 17, 2022
@JmPotato @ti-chi-bot
*: introduce the two-phase kv_flashback_to_version (#13557)
b37c2f6 
close #13519, ref #13519, ref #13541

Make `FlashbackToVersion` become a two-phase request as described in #13519.

Signed-off-by: JmPotato <ghzpotato@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndreMouche AndreMouche AndreMouche approved these changes

@JmPotato JmPotato JmPotato approved these changes

@Connor1996 Connor1996 Connor1996 approved these changes

Assignees
No one assigned
Labels
contribution Type: PR - From contributors 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
5 participants
@HuSharp @ti-chi-bot @AndreMouche @JmPotato @Connor1996