Skip to content

fix(storage): SSTable 写尾吞掉 I/O 错误——静默丢数据 - #275

Merged
NeverENG merged 1 commit into
mainfrom
fix/sstable-unchecked-writes
Aug 13, 2026
Merged

fix(storage): SSTable 写尾吞掉 I/O 错误——静默丢数据#275
NeverENG merged 1 commit into
mainfrom
fix/sstable-unchecked-writes

Conversation

@NeverENG

Copy link
Copy Markdown
Owner

巡检时发现的数据丢失缺陷,不是风格问题。

缺陷

块索引与 Footer 的写入丢弃了全部返回值(binary.Write / file.Write 共 8 处),定位偏移的 file.Seek 也用 _ 忽略错误:

indexStart, _ := file.Seek(0, io.SeekCurrent)     // 失败则 indexStart = 0
for _, b := range blockIdx {
    binary.Write(file, binary.BigEndian, uint32(len(b.lastKey)))  // 未检查
    file.Write(b.lastKey)                                        // 未检查
    binary.Write(file, binary.BigEndian, b.blockOffset)          // 未检查
}
binary.Write(file, binary.BigEndian, uint32(len(blockIdx)))      // Footer,未检查
binary.Write(file, binary.BigEndian, indexStart)
binary.Write(file, binary.BigEndian, indexFooterMagic)

后果不是「少写几个字节」,是静默丢数据

读侧:尾部残缺 → footer magic 校验失败 → loadBlockIndexFromFile 返回 nil → 重启时 LoadSSTableMetaList 保留 MaxKeyLoaded=false,由 EnsureMeta 顺序扫描兜底——而它在新格式文件上会把索引/布隆字节当记录读,算出错误的 MaxKey(这一行为本就记在代码注释里)。[MinKey,MaxKey] 过滤随即跳过整个文件,数据读不到

写侧:调用方以为落盘成功,于是销毁另一份副本——

路径 成功后的动作
Flush m.dirty = nil,丢弃内存中的唯一副本
CompactSSTable DeleteSSTable(meta) 删除全部源文件

两条路径都以「写成功」为前提销毁副本。也就是说,吞掉这里的错误 = 静默丢数据。

Seek 失败还会让 indexStart 为 0,Footer 会声称块索引位于数据区起点。

修法

WriteToSSTableMergeSSTable 的写尾逻辑本就完全相同(原注释即写着「以下写尾与 WriteToSSTable 完全一致,保证字节布局相同」),故合并为单一 writeTail:逐段检查并包裹出失败段落。读路径只有一份解析实现,写路径也应只留一份——否则布局迟早漂移。两处局部的 blk 类型提为包级 blockMeta

另两类未检查的写,经核对是安全的

不盲目加检查,而是把不变量写进注释:

  • 数据段先写 bytes.Buffer —— 其 Write 按文档永不返回错误(容量不足直接 panic)。
  • MergeSSTable 的数据循环写 bufio.Writer —— 错误具粘性,首个错误会在已被检查bw.Flush() 处浮现。

测试

sstable_tail_test.go 在尾部每一个字节位置注入写失败,断言 writeTail 必须返回包裹了底层错误、且标明失败段落(block index / bloom / footer)的错误。另一例校验写出的字节仍被读路径的 Footer 解析接受,确保两个调用方共用实现后布局未漂移。

验证

go build ./...(含 -tags pprof)、go vet ./...go test -race ./...gofmt 全绿——其中 reload_recover / recency / merge / sstable_bloom 都读真实文件,可作磁盘格式回归。

改了写路径故复验崩溃恢复:写入 5 万 key → kill -9 → 重启抽样 516 个 key,缺失 0

🤖 Generated with Claude Code

块索引与 Footer 的写入此前丢弃了全部返回值(binary.Write / file.Write 共 8 处),
定位偏移的 file.Seek 也用 _ 忽略错误。后果不是「少写几个字节」,而是静默的数据丢失:

- 尾部残缺 → footer magic 校验失败 → loadBlockIndexFromFile 返回 nil → 重启时
  EnsureMeta 在新格式文件上算出错误的 MaxKey(该行为已由既有注释记载),
  [MinKey,MaxKey] 过滤随即跳过整个文件,数据读不到;
- 而调用方以为落盘成功:Flush 会把 dirty 置 nil,丢弃内存中的唯一副本;
  CompactSSTable 会删除全部源文件。两条路径都以「成功」为前提销毁另一份副本。
- Seek 失败时 indexStart 为 0,Footer 会声称块索引位于数据区起点。

WriteToSSTable 与 MergeSSTable 的写尾逻辑本就完全相同(原注释即写明「以下写尾与
WriteToSSTable 完全一致」),故合并为单一 writeTail:逐段检查错误并包裹出失败段落,
读路径只有一份解析实现,写路径也只留一份。两处局部的 blk 类型提为包级 blockMeta。

