Refactor stale-read check for testability

store/state.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"github.com/hashicorp/raft"
 	"github.com/rqlite/rqlite/v8/command/chunking"
@@ -15,6 +16,31 @@ import (
 	"github.com/rqlite/rqlite/v8/snapshot"
 )
 
+// IsStaleRead returns whether a read is stale.
+func IsStaleRead(
+	LeaderLastContact time.Time,
+	LastFSMUpdateTime time.Time,
+	LastAppendedAtTime time.Time,
+	FSMIndex uint64,
+	CommitIndex uint64,
+	Freshness int64,
+	MaxStale int64,
+) bool {
+	if Freshness == 0 {
+		return false
+	}
+	if time.Since(LeaderLastContact).Nanoseconds() > Freshness {
+		return true
+	}
+	if MaxStale == 0 {
+		return false
+	}
+	if FSMIndex == CommitIndex {
+		return false
+	}
+	return LastFSMUpdateTime.Sub(LastAppendedAtTime).Nanoseconds() > MaxStale
+}
+
 // IsNewNode returns whether a node using raftDir would be a brand-new node.
 // It also means that the window for this node joining a different cluster has passed.
 func IsNewNode(raftDir string) bool {

store/state_test.go

@@ -9,6 +9,94 @@ import (
 	"github.com/rqlite/rqlite/v8/command/proto"
 )
 
+func Test_IsStaleRead(t *testing.T) {
+	tests := []struct {
+		Name               string
+		LeaderLastContact  time.Time
+		LastFSMUpdateTime  time.Time
+		LastAppendedAtTime time.Time
+		FSMIndex           uint64
+		CommitIndex        uint64
+		Freshness          int64
+		MaxStale           int64
+		Exp                bool
+	}{
+		{
+			Name:      "no freshness set",
+			Freshness: 0,
+			Exp:       false,
+		},
+		{
+			Name:              "freshness set, but not exceeded",
+			LeaderLastContact: time.Now().Add(-1 * time.Second),
+			Freshness:         time.Minute.Nanoseconds(),
+			Exp:               false,
+		},
+		{
+			Name:              "freshness set and exceeded",
+			LeaderLastContact: time.Now().Add(-10 * time.Second),
+			Freshness:         time.Second.Nanoseconds(),
+			Exp:               true,
+		},
+		{
+			Name:               "freshness set, is ok, max stale exceeded, but applied index is up-to-date",
+			LeaderLastContact:  time.Now().Add(-1 * time.Second),
+			LastFSMUpdateTime:  time.Now().Add(-1 * time.Second),
+			LastAppendedAtTime: time.Now().Add(-10 * time.Second),
+			FSMIndex:           10,
+			CommitIndex:        10,
+			Freshness:          time.Minute.Nanoseconds(),
+			MaxStale:           time.Second.Nanoseconds(),
+			Exp:                false,
+		},
+		{
+			Name:               "freshness set, is ok, max stale exceeded, and applied index is behind",
+			LeaderLastContact:  time.Now().Add(-1 * time.Second),
+			LastFSMUpdateTime:  time.Now().Add(-1 * time.Second),
+			LastAppendedAtTime: time.Now().Add(-10 * time.Second),
+			FSMIndex:           9,
+			CommitIndex:        10,
+			Freshness:          time.Minute.Nanoseconds(),
+			MaxStale:           time.Second.Nanoseconds(),
+			Exp:                true,
+		},
+		{
+			Name:               "freshness set, is ok, max stale not exceeded, but applied index is behind",
+			LeaderLastContact:  time.Now().Add(-1 * time.Second),
+			LastFSMUpdateTime:  time.Now().Add(-1 * time.Second),
+			LastAppendedAtTime: time.Now().Add(-10 * time.Second),
+			FSMIndex:           9,
+			CommitIndex:        10,
+			Freshness:          time.Minute.Nanoseconds(),
+			MaxStale:           time.Minute.Nanoseconds(),
+			Exp:                false,
+		},
+		{
+			Name:               "freshness set, is ok, applied index is behind, but max stale not set",
+			LeaderLastContact:  time.Now().Add(-1 * time.Second),
+			LastFSMUpdateTime:  time.Now().Add(-1 * time.Second),
+			LastAppendedAtTime: time.Now().Add(-10 * time.Second),
+			FSMIndex:           9,
+			CommitIndex:        10,
+			Freshness:          time.Minute.Nanoseconds(),
+			Exp:                false,
+		},
+	}
+
+	for _, tt := range tests {
+		if got, exp := IsStaleRead(
+			tt.LeaderLastContact,
+			tt.LastFSMUpdateTime,
+			tt.LastAppendedAtTime,
+			tt.FSMIndex,
+			tt.CommitIndex,
+			tt.Freshness,
+			tt.MaxStale), tt.Exp; got != exp {
+			t.Fatalf("unexpected result for IsStaleRead test %s\nexp: %v\ngot: %v", tt.Name, exp, got)
+		}
+	}
+}
+
 func Test_Store_IsNewNode(t *testing.T) {
 	s, ln := mustNewStore(t)
 	defer ln.Close()

store/store.go

@@ -1791,19 +1791,17 @@ func (s *Store) updateAppliedIndex() chan struct{} {
 }
 
 func (s *Store) isStaleRead(freshness, maxStale int64) bool {
-	if freshness == 0 || s.raft.State() == raft.Leader {
+	if s.raft.State() == raft.Leader {
 		return false
 	}
-	if time.Since(s.raft.LastContact()).Nanoseconds() > freshness {
-		return true
-	}
-	if maxStale == 0 {
-		return false
-	}
-	if s.fsmIdx.Load() == s.raft.CommitIndex() {
-		return false
-	}
-	return s.fsmUpdateTime.Sub(s.appendedAtTime).Nanoseconds() > maxStale
+	return IsStaleRead(
+		s.raft.LastContact(),
+		s.fsmUpdateTime.Load(),
+		s.appendedAtTime.Load(),
+		s.fsmIdx.Load(),
+		s.raft.CommitIndex(),
+		freshness,
+		maxStale)
 }
 
 type fsmExecuteResponse struct {