Skip to content

fix(storage): 修复 EnsureMeta nil panic 并完善 SSTable 工程化细节#30

Merged
NeverENG merged 1 commit into
mainfrom
fix/sstable-ensuremeta
May 11, 2026
Merged

fix(storage): 修复 EnsureMeta nil panic 并完善 SSTable 工程化细节#30
NeverENG merged 1 commit into
mainfrom
fix/sstable-ensuremeta

Conversation

@NeverENG
Copy link
Copy Markdown
Owner

@NeverENG NeverENG commented May 11, 2026

  • EnsureMeta() 检查 os.Open 错误, 使用 io.ReadFull 替代裸 file.Read()
  • 修复 ISSTable 接口签名与实现不匹配(GetLevelFiles/AddMata/RemoveMata 等)
  • ReadAllFromSSTable 中使用 io.ReadFull 确保完整读取
  • file.Stat() 错误不再被丢弃(2处)
  • MergeSSTable 中 binary.Write/file.Write 增加错误检查
  • writeToSSTable→WriteToSSTable(导出以匹配接口)
  • 添加编译期接口断言 var _ istorage.ISSTable = &SSTable{}

Summary by CodeRabbit

  • Bug Fixes

    • Improved file I/O error handling and validation for storage operations to prevent silent failures.
  • Refactor

    • Enhanced storage interface for more explicit, type-safe operations with better resource management.

Review Change Stack

- EnsureMeta() 检查 os.Open 错误, 使用 io.ReadFull 替代裸 file.Read()
- 修复 ISSTable 接口签名与实现不匹配(GetLevelFiles/AddMata/RemoveMata 等)
- ReadAllFromSSTable 中使用 io.ReadFull 确保完整读取
- file.Stat() 错误不再被丢弃(2处)
- MergeSSTable 中 binary.Write/file.Write 增加错误检查
- writeToSSTable→WriteToSSTable(导出以匹配接口)
- 添加编译期接口断言 var _ istorage.ISSTable = &SSTable{}
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 11, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

SSTable I/O operations are refactored with explicit, typed interface contracts and hardened error handling. The ISSTable interface is redesigned with concrete method signatures requiring explicit parameters for metadata and file paths. All read/write paths switch from partial reads to io.ReadFull with validation, and the SSTable write method is exported for use by callers.

Changes

SSTable I/O Hardening and Interface Refactor

Layer / File(s) Summary
Interface Contract Definition
storage/istorage/SSTable.go
ISSTable interface methods are rewritten with explicit typed parameters: metadata operations take *SSTableMata, file reads/writes require filepath, reads return ([]byte, bool) for presence/absence, and merge takes files []*SSTableMata and targetLevel.
Import and Assertion Setup
storage/istorage/Entry.go, storage/zstorage/SSTable.go
io package imported for io.ReadFull and io.SeekCurrent. Compile-time assertion confirms *SSTable implements istorage.ISSTable.
Metadata Loading Hardening
storage/istorage/Entry.go, storage/zstorage/SSTable.go
EnsureMeta validates file open, uses io.ReadFull for key reads with error checks before seeks. LoadSSTableMetaList replaces partial file.Read with io.ReadFull for MinKey loading.
SSTable Write Persistence
storage/zstorage/SSTable.go
WriteToSSTable method exported (was writeToSSTable), returns error only, and now validates file.Stat() errors instead of ignoring them.
Bulk Read Operations
storage/zstorage/SSTable.go
ReadAllFromSSTable uses io.ReadFull for both key and value byte reads, with explicit error returns if full reads fail.
Merge Operations Hardening
storage/zstorage/SSTable.go
MergeSSTable validates all binary.Write and file.Write errors during merged-entry persistence, and checks file.Stat() errors when obtaining merged-file metadata.
Caller Updates
storage/zstorage/memtable.go
MemTable.Flush and MemTable.WriteSSTable call exported WriteToSSTable instead of private writeToSSTable.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 Stronger reads, no more guessing,
io.ReadFull passes the testing!
Types are explicit, contracts ring clear,
Error checks everywhere—safety's here!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is partially related to the changeset. It mentions fixing 'EnsureMeta nil panic' and improving 'SSTable engineering details', which aligns with the actual changes (EnsureMeta improvements, interface signature fixes, error handling, and method exports).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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/sstable-ensuremeta

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

@NeverENG NeverENG merged commit 485e175 into main May 11, 2026
3 of 11 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