Merge cockroachdb#129943

129943: raft: enforce invariant matchCommit <= sentCommit r=iskettaneh a=iskettaneh This commit ensures that the matchCommit is always less than or equal to the sentCommit. This avoids sending an extra empty MsgApp. Moreover, it adds SentCommit and MatchCommit to Progress.String() to be explicit in the tests. Epic: None Release note: None Co-authored-by: Ibrahim Kettaneh <[email protected]>
cthumuluru-crdb · Sep 6, 2024 · 8c9f423 · 8c9f423
2 parents 9d48d94 + dbdb651
commit 8c9f423
Show file tree

Hide file tree

Showing 29 changed files with 247 additions and 218 deletions.
diff --git a/pkg/kv/kvserver/split_delay_helper_test.go b/pkg/kv/kvserver/split_delay_helper_test.go
@@ -155,7 +155,7 @@ func TestSplitDelayToAvoidSnapshot(t *testing.T) {
 			}
 			s := maybeDelaySplitToAvoidSnapshot(ctx, h)
 			assert.EqualValues(t, "; delayed by 5.5s to resolve: replica r1/2 not caught up: "+
-				state.String()+" match=0 next=0 paused (without success)", s)
+				state.String()+" match=0 next=0 sentCommit=0 matchCommit=0 paused (without success)", s)
 		})
 	}
 
@@ -193,6 +193,7 @@ func TestSplitDelayToAvoidSnapshot(t *testing.T) {
 			}
 		}
 		s := maybeDelaySplitToAvoidSnapshot(ctx, h)
-		assert.EqualValues(t, "; delayed by 2.5s to resolve: replica r1/2 not caught up: StateProbe match=0 next=0", s)
+		assert.EqualValues(t, "; delayed by 2.5s to resolve: replica r1/2 not caught up: "+
+			"StateProbe match=0 next=0 sentCommit=0 matchCommit=0", s)
 	})
 }
