etcd-io / raft

Raft library for maintaining a replicated state machine
Apache License 2.0
666 stars 164 forks source link

raft: error on ignored proposal and wait for EntryConfChange results #44

Open lavacat opened 1 year ago

lavacat commented 1 year ago

fixes: https://github.com/etcd-io/raft/issues/43

lavacat commented 1 year ago

there might be some other side effects of returning ErrProposalDropped. If there is interest in merging this PR, I can do some more digging/testing.

chaochn47 commented 1 year ago

etcd only uses so-called deprecated pb.ConfChange, but I am wondering how this change will impact pb.ConfChangeV2 handling.

https://github.com/etcd-io/raft/blob/3cbcd6e7088c773cab6331459162561682e0d1df/node.go#L141-L153

cc @ahrtr @serathius for insights~

ahrtr commented 1 year ago

etcd only uses so-called deprecated pb.ConfChange, but I am wondering how this change will impact pb.ConfChangeV2 handling.

raft will automatically convert it to V2, see node.go#L533.

m.Entries[i] = pb.Entry{Type: pb.EntryNormal}

The intention is just to ignore the confChange, as the log "ignoring conf change" mentions.

chaochn47 commented 1 year ago

etcd only uses so-called deprecated pb.ConfChange, but I am wondering how this change will impact pb.ConfChangeV2 handling.

To be clear, I am not sure what's the impact of fail fast in case of pb.ConfChangeV2 handling https://github.com/etcd-io/raft/blob/7357439323abcd0654b7146022f7a58793d667eb/raft.go#L1222-L1226

Is there an unknown reason that designs like this? @tbg for consulting..

lavacat commented 1 year ago

cc @nvanbenschoten, noticed some of your recent comments here

https://github.com/etcd-io/raft/blob/7357439323abcd0654b7146022f7a58793d667eb/raft.go#L698-L704

can you take a look at this PR?

nvanbenschoten commented 1 year ago

@lavacat thanks for the ping. I don't see any issue with the changes made to stepLeader. There's now a decent amount of coupling between the rejection logic in stepLeader and the auto-leave proposal logic in appliedTo, but it appears to be correct.