Repository navigation
internal/rest/resources: Allow forced removal of pending members - #774
Open
claudiubelu wants to merge 1 commit into
Open
claudiubelu wants to merge 1 commit into
claudiubelu wants to merge 1 commit into
Conversation
dqlite.Close() tears down Raft connections but does not remove the node from the dqlite cluster membership. When db.Join() fails after the node has already been added to the cluster (via cli.Add in app.run), the reverter only called Close(), leaving a ghost member in dqlite. This ghost node causes CheckMembershipConsistency to fail (the node exists in dqlite but not in core_cluster_members), which blocks token generation and all future cluster joins. Fix the Join reverter to explicitly ask the dqlite leader to remove us before closing. Also correct a misleading comment in the Bootstrap reverter. Signed-off-by: Claudiu Belu <cbelu@cloudbasesolutions.com>
roosterfish
requested changes
Jul 1, 2026
roosterfish
left a comment
roosterfish
left a comment
Contributor
There was a problem hiding this comment.
Hey, can you please share some reproducer steps how you could end up in this state?
I see we have an exception to continue starting up the database in case we hit a update.ErrGracefulAbort error. But I would be very interested to see what else could go wrong.
Also could you please add the reproducer as an additional check to test_membership_consistency ?
There was a problem hiding this comment.
Pull request overview
Fixes a cluster-join failure mode where a node that was added to dqlite membership but failed later in db.Join() could be left behind as a “ghost” dqlite member, breaking membership consistency checks and blocking future operations like token generation.
Changes:
- Corrected comments clarifying that
dqlite.Close()tears down Raft connections but does not remove a node from dqlite membership. - Updated the
Join()reverter to attempt removal of the local node from the dqlite cluster before closing, to avoid leaving a ghost member behind.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+247
to
+251
| cli, err := db.dqlite.Leader(removeCtx, dqliteClient.WithConcurrentLeaderConns(1)) | ||
| if err != nil { | ||
| db.log().Warn("Failed to connect to dqlite leader to remove ourselves from the cluster", slog.String("address", db.listenAddr.String()), slog.String("error", err.Error())) | ||
| } else { | ||
| err = cli.Remove(removeCtx, db.dqlite.ID()) |
Comment on lines
+244
to
+248
| removeCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | ||
| defer cancel() | ||
|
|
||
| cli, err := db.dqlite.Leader(removeCtx, dqliteClient.WithConcurrentLeaderConns(1)) | ||
| if err != nil { |
Contributor
|
Marking as draft until you get a chance to get back to it @claudiubelu. |
roosterfish
marked this pull request as draft
July 8, 2026 12:40
claudiubelu
changed the title
Contributor
Author
|
Revisited the problem(s) again:
|
claudiubelu
marked this pull request as ready for review
October 28, 2026 13:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A freshly bootstrapped member stays
PENDINGuntil the first heartbeat. If a second member joins in that window and its join fails after dqlite added it, the join failure cleanup sends a forcedDELETEfor it, which was refused with"Cannot remove cluster members, there are no remaining non-pending members".The failed joiner was then left behind in
core_cluster_members, the truststore and dqlite, and was never reset. As its dqlite app is closed, the cluster cannot reliably reach its dqlite leader anymore, which blocks token generation and further joins until the member is removed manually.Skip the pending members check for the forced removal of a member that is itself pending, as is always the case for a failed joiner. Removing a pending member does not change the number of non-pending members, and the check that the cluster keeps at least 2 dqlite members still applies.
Extend
test_membership_consistencywith a join of c2 right after the bootstrap, while c1 is still PENDING, which fails after dqlite added c2 because of an invalid schema patch.The failed join must be rolled back on the cluster, which must stay consistent and allow c2 to join afterwards.
Depends-On: #850