diff --git a/CHANGELOG.md b/CHANGELOG.md index 719ec6f6..28350ace 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ - [PR #1522](https://github.com/rqlite/rqlite/pull/1522): Minor refactoring of main module. - [PR #1523](https://github.com/rqlite/rqlite/pull/1523): Move download functionality into _restore_ module. - [PR #1524](https://github.com/rqlite/rqlite/pull/1524): Disco mode not supported when explicitly joining. +- [PR #1525](https://github.com/rqlite/rqlite/pull/1525): Make Store _Notify_ logic clearer. ## 8.13.4 (December 23rd 2023) This release makes sure the version information is correctly recorded in the released binaries. There are no functional changes. diff --git a/store/store.go b/store/store.go index 25082d5a..5cb94dab 100644 --- a/store/store.go +++ b/store/store.go @@ -675,6 +675,11 @@ func (s *Store) IsLeader() bool { return s.raft.State() == raft.Leader } +// HasLeader returns true if the cluster has a leader, false otherwise. +func (s *Store) HasLeader() bool { + return s.raft.Leader() != "" +} + // IsVoter returns true if the current node is a voter in the cluster. If there // is no reference to the current node in the current cluster configuration then // false will also be returned. @@ -1352,8 +1357,13 @@ func (s *Store) Notify(nr *command.NotifyRequest) error { s.notifyMu.Lock() defer s.notifyMu.Unlock() - if s.BootstrapExpect == 0 || s.bootstrapped || s.raft.Leader() != "" { + if s.BootstrapExpect == 0 || s.bootstrapped || s.HasLeader() { // There is no reason this node will bootstrap. + // + // - Read-only nodes require that BootstrapExpect is set to 0, so this + // block ensures that notifying a read-only node will not cause a bootstrap. + // - If the node is already bootstrapped, then there is nothing to do. + // - If the node already has a leader, then no bootstrapping is required. return nil } diff --git a/store/store_test.go b/store/store_test.go index 67504c07..dd3f7b16 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -2627,6 +2627,56 @@ func Test_IsLeader(t *testing.T) { if !s.IsLeader() { t.Fatalf("single node is not leader!") } + if !s.HasLeader() { + t.Fatalf("single node does not have leader!") + } +} + +func Test_MultiNodeIsLeaderHasLeader(t *testing.T) { + s0, ln0 := mustNewStore(t) + defer ln0.Close() + if err := s0.Open(); err != nil { + t.Fatalf("failed to open single-node store: %s", err.Error()) + } + defer s0.Close(true) + if err := s0.Bootstrap(NewServer(s0.ID(), s0.Addr(), true)); err != nil { + t.Fatalf("failed to bootstrap single-node store: %s", err.Error()) + } + if _, err := s0.WaitForLeader(10 * time.Second); err != nil { + t.Fatalf("Error waiting for leader: %s", err) + } + + s1, ln1 := mustNewStore(t) + defer ln1.Close() + if err := s1.Open(); err != nil { + t.Fatalf("failed to open single-node store: %s", err.Error()) + } + defer s1.Close(true) + if err := s1.Bootstrap(NewServer(s1.ID(), s1.Addr(), true)); err != nil { + t.Fatalf("failed to bootstrap single-node store: %s", err.Error()) + } + + // Join the second node to the first. + if err := s0.Join(joinRequest(s1.ID(), s1.Addr(), true)); err != nil { + t.Fatalf("failed to join to node at %s: %s", s0.Addr(), err.Error()) + } + _, err := s1.WaitForLeader(10 * time.Second) + if err != nil { + t.Fatalf("failed to get leader address on follower: %s", err.Error()) + } + + if !s0.IsLeader() { + t.Fatalf("s0 is not leader") + } + if !s0.HasLeader() { + t.Fatalf("s0 does not have a leader") + } + if s1.IsLeader() { + t.Fatalf("s1 is leader") + } + if !s1.HasLeader() { + t.Fatalf("s1 does not have a leader") + } } func Test_IsVoter(t *testing.T) {