diff --git a/pkg/raft/confchange/confchange.go b/pkg/raft/confchange/confchange.go
@@ -271,10 +271,11 @@ func (c Changer) initProgress(
 		// at all (and will thus likely need a snapshot), though the app may
 		// have applied a snapshot out of band before adding the replica (thus
 		// making the first index the better choice).
-		Match:     0,
-		Next:      max(c.LastIndex, 1), // invariant: Match < Next
-		Inflights: tracker.NewInflights(c.MaxInflight, c.MaxInflightBytes),
-		IsLearner: isLearner,
+		Match:       0,
+		MatchCommit: 0,
+		Next:        max(c.LastIndex, 1), // invariant: Match < Next
+		Inflights:   tracker.NewInflights(c.MaxInflight, c.MaxInflightBytes),
+		IsLearner:   isLearner,
 		// When a node is first added, we should mark it as recently active.
 		// Otherwise, CheckQuorum may cause us to step down if it is invoked
 		// before the added node has had a chance to communicate with us.

diff --git a/pkg/raft/confchange/testdata/joint_autoleave.txt b/pkg/raft/confchange/testdata/joint_autoleave.txt
@@ -5,16 +5,16 @@ simple
 v1
 ----
 voters=(1)
-1: StateProbe match=0 next=1
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
 
 # Autoleave is reflected in the config.
 enter-joint autoleave=true
 v2 v3
 ----
 voters=(1 2 3)&&(1) autoleave
-1: StateProbe match=0 next=1
-2: StateProbe match=0 next=1
-3: StateProbe match=0 next=1
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+2: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+3: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
 
 # Can't enter-joint twice, even if autoleave changes.
 enter-joint autoleave=false
@@ -24,6 +24,6 @@ config is already joint
 leave-joint
 ----
 voters=(1 2 3)
-1: StateProbe match=0 next=1
-2: StateProbe match=0 next=1
-3: StateProbe match=0 next=1
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+2: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+3: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
diff --git a/pkg/raft/confchange/testdata/joint_idempotency.txt b/pkg/raft/confchange/testdata/joint_idempotency.txt
@@ -5,19 +5,19 @@ simple
 v1
 ----
 voters=(1)
-1: StateProbe match=0 next=1
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
 
 enter-joint
 r1 r2 r9 v2 v3 v4 v2 v3 v4 l2 l2 r4 r4 l1 l1
 ----
 voters=(3)&&(1) learners=(2) learners_next=(1)
-1: StateProbe match=0 next=1
-2: StateProbe match=0 next=1 learner
-3: StateProbe match=0 next=1
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+2: StateProbe match=0 next=1 sentCommit=0 matchCommit=0 learner
+3: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
 
 leave-joint
 ----
 voters=(3) learners=(1 2)
-1: StateProbe match=0 next=1 learner
-2: StateProbe match=0 next=1 learner
-3: StateProbe match=0 next=1
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0 learner
+2: StateProbe match=0 next=1 sentCommit=0 matchCommit=0 learner
+3: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
diff --git a/pkg/raft/confchange/testdata/joint_learners_next.txt b/pkg/raft/confchange/testdata/joint_learners_next.txt
@@ -8,17 +8,17 @@ simple
 v1
 ----
 voters=(1)
-1: StateProbe match=0 next=1
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
 
 enter-joint
 v2 l1
 ----
 voters=(2)&&(1) learners_next=(1)
-1: StateProbe match=0 next=1
-2: StateProbe match=0 next=1
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+2: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
 
 leave-joint
 ----
 voters=(2) learners=(1)
-1: StateProbe match=0 next=1 learner
-2: StateProbe match=0 next=1
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0 learner
+2: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
diff --git a/pkg/raft/confchange/testdata/joint_safety.txt b/pkg/raft/confchange/testdata/joint_safety.txt
@@ -15,7 +15,7 @@ simple
 v1
 ----
 voters=(1)
-1: StateProbe match=0 next=3
+1: StateProbe match=0 next=3 sentCommit=0 matchCommit=0
 
 leave-joint
 ----
@@ -25,7 +25,7 @@ can't leave a non-joint config
 enter-joint
 ----
 voters=(1)&&(1)
-1: StateProbe match=0 next=3
+1: StateProbe match=0 next=3 sentCommit=0 matchCommit=0
 
 enter-joint
 ----
@@ -34,7 +34,7 @@ config is already joint
 leave-joint
 ----
 voters=(1)
-1: StateProbe match=0 next=3
+1: StateProbe match=0 next=3 sentCommit=0 matchCommit=0
 
 leave-joint
 ----
@@ -45,10 +45,10 @@ enter-joint
 r1 v2 v3 l4
 ----
 voters=(2 3)&&(1) learners=(4)
-1: StateProbe match=0 next=3
-2: StateProbe match=0 next=9
-3: StateProbe match=0 next=9
-4: StateProbe match=0 next=9 learner
+1: StateProbe match=0 next=3 sentCommit=0 matchCommit=0
+2: StateProbe match=0 next=9 sentCommit=0 matchCommit=0
+3: StateProbe match=0 next=9 sentCommit=0 matchCommit=0
+4: StateProbe match=0 next=9 sentCommit=0 matchCommit=0 learner
 
 enter-joint
 ----
@@ -67,15 +67,15 @@ can't apply simple config change in joint config
 leave-joint
 ----
 voters=(2 3) learners=(4)
-2: StateProbe match=0 next=9
-3: StateProbe match=0 next=9
-4: StateProbe match=0 next=9 learner
+2: StateProbe match=0 next=9 sentCommit=0 matchCommit=0
+3: StateProbe match=0 next=9 sentCommit=0 matchCommit=0
+4: StateProbe match=0 next=9 sentCommit=0 matchCommit=0 learner
 
 simple
 l9
 ----
 voters=(2 3) learners=(4 9)
-2: StateProbe match=0 next=9
-3: StateProbe match=0 next=9
-4: StateProbe match=0 next=9 learner
-9: StateProbe match=0 next=14 learner
+2: StateProbe match=0 next=9 sentCommit=0 matchCommit=0
+3: StateProbe match=0 next=9 sentCommit=0 matchCommit=0
+4: StateProbe match=0 next=9 sentCommit=0 matchCommit=0 learner
+9: StateProbe match=0 next=14 sentCommit=0 matchCommit=0 learner
diff --git a/pkg/raft/confchange/testdata/simple_idempotency.txt b/pkg/raft/confchange/testdata/simple_idempotency.txt
@@ -2,68 +2,68 @@ simple
 v1
 ----
 voters=(1)
-1: StateProbe match=0 next=1
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
 
 simple
 v1
 ----
 voters=(1)
-1: StateProbe match=0 next=1
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
 
 simple
 v2
 ----
 voters=(1 2)
-1: StateProbe match=0 next=1
-2: StateProbe match=0 next=2
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+2: StateProbe match=0 next=2 sentCommit=0 matchCommit=0
 
 simple
 l1
 ----
 voters=(2) learners=(1)
-1: StateProbe match=0 next=1 learner
-2: StateProbe match=0 next=2
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0 learner
+2: StateProbe match=0 next=2 sentCommit=0 matchCommit=0
 
 simple
 l1
 ----
 voters=(2) learners=(1)
-1: StateProbe match=0 next=1 learner
-2: StateProbe match=0 next=2
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0 learner
+2: StateProbe match=0 next=2 sentCommit=0 matchCommit=0
 
 simple
 r1
 ----
 voters=(2)
-2: StateProbe match=0 next=2
+2: StateProbe match=0 next=2 sentCommit=0 matchCommit=0
 
 simple
 r1
 ----
 voters=(2)
-2: StateProbe match=0 next=2
+2: StateProbe match=0 next=2 sentCommit=0 matchCommit=0
 
 simple
 v3
 ----
 voters=(2 3)
-2: StateProbe match=0 next=2
-3: StateProbe match=0 next=7
+2: StateProbe match=0 next=2 sentCommit=0 matchCommit=0
+3: StateProbe match=0 next=7 sentCommit=0 matchCommit=0
 
 simple
 r3
 ----
 voters=(2)
-2: StateProbe match=0 next=2
+2: StateProbe match=0 next=2 sentCommit=0 matchCommit=0
 
 simple
 r3
 ----
 voters=(2)
-2: StateProbe match=0 next=2
+2: StateProbe match=0 next=2 sentCommit=0 matchCommit=0
 
 simple
 r4
 ----
 voters=(2)
-2: StateProbe match=0 next=2
+2: StateProbe match=0 next=2 sentCommit=0 matchCommit=0
diff --git a/pkg/raft/confchange/testdata/simple_promote_demote.txt b/pkg/raft/confchange/testdata/simple_promote_demote.txt
@@ -4,57 +4,57 @@ simple
 v1
 ----
 voters=(1)
-1: StateProbe match=0 next=1
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
 
 simple
 v2
 ----
 voters=(1 2)
-1: StateProbe match=0 next=1
-2: StateProbe match=0 next=1
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+2: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
 
 simple
 v3
 ----
 voters=(1 2 3)
-1: StateProbe match=0 next=1
-2: StateProbe match=0 next=1
-3: StateProbe match=0 next=2
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+2: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+3: StateProbe match=0 next=2 sentCommit=0 matchCommit=0
 
 # Can atomically demote and promote without a hitch.
 # This is pointless, but possible.
 simple
 l1 v1
 ----
 voters=(1 2 3)
-1: StateProbe match=0 next=1
-2: StateProbe match=0 next=1
-3: StateProbe match=0 next=2
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+2: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+3: StateProbe match=0 next=2 sentCommit=0 matchCommit=0
 
 # Can demote a voter.
 simple
 l2
 ----
 voters=(1 3) learners=(2)
-1: StateProbe match=0 next=1
-2: StateProbe match=0 next=1 learner
-3: StateProbe match=0 next=2
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+2: StateProbe match=0 next=1 sentCommit=0 matchCommit=0 learner
+3: StateProbe match=0 next=2 sentCommit=0 matchCommit=0
 
 # Can atomically promote and demote the same voter.
 # This is pointless, but possible.
 simple
 v2 l2
 ----
 voters=(1 3) learners=(2)
-1: StateProbe match=0 next=1
-2: StateProbe match=0 next=1 learner
-3: StateProbe match=0 next=2
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+2: StateProbe match=0 next=1 sentCommit=0 matchCommit=0 learner
+3: StateProbe match=0 next=2 sentCommit=0 matchCommit=0
 
 # Can promote a voter.
 simple
 v2
 ----
 voters=(1 2 3)
-1: StateProbe match=0 next=1
-2: StateProbe match=0 next=1
-3: StateProbe match=0 next=2
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+2: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+3: StateProbe match=0 next=2 sentCommit=0 matchCommit=0
diff --git a/pkg/raft/confchange/testdata/simple_safety.txt b/pkg/raft/confchange/testdata/simple_safety.txt
@@ -7,15 +7,15 @@ simple
 v1
 ----
 voters=(1)
-1: StateProbe match=0 next=1
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
 
 simple
 v2 l3
 ----
 voters=(1 2) learners=(3)
-1: StateProbe match=0 next=1
-2: StateProbe match=0 next=2
-3: StateProbe match=0 next=2 learner
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+2: StateProbe match=0 next=2 sentCommit=0 matchCommit=0
+3: StateProbe match=0 next=2 sentCommit=0 matchCommit=0 learner
 
 simple
 r1 v5
@@ -46,11 +46,11 @@ simple
 l2 l3 l4 l5
 ----
 voters=(1) learners=(2 3 4 5)
-1: StateProbe match=0 next=1
-2: StateProbe match=0 next=2 learner
-3: StateProbe match=0 next=2 learner
-4: StateProbe match=0 next=8 learner
-5: StateProbe match=0 next=8 learner
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0
+2: StateProbe match=0 next=2 sentCommit=0 matchCommit=0 learner
+3: StateProbe match=0 next=2 sentCommit=0 matchCommit=0 learner
+4: StateProbe match=0 next=8 sentCommit=0 matchCommit=0 learner
+5: StateProbe match=0 next=8 sentCommit=0 matchCommit=0 learner
 
 simple
 r1
@@ -61,4 +61,4 @@ simple
 r2 r3 r4 r5
 ----
 voters=(1)
-1: StateProbe match=0 next=1
+1: StateProbe match=0 next=1 sentCommit=0 matchCommit=0