raftstorev2: add basic layout #12843

BusyJay · 2022-06-16T23:04:23Z

What is changed and how it works?

Issue Number: Ref #12842

What's Changed:

This is an attempt to reimplement raftstore using the new assumptions
that peer's range can be overlapped.

Currently, compatability is not considered, though we may think about
how to migrate from old version by the end of this year.

No concrete implementations is added yet, we may choose reuse
implementation from v1 or implementing new logic base on actual
requirement. The principle is
1. do not introduce history debt while reusing code as much as possible.
2. do not change the current implementations.

Check List

Tests

Unit test
Integration test

Release note

None

This is an attempt to reimplement raftstore using the new assumptions that peer's range can be overlapped. Currently, compatability is not considered, though we may think about how to migrate from old version by the end of this year. No concrete implementations is added yet, we may choose reuse implementation from v1 or implementing new logic base on actual requirement. The principle is do not introduce history debt while reusing code as much as possible. Signed-off-by: Jay Lee <BusyJayLee@gmail.com>

ti-chi-bot · 2022-06-16T23:04:25Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

tonyxuqqi
zhangjinpeng1987

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.

Signed-off-by: Jay Lee <BusyJayLee@gmail.com>

zhangjinpeng87

LGTM

tonyxuqqi · 2022-06-17T18:30:28Z

components/raftstore-v2/src/router/message.rs

@@ -0,0 +1,293 @@
+// Copyright 2016 TiKV Project Authors. Licensed under Apache-2.0.


ASF suggests using the interception year of the project consistently for all its files. But of course it's not a requirement.

[Bug] skyWalking-eyes checks header should not require to match this year apache/skywalking#10223

https://issues.apache.org/jira/browse/LEGAL-629

tonyxuqqi · 2022-06-17T18:36:13Z

components/raftstore-v2/src/raft/storage.rs

+    logger: Logger,
+}
+
+impl<ER> Storage<ER> {


for Raft Storage, what's the expecting difference between v2 and v1?

The initialization is different.

tonyxuqqi · 2022-06-17T18:40:50Z

components/raftstore-v2/src/router/message.rs

+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+#[repr(u8)]
+pub enum PeerTick {


why not use V1's implementation as they're identical?

Several ticks will be moved from StoreTick to PeerTick.

tonyxuqqi · 2022-06-17T18:41:30Z

components/raftstore-v2/src/router/message.rs

+}
+
+#[derive(Debug, Clone, Copy)]
+pub enum StoreTick {


Same here. It seems identical to v1.

No, they are different. Several ticks are removed.

tonyxuqqi · 2022-06-17T18:44:55Z

components/raftstore-v2/src/router/message.rs

+}
+
+/// Message that can be sent to a peer.
+pub enum PeerMsg<EK: KvEngine> {


Comparing with V1, it does not have CasualMessage, HeartbeatPd, Destroy. So are they not needed anymore in v2?

No, I delete them on purpose and add them as needed in future development.

tonyxuqqi · 2022-06-17T18:50:00Z

Also regarding the assumptions:

peer's range can be overlapped: do you mean the actual tablet's data range or the region's range.
"Currently, compatability is not considered, though we may think about
how to migrate from old version by the end of this year", does it mean the TiKV won't work for both version until this end of the year? (Or do you mean we will essentially two version of tikv?)

BusyJay · 2022-06-17T18:53:19Z

1. peer's range can be overlapped: do you mean the actual tablet's data range or the region's range.

Both can be overlapped.

does it mean the TiKV won't work for both version until this end of the year? (Or do you mean we will essentially two version of tikv?)

Yes, it will work with either one but not both at the same time. There will be only one version TiKV while can start with different version of raftstore.

tonyxuqqi · 2022-06-18T00:49:04Z

/LGTM

zhangjinpeng87 · 2022-06-18T08:27:59Z

/merge

ti-chi-bot · 2022-06-18T08:28:00Z

@zhangjinpeng1987: 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 · 2022-06-18T08:28:02Z

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

Commit hash: 97a3faa

BusyJay requested review from tonyxuqqi and 5kbpers June 16, 2022 23:04

ti-chi-bot added release-note-none size/XXL labels Jun 16, 2022

BusyJay added this to In progress in Make region size dynamic via automation Jun 16, 2022

fix warnings

97a3faa

Signed-off-by: Jay Lee <BusyJayLee@gmail.com>

Connor1996 self-requested a review June 17, 2022 05:58

Make region size dynamic automation moved this from In progress to Reviewer approved Jun 17, 2022

zhangjinpeng87 approved these changes Jun 17, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Status: PR - There is already 1 approval label Jun 17, 2022

tonyxuqqi reviewed Jun 17, 2022

View reviewed changes

tonyxuqqi approved these changes Jun 17, 2022

View reviewed changes

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 Jun 17, 2022

ti-chi-bot added the status/can-merge Status: Can merge to base branch label Jun 18, 2022

ti-chi-bot merged commit c1a09b8 into tikv:master Jun 18, 2022

Make region size dynamic automation moved this from Reviewer approved to Done Jun 18, 2022

BusyJay deleted the raftstorev2 branch June 18, 2022 18:00

BusyJay mentioned this pull request Aug 9, 2022

Support physical isolation between regions #12842

Open

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

raftstorev2: add basic layout #12843

raftstorev2: add basic layout #12843

BusyJay commented Jun 16, 2022 •

edited

ti-chi-bot commented Jun 16, 2022 •

edited

zhangjinpeng87 left a comment

tonyxuqqi Jun 17, 2022

tisonkun Jun 17, 2023

tonyxuqqi Jun 17, 2022

BusyJay Jun 17, 2022

tonyxuqqi Jun 17, 2022

BusyJay Jun 17, 2022

tonyxuqqi Jun 17, 2022

BusyJay Jun 17, 2022

tonyxuqqi Jun 17, 2022

BusyJay Jun 17, 2022

tonyxuqqi commented Jun 17, 2022

BusyJay commented Jun 17, 2022

tonyxuqqi commented Jun 18, 2022

zhangjinpeng87 commented Jun 18, 2022

ti-chi-bot commented Jun 18, 2022

ti-chi-bot commented Jun 18, 2022

		@@ -0,0 +1,293 @@
		// Copyright 2016 TiKV Project Authors. Licensed under Apache-2.0.

raftstorev2: add basic layout #12843

raftstorev2: add basic layout #12843

Conversation

BusyJay commented Jun 16, 2022 • edited

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Jun 16, 2022 • edited

zhangjinpeng87 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tonyxuqqi commented Jun 17, 2022

BusyJay commented Jun 17, 2022

tonyxuqqi commented Jun 18, 2022

zhangjinpeng87 commented Jun 18, 2022

ti-chi-bot commented Jun 18, 2022

ti-chi-bot commented Jun 18, 2022

BusyJay commented Jun 16, 2022 •

edited

ti-chi-bot commented Jun 16, 2022 •

edited