Skip to content

Comments

NRG: Fix leader peer removal#7600

Merged
neilalexander merged 1 commit intomainfrom
raft-leader-peer-remove
Dec 2, 2025
Merged

NRG: Fix leader peer removal#7600
neilalexander merged 1 commit intomainfrom
raft-leader-peer-remove

Conversation

@sciascid
Copy link
Contributor

@sciascid sciascid commented Dec 2, 2025

Revisit quorum counting in trackPeer so that the implicit ack from the leader is counted only if the node is still part of the membership. So the corresponsing EntryPeerRemove message must be replicated to a majority of the new membership, not including the leader to be removed.
When the leader commits an EntryPeerRemove message that removes the leader itself, attempt a proper step-down to a new leader, before stopping.

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

@sciascid sciascid requested a review from a team as a code owner December 2, 2025 06:53
@sciascid sciascid force-pushed the raft-leader-peer-remove branch from 08c8f0b to 6211f1c Compare December 2, 2025 06:56
Copy link
Member

@MauriceVanVeen MauriceVanVeen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


newLeader := rg.waitOnLeader()
require_True(t, leader.State() == Closed)
require_True(t, leader.ID() != newLeader.node().ID())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit, but require_Equal and require_NotEqual are better for these cases. That way if the assertion fails, the log line contains much more useful information.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Revisit quorum counting in `trackPeer` so that the implicit ack
from the leader is counted only if the node is still part of
the membership. So the corresponsing EntryPeerRemove message
must be replicated to a majority of the new membership, not
including the leader to be removed.
When the leader commits an EntryPeerRemove message that removes
the leader itself, attempt a proper step-down to a new leader,
before stopping.

Signed-off-by: Daniele Sciascia <daniele@nats.io>
@sciascid sciascid force-pushed the raft-leader-peer-remove branch from 6211f1c to d88e9e8 Compare December 2, 2025 10:17
Copy link
Member

@neilalexander neilalexander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@neilalexander neilalexander merged commit b95ddcd into main Dec 2, 2025
48 checks passed
@neilalexander neilalexander deleted the raft-leader-peer-remove branch December 2, 2025 10:33
neilalexander added a commit that referenced this pull request Dec 5, 2025
Includes the following:

- #7581
- #7585
- #7586
- #7565
- #7588
- #7593
- #7589
- #7594
- #7595
- #7596
- #7597
- #7598
- #7600
- #7601
- #7602
- #7604
- #7605
- #7607
- #7609
- #7610
- #7616
- #7614

Signed-off-by: Neil Twigg <neil@nats.io>
neilalexander added a commit that referenced this pull request Jan 6, 2026
Includes the following:

- #7565
- #7589
- #7600
- #7602
- #7609
- #7610
- #7632
- #7649
- #7642
- #7658
- #7659
- #7661
- #7662
- #7663
- #7668
- #7683
- #7685
- #7686
- #7678
- #7691
- #7696
- #7698
- #7699
- #7700

Signed-off-by: Neil Twigg <neil@nats.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants