From ed7ce918192ffa4da084e003f1c931bcc3b445ac Mon Sep 17 00:00:00 2001
From: Ian Gulliver <github@flamingcow.io>
Date: Sun, 4 Jun 2023 13:59:06 -0700
Subject: [PATCH] Clear time separation for loss before gain

---
 candidate.go  | 44 +++++++++++++++++++++++++++++++++++---------
 elect_test.go | 12 ++++++------
 voter.go      |  5 ++---
 3 files changed, 43 insertions(+), 18 deletions(-)

diff --git a/candidate.go b/candidate.go
index 1880189..36e512a 100644
--- a/candidate.go
+++ b/candidate.go
@@ -23,9 +23,10 @@ type Candidate struct {
 	resp       voteResponse
 	c          chan<- CandidateState
 
-	votes map[string]*vote
-	state CandidateState
-	mu    sync.Mutex
+	votes    map[string]*vote
+	state    CandidateState
+	firstYes time.Time
+	mu       sync.Mutex
 }
 
 type CandidateState string
@@ -35,6 +36,14 @@ var (
 	StateNotLeader CandidateState = "NOT_LEADER"
 )
 
+const (
+	maxVotePeriod  = 5 * time.Second
+	voteTimeout    = 10 * time.Second
+	leadershipWait = 15 * time.Second
+
+	maxFastVotePeriod = 100 * time.Millisecond
+)
+
 func NewCandidate(numVoters int, signingKey string) *Candidate {
 	change := make(chan CandidateState, 100)
 
@@ -200,30 +209,47 @@ func (c *Candidate) elect() {
 	no := 0
 	yes := 0
 
-	cutoff := time.Now().Add(-10 * time.Second)
-
 	for key, vote := range c.votes {
-		if vote.received.Before(cutoff) {
+		if time.Since(vote.received) > voteTimeout {
+			// Remove stale vote from consideration
 			delete(c.votes, key)
 			continue
 		}
 
 		if vote.LastSeenCandidateID != c.resp.CandidateID {
+			// Hard no; voted for someone else
 			no++
 		}
 
 		if vote.NumPollsSinceChange < 10 {
+			// Soft no; voted for us but not enough times in a row
 			continue
 		}
 
 		yes++
 	}
 
-	if no == 0 && yes > c.numVoters/2 {
-		c.update(StateLeader)
-	} else {
+	if no > 0 || yes <= c.numVoters/2 {
+		// We lost the vote
+		c.firstYes = time.Time{}
 		c.update(StateNotLeader)
+
+		return
 	}
+
+	if c.firstYes.IsZero() {
+		// First round of "yes" voting since the last "no" vote
+		c.firstYes = time.Now()
+	}
+
+	if time.Since(c.firstYes) < leadershipWait {
+		// Not enough time in "yes" state
+		c.update(StateNotLeader)
+		return
+	}
+
+	// All checks passed
+	c.update(StateLeader)
 }
 
 func (c *Candidate) update(state CandidateState) {
diff --git a/elect_test.go b/elect_test.go
index 152ead2..859a98e 100644
--- a/elect_test.go
+++ b/elect_test.go
@@ -13,7 +13,7 @@ func TestOne(t *testing.T) {
 	ts := NewTestSystem(t, 1)
 	defer ts.Stop()
 
-	require.Eventually(t, ts.Candidate(0).IsLeader, 15*time.Second, 100*time.Millisecond)
+	require.Eventually(t, ts.Candidate(0).IsLeader, 20*time.Second, 100*time.Millisecond)
 }
 
 func TestThree(t *testing.T) {
@@ -22,7 +22,7 @@ func TestThree(t *testing.T) {
 	ts := NewTestSystem(t, 3)
 	defer ts.Stop()
 
-	require.Eventually(t, ts.Candidate(0).IsLeader, 15*time.Second, 100*time.Millisecond)
+	require.Eventually(t, ts.Candidate(0).IsLeader, 20*time.Second, 100*time.Millisecond)
 }
 
 func TestFailover(t *testing.T) {
@@ -31,14 +31,14 @@ func TestFailover(t *testing.T) {
 	ts := NewTestSystem(t, 3)
 	defer ts.Stop()
 
-	require.Eventually(t, ts.Candidate(0).IsLeader, 15*time.Second, 100*time.Millisecond)
+	require.Eventually(t, ts.Candidate(0).IsLeader, 20*time.Second, 100*time.Millisecond)
 
 	ts.SetServer(1)
 
 	require.Eventually(t, func() bool { return !ts.Candidate(0).IsLeader() }, 15*time.Second, 100*time.Millisecond)
 
-	// TODO: Check that new candidate doesn't become leader before old candidate loses it
-	// require.False(t, ts.Candidate(1).IsLeader())
+	// New candidate must not get leadership before old candidate loses it
+	require.False(t, ts.Candidate(1).IsLeader())
 
-	require.Eventually(t, ts.Candidate(1).IsLeader, 15*time.Second, 100*time.Millisecond)
+	require.Eventually(t, ts.Candidate(1).IsLeader, 20*time.Second, 100*time.Millisecond)
 }
diff --git a/voter.go b/voter.go
index 1eba2c9..5019a7a 100644
--- a/voter.go
+++ b/voter.go
@@ -52,7 +52,7 @@ func NewVoter(url string, signingKey string, candidate *Candidate) *Voter {
 		vote: vote{
 			VoterID: uniuri.New(),
 		},
-		period: 5 * time.Second,
+		period: maxVotePeriod,
 	}
 
 	go v.loop()
@@ -82,8 +82,7 @@ func (v *Voter) poll() bool {
 	t2 := &time.Timer{}
 
 	if v.vote.NumPollsSinceChange <= 10 {
-		// mean: 100ms, max: 200ms
-		t2 = time.NewTimer(randDurationN(100 * time.Millisecond))
+		t2 = time.NewTimer(randDurationN(maxFastVotePeriod))
 		defer t2.Stop()
 	}