raft: ProposeConfChange/ApplyConfChange doesn't match thesis

[bdarnell]

Configuration changes do not currently work as described in the thesis (pages 35-36): we call raft.addNode() as a result of ApplyConfChange, when it should be done as soon as the change is proposed (or when we scan the tail of the log in becomeLeader). If this is safe for some other reason (I know @xiang90 understands this issue well), it should be documented.

[xiang90]

@bdarnell There are main two differences:

1. pre-check if the new node can catch up or not.
2. apply the configuration change before committing it.

For 1, I think it can be done outside raft. We plan to restore the snapshot and some of the committed log entries before the raft node actually starts.

For 2, the only difference I can see is when removing the node it might require a smaller quorum and when adding it might require a larger quorum.

If we apply the configuration change after commit, then that entry can still follow the normal path and does not require extra care. But it introduces a problem when you try to remove a member from a two members cluster. If one of the members dies, then the member cannot be removed since the cluster cannot make progress.

I think the safety guarantees remain the same.

[bdarnell]

For 1, I agree this can and should be done outside of raft.

My main concern is that the thesis specifically says that the change should take affect as soon as it is appended, so I just want to understand whether we're risking anything by doing it differently. I think the rule that no membership change can be proposed while another is pending covers every case I can think of; I haven't been able to poke a hole in it yet.

One difference in multiraft is that we send committed entries into a channel to be applied in another thread, while in etcd (I think) you apply everything before consuming the ready channel again. This means that multiraft may acknowledge some entries before older committed entries can be applied. Is it expected that all committed entries are applied before the next entries and messages are processed, or is it OK to do this asynchronously as we are?

[xiang90]

One difference in multiraft is that we send committed entries into a channel to be applied in another thread, while in etcd (I think) you apply everything before consuming the ready channel again.

That is not true in the future. etcd needs to apply in another thread too: #2117

Is it expected that all committed entries are applied before the next entries and messages are processed

No.

or is it OK to do this asynchronously as we are?

You must ensure the apply routine applies the entries as the same order they were committed.

The only difference between "apply configuration before commit it" and "apply configuration after commit it" is that the proposals between the real time the leader receives the configuration and the real time the leader commits the configuration are committed by the new configuration or the old configuration. (sorry about my english...) They are both safe IMO since the new configuration and the old configuration must be overlapped.

[xiang90]

@bdarnell It would be great if you can spend some time to write a doc for this and add it into /raft/doc.go.