Skip to content

Commit e35b433

Browse files
committed
fix: noop
1 parent 9404752 commit e35b433

File tree

2 files changed

+102
-2
lines changed

2 files changed

+102
-2
lines changed

fast_raft_e2e_test.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,3 +1040,87 @@ func TestE2E_Reconfig_RemoveNode_Contention_QuorumsSwitchAfterApply(t *testing.T
10401040
t.Fatalf("data did not commit with 2/3 after remove(4) applied")
10411041
}
10421042
}
1043+
1044+
func TestLeaderFallbackRunsWhenKHasUncommittedLeaderNoop(t *testing.T) {
1045+
// Build a single-node leader with fast path on.
1046+
cfg := baseConfigFast(1) // or baseConfigFast if you have it
1047+
cfg.EnableFastPath = true
1048+
rl := newRaft(cfg)
1049+
primeSingleVoter(rl, rl.id)
1050+
1051+
rl.becomeCandidate()
1052+
rl.becomeLeader()
1053+
1054+
// Build a committed prefix: committed = 5
1055+
mustAppendCommitted(rl, 5)
1056+
1057+
// Now append a leader-approved noop at k = committed+1 (index 6).
1058+
// This simulates the upstream leader's noop at term start.
1059+
if !rl.appendEntry(pb.Entry{Data: nil}) {
1060+
t.Fatalf("append noop failed")
1061+
}
1062+
noopIdx := rl.raftLog.lastIndex() // should be 6
1063+
if noopIdx != rl.raftLog.committed+1 {
1064+
t.Fatalf("noop not at k; noopIdx=%d committed=%d", noopIdx, rl.raftLog.committed)
1065+
}
1066+
// Sanity: hasLeaderApprovedAt(k) should be true for the noop.
1067+
if !rl.hasLeaderApprovedAt(noopIdx) {
1068+
t.Fatalf("expected leader-approved noop at k=%d", noopIdx)
1069+
}
1070+
1071+
// Pre-cache the leader payload at leader's current k so the decision
1072+
// will use *our* bytes (important for etcd waiter).
1073+
rl.CacheLeaderFastPayload([]byte("leader-payload"), []byte("cid-x"))
1074+
1075+
// Record state before attempting fallback.
1076+
lastIdxBefore := rl.raftLog.lastIndex()
1077+
1078+
// Send the leader's own fast-prop (From=None → normalized to leader).
1079+
msg := pb.Message{
1080+
Type: pb.MsgFastProp,
1081+
From: None, // local
1082+
Index: 0, // ignored by leader
1083+
Entries: []pb.Entry{{
1084+
Type: pb.EntryNormal,
1085+
Data: []byte("leader-payload"),
1086+
ContentId: []byte("cid-x"),
1087+
}},
1088+
}
1089+
if err := rl.Step(msg); err != nil {
1090+
t.Fatalf("leader step: %v", err)
1091+
}
1092+
1093+
// After the fix:
1094+
// - fallback should *run*, i.e. append the decision (either at k, replacing noop,
1095+
// or at k+1 if your fallback still uses appendEntry), so lastIndex must increase by 1.
1096+
// - proposalCache[k] should be cleared.
1097+
// - fastMatchIndex[leader] should count k.
1098+
lastIdxAfter := rl.raftLog.lastIndex()
1099+
if lastIdxAfter != lastIdxBefore+1 {
1100+
t.Fatalf("fallback did not append decision; lastIdx before=%d after=%d", lastIdxBefore, lastIdxAfter)
1101+
}
1102+
1103+
// proposalCache for k should be cleared after install.
1104+
if rl.proposalCache[noopIdx] != nil {
1105+
t.Fatalf("proposalCache[%d] not cleared", noopIdx)
1106+
}
1107+
1108+
// Leader should count itself at k toward fast quorum.
1109+
if rl.fastMatchIndex[rl.id] < noopIdx {
1110+
t.Fatalf("fastMatchIndex[self]=%d want >= %d", rl.fastMatchIndex[rl.id], noopIdx)
1111+
}
1112+
1113+
// Optional: if your fallback installs *exactly* at k (ideal), validate it replaced the noop.
1114+
ents, _ := rl.raftLog.slice(noopIdx, noopIdx+1, noLimit)
1115+
if len(ents) == 1 && len(ents[0].Data) != 0 {
1116+
if string(ents[0].Data) != "leader-payload" || getOrigin(&ents[0]) != pb.EntryOriginLeader {
1117+
t.Fatalf("entry at k not leader-approved payload: %+v", ents[0])
1118+
}
1119+
} else {
1120+
// If you still append at k+1, ensure the leader payload is at lastIdxAfter.
1121+
ents2, _ := rl.raftLog.slice(lastIdxAfter, lastIdxAfter+1, noLimit)
1122+
if len(ents2) != 1 || string(ents2[0].Data) != "leader-payload" || getOrigin(&ents2[0]) != pb.EntryOriginLeader {
1123+
t.Fatalf("leader decision not appended properly: %+v", ents2)
1124+
}
1125+
}
1126+
}

raft.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,23 @@ func (r *raft) CacheLeaderFastPayload(data, cid []byte) {
575575
}
576576
}
577577

578+
func (r *raft) leaderEntryAtKIsNoop(k uint64) bool {
579+
if k == 0 || k > r.raftLog.lastIndex() {
580+
return false
581+
}
582+
ents, err := r.raftLog.slice(k, k+1, noLimit)
583+
if err != nil || len(ents) != 1 {
584+
return false
585+
}
586+
e := ents[0]
587+
// leader-origin (or legacy ‘Unknown’ treated as leader-origin)
588+
if getOrigin(&e) != pb.EntryOriginLeader && getOrigin(&e) != pb.EntryOriginUnknown {
589+
return false
590+
}
591+
// a true noop/placeholder: no Data and no ContentId
592+
return len(e.Data) == 0 && len(e.ContentId) == 0
593+
}
594+
578595
func (r *raft) recomputeLastLeaderIndex() {
579596
fi := r.raftLog.firstIndex()
580597
li := r.raftLog.lastIndex()
@@ -1832,8 +1849,7 @@ func stepLeader(r *raft, m pb.Message) error {
18321849
}
18331850

18341851
// Classic fallback only on OUR own fast-prop for k.
1835-
if from == r.id && !r.hasLeaderApprovedAt(k) {
1836-
// Materialize leader-approved entry from the cached payload.
1852+
if from == r.id && (!r.hasLeaderApprovedAt(k) || r.leaderEntryAtKIsNoop(k)) { // Materialize leader-approved entry from the cached payload.
18371853
chosen := bucket[cid]
18381854
setOrigin(&chosen, pb.EntryOriginLeader) // leader canon
18391855
chosen.Index = 0 // appendEntry sets Index/Term

0 commit comments

Comments
 (0)