Merge pull request #1186 from rqlite/better-voter-mgmt

Improve read-only node management

CHANGELOG.md

@@ -2,6 +2,7 @@
 ## Implementation changes and bug fixes
 - [PR #1179](https://github.com/rqlite/rqlite/pull/1179): go mod updates.
 - [PR #1180](https://github.com/rqlite/rqlite/pull/1180): Support large numbers in requests.
+- [PR #1186](https://github.com/rqlite/rqlite/pull/1186): Improve read-only node management. Fixes [issue #1182](https://github.com/rqlite/rqlite/issues/1182).
 
 ## 7.14.1 (March 17th 2023)
 ## Implementation changes and bug fixes

cmd/rqlited/main.go

@@ -367,6 +367,10 @@ func createCluster(cfg *Config, hasPeers bool, str *store.Store,
 
 	joins := cfg.JoinAddresses()
 	if joins == nil && cfg.DiscoMode == "" && !hasPeers {
+		if cfg.RaftNonVoter {
+			return fmt.Errorf("cannot create a new non-voting node without joining it to an existing cluster")
+		}
+
 		// Brand new node, told to bootstrap itself. So do it.
 		log.Println("bootstraping single new node")
 		if err := str.Bootstrap(store.NewServer(str.ID(), cfg.RaftAdv, true)); err != nil {

store/store.go

@@ -511,6 +511,24 @@ func (s *Store) IsLeader() bool {
 	return s.raft.State() == raft.Leader
 }
 
+// IsVoter returns true if the current node is a voter in the cluster.
+func (s *Store) IsVoter() (bool, error) {
+	cfg := s.raft.GetConfiguration()
+	if err := cfg.Error(); err != nil {
+		return false, err
+	}
+	if len(cfg.Configuration().Servers) == 0 {
+		return false, nil
+	}
+
+	for _, srv := range cfg.Configuration().Servers {
+		if srv.ID == raft.ServerID(s.raftID) {
+			return srv.Suffrage == raft.Voter, nil
+		}
+	}
+	return false, fmt.Errorf("this node (id=%s) not found in configuration", s.raftID)
+}
+
 // State returns the current node's Raft state
 func (s *Store) State() ClusterState {
 	state := s.raft.State()
@@ -698,6 +716,10 @@ func (s *Store) Stats() (map[string]interface{}, error) {
 	if err != nil {
 		return nil, err
 	}
+	raftStats["voter"], err = s.IsVoter()
+	if err != nil {
+		return nil, err
+	}
 	raftStats["bolt"] = s.boltStore.Stats()
 
 	dirSz, err := dirSize(s.raftDir)
@@ -1044,7 +1066,6 @@ func (s *Store) Join(id, addr string, voter bool) error {
 	if voter {
 		f = s.raft.AddVoter(raft.ServerID(id), raft.ServerAddress(addr), 0, 0)
 	} else {
-
 		f = s.raft.AddNonvoter(raft.ServerID(id), raft.ServerAddress(addr), 0, 0)
 	}
 	if e := f.(raft.Future); e.Error() != nil {

store/store_test.go

@@ -2075,6 +2075,30 @@ func Test_IsLeader(t *testing.T) {
 	}
 }
 
+func Test_IsVoter(t *testing.T) {
+	s, ln := mustNewStore(t, true)
+	defer ln.Close()
+
+	if err := s.Open(); err != nil {
+		t.Fatalf("failed to open single-node store: %s", err.Error())
+	}
+	defer s.Close(true)
+	if err := s.Bootstrap(NewServer(s.ID(), s.Addr(), true)); err != nil {
+		t.Fatalf("failed to bootstrap single-node store: %s", err.Error())
+	}
+	if _, err := s.WaitForLeader(10 * time.Second); err != nil {
+		t.Fatalf("Error waiting for leader: %s", err)
+	}
+
+	v, err := s.IsVoter()
+	if err != nil {
+		t.Fatalf("failed to check if single node is a voter: %s", err.Error())
+	}
+	if !v {
+		t.Fatalf("single node is not a voter!")
+	}
+}
+
 func Test_State(t *testing.T) {
 	s, ln := mustNewStore(t, true)
 	defer ln.Close()

system_test/cluster_test.go

@@ -927,18 +927,31 @@ func Test_MultiNodeClusterNodesNonVoter(t *testing.T) {
 		t.Fatalf("failed to find cluster leader: %s", err.Error())
 	}
 
-	node3 := mustNewNode(false)
-	defer node3.Deprovision()
-	if err := node3.JoinAsNonVoter(leader); err != nil {
+	nonVoter := mustNewNode(false)
+	defer nonVoter.Deprovision()
+	if err := nonVoter.JoinAsNonVoter(leader); err != nil {
 		t.Fatalf("node failed to join leader: %s", err.Error())
 	}
-	_, err = node3.WaitForLeader()
+	_, err = nonVoter.WaitForLeader()
 	if err != nil {
 		t.Fatalf("failed waiting for leader: %s", err.Error())
 	}
 
+	// Check that the voter statuses are correct
+	checkVoterStatus := func(node *Node, exp bool) {
+		v, err := node.IsVoter()
+		if err != nil {
+			t.Fatalf("failed to get voter status: %s", err.Error())
+		}
+		if v != exp {
+			t.Fatalf("incorrect voter status, got %v, exp %v", v, exp)
+		}
+	}
+	checkVoterStatus(leader, true)
+	checkVoterStatus(nonVoter, false)
+
 	// Get the new leader, in case it changed.
-	c = Cluster{node1, node2, node3}
+	c = Cluster{node1, node2, nonVoter}
 	_, err = c.Leader()
 	if err != nil {
 		t.Fatalf("failed to find cluster leader: %s", err.Error())

system_test/full_system_test.py

@@ -318,10 +318,7 @@ class Node(object):
   def is_leader(self):
     '''
     is_leader returns whether this node is the cluster leader
-    It also performs a check, to ensure the node nevers gives out
-    conflicting information about leader state.
     '''
-
     try:
       return self.status()['store']['raft']['state'] == 'Leader'
     except requests.exceptions.ConnectionError:
@@ -333,6 +330,12 @@ class Node(object):
     except requests.exceptions.ConnectionError:
       return False
 
+  def is_voter(self):
+    try:
+      return self.status()['store']['raft']['voter'] == True
+    except requests.exceptions.ConnectionError:
+      return False
+
   def disco_mode(self):
     try:
       return self.status()['disco']['mode']
@@ -1346,6 +1349,10 @@ class TestEndToEndNonVoter(unittest.TestCase):
   def test_execute_fail_rejoin(self):
     '''Test that a non-voter that fails can rejoin the cluster, and pick up changes'''
 
+    # Confirm that voting status reporting is correct
+    self.assertTrue(self.leader.is_voter())
+    self.assertFalse(self.non_voter.is_voter())
+
     # Insert some records via the leader
     j = self.leader.execute('CREATE TABLE foo (id INTEGER NOT NULL PRIMARY KEY, name TEXT)')
     self.assertEqual(j, d_("{'results': [{}]}"))

system_test/helpers.go

@@ -255,17 +255,46 @@ func (n *Node) Status() (string, error) {
 	if err != nil {
 		return "", err
 	}
-	if resp.StatusCode != 200 {
-		return "", fmt.Errorf("status endpoint returned: %s", resp.Status)
-	}
 	defer resp.Body.Close()
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		return "", err
+		return "", fmt.Errorf("failed to read status response: %w", err)
+	}
+	if resp.StatusCode != 200 {
+		return "", fmt.Errorf("status endpoint returned: %s (%s)", resp.Status,
+			strings.TrimSuffix(string(body), "\n"))
 	}
 	return string(body), nil
 }
 
+// IsVoter returns whether the node is a voter or not.
+func (n *Node) IsVoter() (bool, error) {
+	statusJSON, err := n.Status()
+	if err != nil {
+		return false, err
+	}
+	// Marshal the status into a JSON object
+	var status map[string]interface{}
+	err = json.Unmarshal([]byte(statusJSON), &status)
+	if err != nil {
+		return false, err
+	}
+
+	strStatus, ok := status["store"].(map[string]interface{})
+	if !ok {
+		return false, fmt.Errorf("store status not found")
+	}
+	raftStatus, ok := strStatus["raft"].(map[string]interface{})
+	if !ok {
+		return false, fmt.Errorf("raft status not found")
+	}
+	voter, ok := raftStatus["voter"].(bool)
+	if !ok {
+		return false, fmt.Errorf("voter status not found")
+	}
+	return voter, nil
+}
+
 // Ready returns the ready status for the node
 func (n *Node) Ready() (bool, error) {
 	v, _ := url.Parse("http://" + n.APIAddr + "/readyz")