Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Concurrent addition of multiple members is not correct #175

Closed
belaban opened this issue May 17, 2022 · 4 comments
Closed

Concurrent addition of multiple members is not correct #175

belaban opened this issue May 17, 2022 · 4 comments
Assignees
Labels
Milestone

Comments

@belaban
Copy link
Member

belaban commented May 17, 2022

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?

@belaban belaban added the bug label May 17, 2022
@belaban belaban added this to the 1.0.9 milestone May 17, 2022
@belaban belaban self-assigned this May 17, 2022
@belaban belaban modified the milestones: 1.0.9, 1.0.10 May 19, 2022
@belaban belaban closed this as completed May 23, 2022
@belaban belaban reopened this May 23, 2022
@belaban
Copy link
Member Author

belaban commented May 23, 2022

Still fails... investigating

@belaban
Copy link
Member Author

belaban commented May 23, 2022

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.

@belaban
Copy link
Member Author

belaban commented May 25, 2022

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).

@belaban
Copy link
Member Author

belaban commented Jun 9, 2022

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));
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant