Skip to content

feat(storage): 扫描覆盖 SSTable——修复下游投递静默漏投已落盘数据 - #292

Merged
NeverENG merged 1 commit into
mainfrom
feat/scan-covers-sstables
Aug 13, 2026
Merged

feat(storage): 扫描覆盖 SSTable——修复下游投递静默漏投已落盘数据#292
NeverENG merged 1 commit into
mainfrom
feat/scan-covers-sstables

Conversation

@NeverENG

Copy link
Copy Markdown
Owner

问「功能还能不能优化」,查下来第一件事就不是缺功能,是核心功能在静默丢数据

实测证据

写入 200 条,等 flush 落定后:
  逐个 Get  → 命中 200 条
  ScanRange → 看到   0 条

ScanRange 只遍历 activedirty 两张内存表,已 flush 的数据对扫描完全不可见。两条读路径的可见范围不一致,所以很难察觉。

后果不止 SCAN 少返回结果

下游投递正是按游标反复调用 Scan 取数:

delivery.KVSource.Fetch(cursor) → kv.Scan(cursor, end) → Engine.ScanRange  ← 只看内存表

而 flush 阈值上万条、投递每秒百条 —— flush 必然快于投递,落盘的记录于是再也不会被投递。「摄入缓冲 + 可靠投递」是本项目 README 的头号能力,这里却在静默漏投。

实现

复用 compaction 已有的归并迭代器:抽出 entryIterator 接口,让内存表快照与 SSTable 文件迭代器混合参与多路归并。源序按新旧排列(SSTable 由旧到新 → dirtyactive),故同 key 只保留最新版本;最新版本是墓碑时整条跳过。

锁的处理:内存表在锁内按范围拷出快照,锁外做归并。归并要读磁盘——全程持锁会让写入停等 I/O,而无锁遍历跳表又会与并发写相争(Get 曾因此出错)。拷贝量由内存表大小天然有界。

一个必须解决的性能问题

按游标扫描必须能跳过前缀,否则投递的总代价随数据量平方增长。首版实现顺序跳过小于 start 的条目,端到端实测:2000 条投递 6 秒只完成 810 条。

改为借块索引二分定位到目标块起点(blockIndex 本来就在,读路径点查已在用它)。无块索引时(老格式或尾部残缺)退回从头读,正确性由范围裁剪保证。

验证

  • 存储层与 service 层各加回归用例,含覆盖写与墓碑跨 SSTable 的新旧判定
  • 变异验证:退回只扫内存表后用例立即失败
  • 端到端MaxMemTableSize=50 写入 2000 条、产生 56 个 SSTable,投递收敛到 2000/2000、零错误
  • 读吞吐无回归:交替 A/B 对比 main(180556 vs 171419、166164 vs 158165,分支两次均略快)。中途曾看到一次 113k,交替测量后确认是环境漂移而非回归——顺序对比在这台机上会骗人

go build ./...(含 -tags pprof)、go vetgofmtgo test -race ./... 连跑 2 次全绿。

顺带修正的文档

KVServer.Scan 的注释仍写着「扫描 MemTable 热数据」,已更正为覆盖内存表与 SSTable。

🤖 Generated with Claude Code

ScanRange 此前只遍历 active 与 dirty 两张内存表,已 flush 的数据对扫描完全不可见。
实测:写入 200 条并等 flush 落定后,逐个 Get 命中 200 条,而 ScanRange 只看到 0 条——
两条读路径的可见范围不一致,问题因此很难察觉。

后果不止 SCAN 命令少返回结果。下游投递(delivery.KVSource)正是按游标反复调用 Scan 取数,
而 flush 阈值上万条、投递每秒百条,flush 必然快于投递——落盘的记录就再也不会被投递。
「摄入缓冲 + 可靠投递」是本项目的核心能力,这里却在静默丢数据。

实现为多路归并,复用 compaction 已有的归并迭代器:抽出 entryIterator 接口,让内存表快照
与 SSTable 文件迭代器混合参与;源序按新旧排列(SSTable 由旧到新、其后 dirty、最后
active),故同 key 只保留最新版本,最新版本是墓碑时整条跳过。

内存表在锁内按范围拷出快照、锁外做归并:归并要读磁盘,全程持锁会让写入停等 I/O,而无锁
遍历跳表又会与并发写相争(Get 曾因此出错)。拷贝量由内存表大小天然有界。

