raft: make leader transferring workable when quorum check is on #5809
Conversation
| @@ -175,8 +175,8 @@ func testNonleaderStartElection(t *testing.T, state StateType) { | |||
| msgs := r.readMessages() | |||
| sort.Sort(messageSlice(msgs)) | |||
| wmsgs := []pb.Message{ | |||
| {From: 1, To: 2, Term: 2, Type: pb.MsgVote}, | |||
| {From: 1, To: 3, Term: 2, Type: pb.MsgVote}, | |||
| {From: 1, To: 2, Term: 2, Type: pb.MsgVote, Entries: []pb.Entry{{Data: []byte(LeaderElection)}}}, | |||
There was a problem hiding this comment.
empty should still work, right (from the code change in raft.go, i feel it is already so)? we need to ensure compatibility.
|
LGTM. /cc @bdarnell |
| @@ -575,7 +587,9 @@ func (r *raft) Step(m pb.Message) error { | |||
| case m.Term > r.Term: | |||
| lead := m.From | |||
| if m.Type == pb.MsgVote { | |||
| if r.checkQuorum && r.state != StateCandidate && r.electionElapsed < r.electionTimeout { | |||
| force := len(m.Entries) == 1 && bytes.Equal(m.Entries[0].Data, []byte(LeaderTransfer)) | |||
There was a problem hiding this comment.
I don't like using Entries[0].Data to store data in non-MsgApp messages. I'd rather introduce a top-level Data field in raftpb.Message.
|
LGTM |
|
@swingbach I agree with what @bdarnell suggested. |
|
@swingbach You should run ./script/genproto.sh. It will get the right version of gogo-proto for you. For protoc, it requires https://github.com/google/protobuf/releases/tag/v3.0.0-beta-3.1 |
|
already fixed the last one @xiang90 PTAL, Thanks |
| @@ -64,6 +64,7 @@ message Message { | |||
| optional Snapshot snapshot = 9 [(gogoproto.nullable) = false]; | |||
| optional bool reject = 10 [(gogoproto.nullable) = false]; | |||
| optional uint64 rejectHint = 11 [(gogoproto.nullable) = false]; | |||
| optional bytes context = 12; | |||
There was a problem hiding this comment.
I prefer defining a campaign enum and use a campaign type here instead of context.
There was a problem hiding this comment.
@siddontang what you suggested makes sense for this single case, but I hope it can be more generic which can be used for more cases, such as ReadIndexMsg, it still use Entries of Message to store some user context, i wanna store that context into this field too.
thanks
|
LGTM. |
port etcd leader lease. etcd-io/etcd#5468 etcd-io/etcd#5809
Notable changes include etcd-io/etcd#6286 (which fixes the issue that was preventing us from upgrading sooner), and etcd-io/etcd#5809 (which fixes a possible cause of failed leader transfers discussed in cockroachdb#8834) Fixes cockroachdb#8017
port etcd leader lease. etcd-io/etcd#5468 etcd-io/etcd#5809
port etcd leader lease. etcd-io/etcd#5468 etcd-io/etcd#5809
try to make leader transferring works when quorum check is on
i think a few of database guys are gonna need this.
/cc @siddontang