另两类未检查的写经核对是安全的,补注释固定该不变量而非加噪声:数据段先写 bytes.Buffer
(其 Write 按文档永不返回错误);MergeSSTable 的数据循环写 bufio.Writer(错误具粘性,
首个错误会在被检查的 bw.Flush() 处浮现)。

新增 sstable_tail_test.go:在尾部每一个字节位置注入写失败,断言 writeTail 必须返回
包裹了底层错误、且标明失败段落(block index / bloom / footer)的错误;另有一例校验
写出的字节仍被读路径的 Footer 解析接受,确保两个调用方共用实现后布局未漂移。

已复验崩溃恢复:写入 5 万 key → kill -9 → 重启抽样 516 个 key,缺失 0。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@NeverENG
NeverENG merged commit 0dd7d27 into main Aug 13, 2026
3 checks passed
@github-actions

Copy link
Copy Markdown

🐯 BanGD 数据库内核评审

整体风险:🟢 低

变更总结:这是一个针对 SSTable 写入尾部(块索引 + 布隆段 + Footer)I/O 错误被静默吞掉的缺陷修复。改动把 WriteToSSTable 与 MergeSSTable 两处逻辑完全相同的写尾代码合并为单一 writeTail 函数(读路径只有一份解析实现,写路径同样只保留一份以保证字节布局不漂移),并对每段写入检查错误、包裹出失败段落和底层错误。同时还把两处局部的 blk 类型提为包级 blockMeta,并把 writeBloomSection 的参数从 *os.File 放宽为 io.Writer 以复用于内存 buffer 测试。另对两处「经核对安全」的未检查写(bytes.Buffer 数据段、bufio.Writer 的粘性错误)以注释写明其不变量。新增 sstable_tail_test.go 逐字节注入写失败断言 writeTail 必需返回带段落信息的内部错误。

动机根因:尾部残缺会让 footer magic 校验失败,读路径 loadBlockIndexFromFile 返回 nil 后由 EnsureMeta 顺序扫描兜底——但它在含索引/布隆/Footer 的新格式文件上会把非数据字节当记录读,算出错误的 MaxKey,[MinKey,MaxKey] 范围过滤随即跳过整个文件造成数据读不到;而调用方(Flush / CompactSSTable)以「写成功」为前提销毁唯一副本,故吞错等于静默丢数据。

本评审不阻塞合入;架构级建议以 Issue 形式跟踪,普通问题在下方内联列出。

架构问题(共 2 项)

普通问题(共 2 项)

💡 [建议 · 逻辑正确性] storage/sstable.go:491 WriteToSSTable 失败后不清理已创建的半成品文件

  • 在 WriteToSSTable 中,若 writeTail 返回错误(或更早的数据段 file.Write 失败),defer file.Close() 只关闭句柄,已创建的 fullPath 文件残留为尾部残缺的半成品。下次 LoadSSTableMetaList 重启会读到这个文件,并在 EnsureMeta 兜底路径上计算出错误的 MaxKey。diff 强化了错误上报,但没有改变『失败即残留坏文件』的内存/磁盘生命周期。这与 architecture finding Test: 测试 TCP 框架手写协议和 grpc 的差距. #1 属同一根源,此处指出的是其可独立观察到的具体落点。
  • 建议:在 error 返回路径上拒绝把半成品文件留给重启:为 WriteToSSTable 的每个 error return 前调用 os.Remove(fullPath)。MergeSSTable 同理。这是与架构方案(临时文件+原子 rename)互斥的快速止血方案。

💡 [建议 · 逻辑正确性] storage/sstable.go:655 loadBlockIndexFromFile 无 footer 时仍留下已 Seek 的句柄并泄漏赋值

  • 在 loadBlockIndexFromFile 中,当 magic 校验失败或 indexOffset<=0 时返回 nil;文件句柄由 defer 关闭没有泄漏,但 ReadAllFromSSTable 中 readDataEndOffset 的同事路径 sstableDataEnd 在 magic == indexFooterMagic && blockCount > 0 && indexOffset > 0 才会返回 indexOffset,否则返回 -1。这两处读侧对损坏 footer 的判定逻辑重复且各自维护,一旦写侧 footer 布局改动,两处读侧判定都会漂移——这是与 writeTail 合并逻辑不对称的另一处『读侧有多份实现』。
  • 建议:将 loadBlockIndexFromFile 与 sstableDataEnd 的 footer 校验(magic + blockCount>0 + indexOffset>0)收敛为单一 helper,消除重复的解析实现;这也与本 PR 把写侧合并为 writeTail 的动机对齐。

本次评审消耗 token:共 101271 tokens(输入 86211,输出 4180,缓存命中 10880,缓存写入 0)|维度 [concurrency, memory, lock, storage, performance]|补充阅读周边文件 [storage/bloom.go, storage/iterator.go]|对抗式复核 3 票/条,过滤疑似误报 0 条

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