Merge pull request #1135 from rqlite/ignore-join-explicit-if-state

Ignore explicit join request if there is preexisting state

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## 7.12.1 (unreleased)
 ### Implementation changes and bug fixes
+- [PR #1135](https://github.com/rqlite/rqlite/pull/1135): Silently ignore self-joins if nothing has changed on the joining node.
 - [PR #1136](https://github.com/rqlite/rqlite/pull/1136): Stop HTTP server gracefully on node shutdown.
 - [f6c4b17](https://github.com/rqlite/rqlite/commit/f6c4b17a727809696f952a018b2262681932f521): By default, Leader node will stepdown if that node is shutting down.
 

cmd/rqlited/main.go

@@ -381,12 +381,12 @@ func createCluster(cfg *Config, tlsConfig *tls.Config, hasPeers bool, str *store
 	}
 
 	if joins != nil && cfg.BootstrapExpect > 0 {
-		// Bootstrap with explicit join addresses requests.
 		if hasPeers {
 			log.Println("preexisting node configuration detected, ignoring bootstrap request")
 			return nil
 		}
 
+		// Bootstrap with explicit join addresses requests.
 		bs := cluster.NewBootstrapper(cluster.NewAddressProviderString(joins), tlsConfig)
 		if cfg.JoinAs != "" {
 			pw, ok := credStr.Password(cfg.JoinAs)
@@ -408,7 +408,7 @@ func createCluster(cfg *Config, tlsConfig *tls.Config, hasPeers bool, str *store
 	switch cfg.DiscoMode {
 	case DiscoModeDNS, DiscoModeDNSSRV:
 		if hasPeers {
-			log.Printf("preexisting node configuration detected, ignoring %s", cfg.DiscoMode)
+			log.Printf("preexisting node configuration detected, ignoring %s option", cfg.DiscoMode)
 			return nil
 		}
 		rc := cfg.DiscoConfigReader()

store/store.go

@@ -36,10 +36,6 @@ var (
 	// operation.
 	ErrNotLeader = errors.New("not leader")
 
-	// ErrSelfJoin is returned when a join-request is received from a node
-	// with the same Raft ID as this node.
-	ErrSelfJoin = errors.New("self-join attempted")
-
 	// ErrStaleRead is returned if the executing the query would violate the
 	// requested freshness.
 	ErrStaleRead = errors.New("stale read")
@@ -214,9 +210,10 @@ type Store struct {
 	numTrailingLogs uint64
 
 	// For whitebox testing
-	numNoops       int
-	numSnapshotsMu sync.Mutex
-	numSnapshots   int
+	numIgnoredJoins int
+	numNoops        int
+	numSnapshotsMu  sync.Mutex
+	numSnapshots    int
 }
 
 // IsNewNode returns whether a node using raftDir would be a brand-new node.
@@ -1014,9 +1011,6 @@ func (s *Store) Join(id, addr string, voter bool) error {
 	if s.raft.State() != raft.Leader {
 		return ErrNotLeader
 	}
-	if id == s.raftID {
-		return ErrSelfJoin
-	}
 
 	configFuture := s.raft.GetConfiguration()
 	if err := configFuture.Error(); err != nil {
@@ -1032,6 +1026,7 @@ func (s *Store) Join(id, addr string, voter bool) error {
 			// join is actually needed.
 			if srv.Address == raft.ServerAddress(addr) && srv.ID == raft.ServerID(id) {
 				stats.Add(numIgnoredJoins, 1)
+				s.numIgnoredJoins++
 				s.logger.Printf("node %s at %s already member of cluster, ignoring join request", id, addr)
 				return nil
 			}

store/store_test.go

@@ -2,7 +2,6 @@ package store
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
 	"io/ioutil"
 	"math/rand"
@@ -1105,7 +1104,7 @@ func Test_SingleNodeRecoverNetworkChangeSnapshot(t *testing.T) {
 	}
 }
 
-func Test_SingleNodeSelfJoinFail(t *testing.T) {
+func Test_SingleNodeSelfJoinNoChangeOK(t *testing.T) {
 	s0, ln0 := mustNewStore(t, true)
 	defer ln0.Close()
 	if err := s0.Open(); err != nil {
@@ -1120,13 +1119,13 @@ func Test_SingleNodeSelfJoinFail(t *testing.T) {
 		t.Fatalf("Error waiting for leader: %s", err)
 	}
 
-	// Self-join should fail.
+	// Self-join should not be a problem. It should just be ignored.
 	err := s0.Join(s0.ID(), s0.Addr(), true)
-	if err == nil {
-		t.Fatalf("failed to receive error for self-join")
+	if err != nil {
+		t.Fatalf("received error for non-changing self-join")
 	}
-	if !errors.Is(err, ErrSelfJoin) {
-		t.Fatalf("received wrong error for self-join attempt: %s", err)
+	if got, exp := s0.numIgnoredJoins, 1; got != exp {
+		t.Fatalf("wrong number of ignored joins, exp %d, got %d", exp, got)
 	}
 }