Concurrent addition of multiple members is not correct
Closed this issue · 4 comments
DynamicMembershipTest.testAddServerSimultaneously()
is not correct: majority
in a RequestTable.Entry
is cached (per call, possibly with multiple entries being added to the request-table), and the number of votes required for a majority is not correct.
Perhaps use raft.majority()
dynamically?
Still fails... investigating
The issue is that - when adding D
and E
almost simultaniously to {A,B,C}
- D
requires a majority of 2
(of {A,B,C}
), but E
should require 3
(out of {A,B,C,D}
). However, at the time, E
is being added, D
might not yet have been committed (changing the membership from {A,B,C}
-> {A,B,C,D}
), therefore E
might still see the membership as {A,B,C}
, and therefore falsely use a majority of 2
rather than 3
.
Solution: the leader chains all add-server
and remove-server
requests calling setAsync()
and chaining the returned CompletionStage
with all elements of the queue (removing them). This way, a server is added or removed only when the previous request has been completed (committed).
Code:
protected CompletableFuture<byte[]> changeMembers(String name, InternalCommand.Type type) throws Exception {
InternalCommand cmd=new InternalCommand(type, name);
byte[] buf=Util.streamableToByteBuffer(cmd);
// only add/remove one server at a time (https://github.com/belaban/jgroups-raft/issues/175)
return add_server_future=add_server_future
.thenCompose(s -> setAsync(buf, 0, buf.length, true, null));
}