按游标扫描必须能跳过前缀,否则投递的总代价随数据量平方增长——首版实现顺序跳过小于
start 的条目,实测 2000 条投递 6 秒仅完成 810 条。改为借块索引二分定位到目标块起点,
无块索引(老格式或尾部残缺)时退回从头读,正确性由范围裁剪保证。

验证:新增存储层与 service 层回归用例(含覆盖写与墓碑跨 SSTable 的新旧判定),并已变异
验证——退回只扫内存表后用例立即失败。端到端实测:MaxMemTableSize=50 写入 2000 条产生
56 个 SSTable,投递收敛到 2000/2000、零错误。读吞吐用交替 A/B 核对无回归。

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

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@NeverENG, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 10 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: aa1cd44a-ba3f-4d95-b194-890856153ad0

📥 Commits

Reviewing files that changed from the base of the PR and between 5368d6b and d7472fa.

📒 Files selected for processing (7)
  • service/fsm.go
  • service/scan_integration_test.go
  • storage/engine.go
  • storage/iterator.go
  • storage/merge_iterator.go
  • storage/scan_test.go
  • storage/sstable_write.go

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions

Copy link
Copy Markdown

🐯 BanGD 数据库内核评审

整体风险:🟡 中

变更总结:此 PR 修复了 LSM 存储引擎的「扫描覆盖缺口」:此前 ScanRange 只遍历 active 与 dirty 两张内存表,已 flush 到 SSTable 的数据对 SCAN 完全不可见。后果不仅是 SCAN 少返回——依赖按游标反复 Scan 取数的下游投递(delivery.KVSource)会静默漏掉所有已落盘的记录。修复把 entryIterator 接口从 compaction 归并器中抽出,让多条 SSTable 文件迭代器与两张内存表快照在锁外混合做 K 路归并,源序(SSTable 旧→新 → dirty → active)保证同 key 只保留最新版本、墓碑 shadow 旧版。锁处理上采用「锁内按范围拷出内存表快照、锁外归并」,避免全程持锁让写入停等磁盘 I/O。同时借块索引二分定位到目标块起点,避免按游标推进时从头顺序跳过造成 O(n²) 退化。它的核心价值是把两条读路径(Get / Scan)的可见范围拉齐到同一套 latest-visible 语义上,堵住了静默丢数据的通道。

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

架构问题(共 3 项)

普通问题(共 2 项)

⚠️ [重要 · 错误处理] storage/scan_test.go:96 waitFlushed 超时无失败信号、靠 len(metas)>0 且 dirty==nil 判定

  • waitFlushed 循环 200 次(最多 2 秒)。若 flush 在 2 秒内未完成(慢 CI / 构造 Initial 文件 / 极小阈值下 write 自身也触发 flush 但异步未落定),函数 静默返回 而不 t.Fail——调用方 TestScanRange_CoversFlushedData 随后立即 ScanRange,若此时 dirty 仍非 nil 或 metas 仍为空,扫描结果可能不覆盖全部 n 条而测试继续向下跑(因为 len(seen)!=n 断言在 ScanRange 之后才做)。更关键:若 MaxMemTableSize:5 极小时执行到 e.Close()(t.Cleanup 阶段)触发 stopCh,flush 协程退出,等待循环会一直空转到超时。应让 waitFlushed 超时即 Fail,而非静默返回。
  • 建议:在 waitFlushed 循环外追加 t.Fatalf("flush did not settle within 2s"),把「未落定」变成显式失败而非静默继续;这样一旦时序漂移测试立刻定位到等待处,而不是在后续 ScanRange 断言处令人困惑地失败。

💡 [建议 · 兼容] service/fsm.go:265 NewKVServer 在测试中基于全局 config.G 快照的存储路径与新测试相互干扰

  • service 层既有测试 TestKVServer_Scan 用 defer 恢复 config.G.WALPath/SSTablePath/MaxMemTableSize,新测试 TestScanCoversFlushedData 用 t.Cleanup 恢复同样三者。两个测试若并行(Go 测试包无 t.Parallel 时默认串行)问题不大,但两者都会 mutate 全局 config 再做 NewKVServer——若测试框架在包内并发(-parallel 全包),会共享同一份 config G 造成路径串写。虽然当前未标 t.Parallel,但这是全局可变配置在测试间隐式耦合的隐患,与新 PR 引入的第二个改 config 的测试叠加后风险更明显。
  • 建议:建议 service 层对 config 的修改集中到一个 helper(save/restore 成对),并约束测试串行(不加 t.Parallel);或在 NewKVServer 支持注入 opts 而非全部走全局 config,从根上消除测试间通过 config.G 的耦合。

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

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