Skip to content

internal/db: Remove node from dqlite cluster on failed join #775

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.

Sign up for GitHub

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

Draft
claudiubelu wants to merge 1 commit into
base: v3
Choose a base branch
from
Draft

internal/db: Remove node from dqlite cluster on failed join #775

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 29 additions & 7 deletions internal/db/dqlite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,9 @@ func (db *DqliteDB) Bootstrap(extensions types.Extensions, addr *url.URL, cluste
}

// Only close the dqlite app if the entire Bootstrap process fails or exits.
// dqlite.Close() immediately tears down all Raft connections and removes this node from the dqlite cluster.
// NOTE: dqlite.Close() tears down Raft connections but does NOT remove
// the node from the dqlite cluster. For bootstrap this is fine since
// the node is the sole member.
reverter := revert.New()
defer reverter.Fail()
reverter.Add(func() {
Expand Down Expand Up @@ -227,18 +229,38 @@ func (db *DqliteDB) Join(extensions types.Extensions, addr *url.URL, joinAddress

// Only close the dqlite app if the entire Join process fails or exits.
// This ensures dqlite.Close() is not called on every Open() retry, but only if we fully give up joining.
// dqlite.Close() immediately tears down all Raft connections and removes this node from the dqlite cluster.
//
// NOTE: dqlite.Close() tears down Raft connections but does NOT remove
// the node from the dqlite cluster. We must explicitly ask the leader to
// remove us before closing; otherwise the node remains as a ghost member,
// which can break quorum calculations and membership consistency checks.
reverter := revert.New()
defer reverter.Fail()
reverter.Add(func() {
if db.dqlite != nil {
closeErr := db.dqlite.Close()
if closeErr != nil {
db.log().Error("Failed to close dqlite app", slog.String("address", db.listenAddr.String()), slog.String("error", closeErr.Error()))
if db.dqlite == nil {
return
}

removeCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()

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())
cli.Close()
if err != nil {
db.log().Warn("Failed to remove ourselves from the dqlite cluster", slog.Uint64("id", db.dqlite.ID()), slog.String("address", db.listenAddr.String()), slog.String("error", err.Error()))
}
}

db.dqlite = nil
closeErr := db.dqlite.Close()
if closeErr != nil {
db.log().Error("Failed to close dqlite app", slog.String("address", db.listenAddr.String()), slog.String("error", closeErr.Error()))
}

db.dqlite = nil
})

for {
Expand Down