Skip to content

fix(storage): 修复 WAL.Clear() nil panic 并添加并发安全保护#31

Merged
NeverENG merged 1 commit into
mainfrom
fix/wal-concurrency
May 11, 2026
Merged

fix(storage): 修复 WAL.Clear() nil panic 并添加并发安全保护#31
NeverENG merged 1 commit into
mainfrom
fix/wal-concurrency

Conversation

@NeverENG
Copy link
Copy Markdown
Owner

@NeverENG NeverENG commented May 11, 2026

  • Clear() 在 WAL 禁用模式(file==nil)时使用 config.G.WALPath 避免空指针
  • 为 WAL 添加 sync.Mutex 保护所有公共方法(Write/Read/Close/Sync/Clear)
  • Read() 中使用 io.ReadFull 确保完整读取,消除部分数据风险
  • RaftWAL.LoadLogs() 使用 io.ReadFull 修复截断读取
  • 统一使用 slog 格式,移除冗余的 [ERROR]/[WARN] 前缀
  • 重命名 HEADER_LENGTH→headerLength(非导出标识符)

Summary by CodeRabbit

  • Bug Fixes

    • Improved reliability of log loading operations by ensuring complete file reads for each log entry
    • Enhanced log clearing operations with better error handling and file cleanup management
    • Better error propagation when closing and managing log files
  • Refactor

    • Added thread-safety protections to write-ahead logging operations
    • Improved log reading and writing mechanisms to ensure data integrity across concurrent operations
    • Strengthened file synchronization to prevent data loss

Review Change Stack

- Clear() 在 WAL 禁用模式(file==nil)时使用 config.G.WALPath 避免空指针
- 为 WAL 添加 sync.Mutex 保护所有公共方法(Write/Read/Close/Sync/Clear)
- Read() 中使用 io.ReadFull 确保完整读取,消除部分数据风险
- RaftWAL.LoadLogs() 使用 io.ReadFull 修复截断读取
- 统一使用 slog 格式,移除冗余的 [ERROR]/[WARN] 前缀
- 重命名 HEADER_LENGTH→headerLength(非导出标识符)
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 11, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR hardens WAL (write-ahead log) reliability across two layers. The core storage/zstorage/WAL.go adds mutex-protected operations, enforces full-length I/O reads via io.ReadFull, and improves state cleanup and error propagation. The Raft/raft_wal.go layer adopts the same full-length read guarantees and improves error handling during log loading and clearing.

Changes

WAL Reliability and Thread-Safety

Layer / File(s) Summary
WAL Struct and Type Signature
storage/zstorage/WAL.go
Header size constant renamed from HEADER_LENGTH (exported) to headerLength (unexported). WAL struct adds sync.Mutex field for thread-safety and updates header buffer to use new constant.
Write Operation with Locking
storage/zstorage/WAL.go
Write now locks, explicitly checks errors for each header/key/value write, and calls file.Sync() on success; disabled-mode no-op behavior preserved.
Read Operation with Locking
storage/zstorage/WAL.go
Read now locks, seeks to start, uses io.ReadFull for complete header/key/value blocks, treats EOF/short-read as end-of-log, and logs corruption on CRC mismatch while returning collected entries.
Close and Sync Operations
storage/zstorage/WAL.go
Close and Sync acquire lock, preserve nil-file short-circuiting, and return underlying file errors only when enabled.
Clear State Management
storage/zstorage/WAL.go
Clear now locks and defensively handles both modes: removes WAL path and reopens when disabled, or closes/removes/reopens when enabled with explicit sync.
Constructor Logging Updates
storage/zstorage/WAL.go
NewWAL log messages switch from bracketed [INFO]/[WARN]/[ERROR] style to plain slog messages.
Raft WAL Integration
Raft/raft_wal.go
Added io package import; LoadLogs uses io.ReadFull for command payload reads to fail on incomplete data; Clear checks w.Close() error, sets w.file = nil, removes state files while tolerating missing files, and recreates log file.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 The logs now read with careful grace,
Full bytes in their proper place,
With mutex locks to keep them sound,
No partial reads shall here be found,
hop hop toward reliability!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main objectives: fixing WAL.Clear() nil panic and adding concurrency protection with mutex, which are core changes in both files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/wal-concurrency

Comment @coderabbitai help to get the list of available commands and usage tips.

@NeverENG NeverENG merged commit 269c031 into main May 11, 2026
2 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant