refactor: Raft 日志规范化 + 增量持久化 + 单节点快速选举#37
Open
NeverENG wants to merge 1 commit into
Open
Conversation
- 所有 fmt.Printf 改为结构化 slog - 新增 persistStateLocked() 增量持久化 Term/votedFor，O(1) - AppendEntry 改用 wal.AppendLog() 增量追加日志 - 单节点模式无需等待投票，直接成为 Leader - getLastLogIndex() 空日志时返回 -1，修复快照偏移边界问题 - Server/FSM 日志文本同步微调 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
📝 WalkthroughWalkthroughThis PR systematically replaces fmt-based logging with structured slog throughout the Raft consensus module and FSM, introduces incremental state persistence to reduce full-state writes, adds single-node election handling, refines log index semantics when snapshots exist, and wires server router lifecycle callbacks. ChangesRaft Logging, Persistence, and Election Updates
Server Initialization Wiring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Raft/raft.go`:
- Around line 126-131: The persistStateLocked helper currently only persists
Term/votedFor in some places; ensure every mutation of r.Term or r.votedFor
calls persistStateLocked while holding the raft lock so state is durable. Find
all code paths that assign r.Term or r.votedFor (e.g., the higher-term
reply/response handlers such as the functions that process
AppendEntries/RequestVote replies or any onReceiveHigherTerm/becomeFollower
helpers) and add a call to persistStateLocked() immediately after the assignment
(still under the same lock), propagating/logging any persistence error as the
existing SaveState path does. Ensure no code updates Term/votedFor without
invoking persistStateLocked under lock.
- Around line 206-213: The code mixes a new zero-based log-index convention with
old guards that treat LastIncludedIndex==0 as “no snapshot”, causing
relativeStart to be -1 in replicateLog() and skipping valid snapshot index 0;
update the logic around lastLogIndex/lastLogTerm and snapshot checks to
consistently treat LastIncludedIndex as the actual last included log index
(allowing 0), e.g. initialize lastLogIndex = int(r.LastIncludedIndex) when r.log
is empty, compute relativeStart as int(start) - int(r.LastIncludedIndex) (not
subtracting 1), and replace all `> 0` snapshot guards with `>= 0` or explicit
nil/empty checks so functions like replicateLog(), and code paths referencing
r.LastIncludedIndex/r.LastIncludedTerm/r.log use the same baseline across the
file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c4ffd64-0104-4079-97a4-d89e39c6bf6e
📒 Files selected for processing (3)
Raft/raft.goServer/server.goservice/fsm.go
Comment on lines
+126
to
+131
| // persistStateLocked 仅持久化 Term 和 votedFor（增量持久化，O(1)） | ||
| func (r *Raft) persistStateLocked() { | ||
| if err := r.wal.SaveState(int64(r.Term), int64(r.votedFor)); err != nil { | ||
| slog.Error("failed to persist state", "error", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Persist every Term/votedFor mutation, not just election start.
This helper is a good optimization, but the higher-term reply paths still update Term and votedFor without calling it. A crash after one of those downgrades can restart the node with a stale term/vote.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Raft/raft.go` around lines 126 - 131, The persistStateLocked helper currently
only persists Term/votedFor in some places; ensure every mutation of r.Term or
r.votedFor calls persistStateLocked while holding the raft lock so state is
durable. Find all code paths that assign r.Term or r.votedFor (e.g., the
higher-term reply/response handlers such as the functions that process
AppendEntries/RequestVote replies or any onReceiveHigherTerm/becomeFollower
helpers) and add a call to persistStateLocked() immediately after the assignment
(still under the same lock), propagating/logging any persistence error as the
existing SaveState path does. Ensure no code updates Term/votedFor without
invoking persistStateLocked under lock.
Comment on lines
+206
to
+213
| lastLogIndex := -1 | ||
| lastLogTerm := 0 | ||
| if len(r.log) > 0 { | ||
| lastLogIndex = r.log[len(r.log)-1].Index | ||
| lastLogTerm = r.log[len(r.log)-1].Term | ||
| } else if r.LastIncludedIndex > 0 { | ||
| lastLogIndex = int(r.LastIncludedIndex) | ||
| lastLogTerm = int(r.LastIncludedTerm) |
There was a problem hiding this comment.
Keep the new zero-based index convention consistent across Raft.
After these changes, the first real log entry becomes index 0, but the rest of the file still assumes the first in-memory entry is LastIncludedIndex + 1 and that LastIncludedIndex == 0 means “no snapshot”. In a fresh multi-node cluster that makes replicateLog() compute relativeStart == -1 on the first append, and a snapshot taken at index 0 is ignored by the > 0 guards during later bootstrap/election flows. Please align on one baseline before merging.
Also applies to: 306-310, 524-527, 540-545
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Raft/raft.go` around lines 206 - 213, The code mixes a new zero-based
log-index convention with old guards that treat LastIncludedIndex==0 as “no
snapshot”, causing relativeStart to be -1 in replicateLog() and skipping valid
snapshot index 0; update the logic around lastLogIndex/lastLogTerm and snapshot
checks to consistently treat LastIncludedIndex as the actual last included log
index (allowing 0), e.g. initialize lastLogIndex = int(r.LastIncludedIndex) when
r.log is empty, compute relativeStart as int(start) - int(r.LastIncludedIndex)
(not subtracting 1), and replace all `> 0` snapshot guards with `>= 0` or
explicit nil/empty checks so functions like replicateLog(), and code paths
referencing r.LastIncludedIndex/r.LastIncludedTerm/r.log use the same baseline
across the file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fmt.Printf改为结构化slog，统一日志格式persistStateLocked()— 增量持久化 Term/votedFor（O(1)），不再每次全量写入AppendEntry改用wal.AppendLog()增量追加单条日志getLastLogIndex()空日志返回 -1，修复快照偏移边界条件lastLogIndex计算增加快照回退逻辑Files
Raft/raft.go— 核心改动Server/server.go— 移除冗余打印service/fsm.go— 日志措辞微调🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Chores