diff --git a/server/raft.go b/server/raft.go index e7eded90ab9..a7dc872d0fc 100644 --- a/server/raft.go +++ b/server/raft.go @@ -953,16 +953,20 @@ func (n *raft) ProposeAddPeer(peer string) error { // As a leader if we are proposing to remove a peer assume its already gone. func (n *raft) doRemovePeerAsLeader(peer string) { n.Lock() + defer n.Unlock() + n.doRemovePeerLocked(peer) +} + +func (n *raft) doRemovePeerLocked(peer string) { if n.removed == nil { n.removed = map[string]time.Time{} } n.removed[peer] = time.Now() if _, ok := n.peers[peer]; ok { delete(n.peers, peer) - // We should decrease our cluster size since we are tracking this peer and the peer is most likely already gone. n.adjustClusterSizeAndQuorum() + n.writePeerState(&peerState{n.peerNames(), n.csz, n.extSt}) } - n.Unlock() } // ProposeRemovePeer is called to remove a peer from the group. @@ -3218,19 +3222,7 @@ func (n *raft) applyCommit(index uint64) error { peer := string(e.Data) n.debug("Removing peer %q", peer) - // Make sure we have our removed map. - if n.removed == nil { - n.removed = make(map[string]time.Time) - } - n.removed[peer] = time.Now() - - if _, ok := n.peers[peer]; ok { - delete(n.peers, peer) - // We should decrease our cluster size since we are tracking this peer. - n.adjustClusterSizeAndQuorum() - // Write out our new state. - n.writePeerState(&peerState{n.peerNames(), n.csz, n.extSt}) - } + n.doRemovePeerLocked(peer) // Remove from string intern map. peers.Delete(peer) diff --git a/server/raft_test.go b/server/raft_test.go index 47f7dc203bc..b65b24e5118 100644 --- a/server/raft_test.go +++ b/server/raft_test.go @@ -4344,3 +4344,38 @@ func TestNRGProposeRemovePeerLeader(t *testing.T) { require_Equal(t, len(newLeader.node().Peers()), 2) require_False(t, newLeader.node().MembershipChangeInProgress()) } + +func TestNRGLeaderResurrectsRemovedPeers(t *testing.T) { + c := createJetStreamClusterExplicit(t, "R3S", 3) + defer c.shutdown() + + rg := c.createMemRaftGroup("TEST", 3, newStateAdder) + rg.waitOnLeader() + + leader := rg.leader() + followers := rg.followers() + require_Equal(t, len(followers), 2) + + // Remove one follower + require_NoError(t, leader.node().ProposeRemovePeer(followers[0].node().ID())) + checkFor(t, 1*time.Second, 10*time.Millisecond, func() error { + if leader.node().MembershipChangeInProgress() { + return errors.New("membership still in progress") + } else { + return nil + } + }) + + require_Equal(t, len(leader.node().Peers()), 2) + + // Stop the leader and restart it. + // If bug is present: the leader resurrects the previously removed peer. + leader.stop() + followers[1].stop() + + leader.restart() + require_Equal(t, len(leader.node().Peers()), 2) + + followers[1].restart() + require_Equal(t, len(leader.node().Peers()), 2) +}