Skip to content

Commit a8e2cf5

Browse files
committed
fix(kv/fsm): kvFSM directly satisfies raftengine.AppliedIndexReader (codex P2)
Codex round-4 P2 on kv/fsm.go:145 caught a real wiring bug: the round-1 method was named AppliedIndexReader() returning the interface (factory pattern), intended to be called through a separate AppliedIndexReporter interface. But that AppliedIndexReporter was never added to the raftengine package, and the planned Branch 3 cold-start skip gate type-asserts fsm.(raftengine.AppliedIndexReader) directly. A factory method with a different signature does NOT satisfy the interface, so the skip would always fall back to full restore even after meta keys are populated on every apply. Fix: rename kvFSM.AppliedIndexReader() to LastAppliedIndex() and inline the type-assert forward. kvFSM now directly satisfies raftengine.AppliedIndexReader. Added kv/fsm_applied_index_iface_check.go with compile-time guards: var _ raftengine.AppliedIndexReader = (*kvFSM)(nil) var _ raftengine.AppliedIndexWriter = (*kvFSM)(nil) so a future rename or signature drift fails at build time rather than silently degrading the skip optimisation to full-restore-always. No production callers of the old AppliedIndexReader() factory exist (verified by grep) — the consumer is the future Branch 3 skip gate which is not yet wired. Safe rename. Tests: go vet + golangci-lint clean across all touched packages; go test ./kv/ ./store/ ./internal/raftengine/... -short ok 56s. Refs PR #915 round-4 codex P2 (kv/fsm.go:145). Codex P2 on engine.go:4077 (snapshot artifact does not contain post-bump metaAppliedIndex) is acknowledged but out of B2 scope — push-back explanation in the PR comment.
1 parent 5684857 commit a8e2cf5

2 files changed

Lines changed: 38 additions & 12 deletions

File tree

kv/fsm.go

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -129,19 +129,36 @@ func (f *kvFSM) SetApplyIndex(idx uint64) {
129129
f.pendingApplyIdx = idx
130130
}
131131

132-
// AppliedIndexReader implements raftengine.AppliedIndexReporter. It
133-
// exposes the underlying store's durable applied-index when the
134-
// store implements raftengine.AppliedIndexReader (pebbleStore does;
135-
// the in-memory mvccStore does not, in which case (0, false, nil)
136-
// from the missing-key path will land at the caller). nil means
137-
// "not supported on this backend" and triggers the conservative
138-
// full-restore fallback at the cold-start skip gate. See
139-
// docs/design/2026_06_02_idempotent_snapshot_restore.md §3.
140-
func (f *kvFSM) AppliedIndexReader() raftengine.AppliedIndexReader {
141-
if r, ok := f.store.(raftengine.AppliedIndexReader); ok {
142-
return r
132+
// LastAppliedIndex implements raftengine.AppliedIndexReader by
133+
// forwarding to the underlying store when it supports the reader
134+
// seam (pebbleStore does; the in-memory mvccStore does not).
135+
//
136+
// Round-1 of this PR shipped this as a factory method
137+
// (AppliedIndexReader() AppliedIndexReader) intended to be called
138+
// through a separate AppliedIndexReporter interface. Codex P2 on
139+
// round-4 (kv/fsm.go:145) correctly pointed out that the planned
140+
// cold-start skip gate (Branch 3) will type-assert
141+
// fsm.(raftengine.AppliedIndexReader) directly — a factory method
142+
// with a different signature does NOT satisfy the interface, so
143+
// the skip would always fall back even after the meta key is
144+
// populated. Renaming the method to LastAppliedIndex and inlining
145+
// the type-assert forward makes kvFSM directly satisfy
146+
// raftengine.AppliedIndexReader.
147+
//
148+
// (0, false, nil) propagates the strictly-additive fallback when
149+
// the store does not expose the seam — the future skip gate treats
150+
// "missing" as "fall back to full restore." See
151+
// docs/design/2026_06_02_idempotent_snapshot_restore.md §3 / §4.
152+
func (f *kvFSM) LastAppliedIndex() (uint64, bool, error) {
153+
r, ok := f.store.(raftengine.AppliedIndexReader)
154+
if !ok {
155+
return 0, false, nil
143156
}
144-
return nil
157+
idx, present, err := r.LastAppliedIndex()
158+
if err != nil {
159+
return 0, false, errors.WithStack(err)
160+
}
161+
return idx, present, nil
145162
}
146163

147164
// SetDurableAppliedIndex implements raftengine.AppliedIndexWriter by
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package kv
2+
3+
import "github.com/bootjp/elastickv/internal/raftengine"
4+
5+
// Round-4 P2 #2 fix: kvFSM must directly implement
6+
// raftengine.AppliedIndexReader so the cold-start skip gate's
7+
// type-assertion (Branch 3) succeeds.
8+
var _ raftengine.AppliedIndexReader = (*kvFSM)(nil)
9+
var _ raftengine.AppliedIndexWriter = (*kvFSM)(nil)

0 commit comments

Comments
 (0)