Avoid concurrent raft membership changes

[sciascid]

A simple approach based to avoid concurrent membership changes. The leader simply drops a membership change if there is another membership change in progress. A membership is in progress if it has been proposed, but not committed yet.

Signed-off-by: Daniele Sciascia daniele@nats.io

[neilalexander]

Do we need to include EntryPeerState here too?

[sciascid]

This could be considered complete.
Possible improvements:

• Since this is tracking some state (the boolean) we might as well track the entire entry, for example. This would allow for some sanity checks.
• Could return an error when there's an attempt to add / remove concurrently, instead of dropping the proposal silently
• Could queue up concurrent proposals instead of dropping them
• Testing: it's complicated... tried very hard to come up with a test that shows concurrent proposals are not possible, even under leader change. Don't think it is possible right now.

[MauriceVanVeen]

My primary worry with only disallowing membership in Raft is that we'll be protected against concurrent changes on the lowest level. But the user issuing nats server cluster peer-remove will still observe "success" for all meta peer-remove requests, even if they'd try to do so concurrently or when there's no quorum on the peer-remove. This is because the upper layer will still always respond without waiting for quorum, or knowing that another peer-remove was already in progress.

[MauriceVanVeen]

Generally LGTM, but could you try removing these loops in monitorStream and monitorConsumer that are used for scale moves and see if the tests still pass? Since that in essence would still rely on being able to do concurrent membership changes.

for _, p := range oldPeers {
	n.ProposeRemovePeer(p)
}

[sciascid]

Generally LGTM, but could you try removing these loops in monitorStream and monitorConsumer that are used for scale moves and see if the tests still pass? Since that in essence would still rely on being able to do concurrent membership changes.

for _, p := range oldPeers {
	n.ProposeRemovePeer(p)
}

Removed the loops with a call to ProposeKnownPeers()

[MauriceVanVeen]

LGTM

[neilalexander]

LGTM