Skip to content

refactor(storage): MemTable 更名为 Engine,跳表拆出独立文件 - #272

Merged
NeverENG merged 1 commit into
mainfrom
refactor/name-the-engine
Aug 13, 2026
Merged

refactor(storage): MemTable 更名为 Engine,跳表拆出独立文件#272
NeverENG merged 1 commit into
mainfrom
refactor/name-the-engine

Conversation

@NeverENG

@NeverENG NeverENG commented Aug 13, 2026

Copy link
Copy Markdown
Owner

删掉那层纯转发的 Engine 空壳后暴露出的真问题:真正的引擎一直叫 MemTable

它到底持有什么

type MemTable struct {
    active *SkipList        // 活跃表
    dirty  *SkipList        // 正在 flush 的不可变快照
    sst    *SSTable         // ← 整组 SSTable
    credits *credit.Pool    // ← 字节级写入背压
    FlushChan, compactCh    // ← 驱动 flush 与 compaction 两个后台协程
}

再加上 getFromSSTables / FlushToSSTable / CompactSSTable —— 这是完整的 LSM 引擎,不是一张内存表。名字与职责不符会双向误导:想找存储引擎的人找不到,看到 MemTable 的人以为它只管内存。

对照 Badger 的 DB{mt, imm, lc}、Pebble 的 DB{mu.mem, mu.versions}——引擎持有内存表与层级,而不是反过来由内存表持有一切。

改了什么

  • 类型 MemTableEngine,构造器 NewMemTableNewEngine
  • 真正的内存表是 SkipList,从同一文件拆到 skiplist.go:它是一个不加锁的有序数据结构,并发由持有者统一同步,与 flush / compaction / SSTable 无关。两个文件职责因此各自单一(engine.go 436 行 / skiplist.go 175 行)。
  • 测试文件随之更名为 engine_test.goengine_semantics_test.go

刻意不改的

配置项 MaxMemTableSize / MemTableMaxInflightBytes / MaxMemTableLevel / MaxMemTableP,以及指标名 SetMemTableGauges / memtable_inflight_bytes。它们对外可见(配置文件、观测日志),且描述的确实是 memtable 那一部分——改名只会破坏既有配置与监控,不会让谁更清楚。

过程中的两处自我修正

  1. 按顶层声明切分文件时,两处文档注释被挂到了相邻类型上(Engine 顶着 SkipList 的说明,SkipNode 顶着旧 MemTable 的说明)。已重写为各自准确的描述,并补上「双表为何能让 flush 在锁外进行」这一关键设计理由。
  2. 新写的 Engine 文档里我一度写了「该类型此前叫 MemTable」——这正是我两个 PR 前刚清理掉的「注释叙述历史」。已改为陈述当前的区分(内存表是 SkipList,Engine 是持有它们的整条链路),历史留在 git 里。

验证

go build ./...(含 -tags pprof)、go vet ./...go test -race ./...gofmt 全绿;scripts/bench.sh 实跑通过。

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added an ordered in-memory storage component supporting lookups, updates, deletions, size tracking, and conversion to log entries.
    • Exposed storage functionality through the unified Engine interface.
  • Refactor

    • Updated server storage, ingestion benchmarks, and storage workflows to use the engine implementation.
    • Preserved flushing, snapshots, recovery, compaction, tombstones, and write backpressure behavior.
  • Tests

    • Updated storage, recovery, scanning, and benchmark coverage to validate engine-based operation.

该类型持有 active/dirty 两张跳表、一组 SSTable(sst 字段)、字节级背压信用,并驱动
flush 与 compaction 两个后台协程——它是完整的 LSM 引擎,而不是一张内存表。名字与职责
不符会误导读者:想找存储引擎的人找不到,看到 MemTable 的人以为它只管内存。

真正的内存表是 SkipList,随之从同一文件拆到 skiplist.go:它是一个不加锁的有序数据结构,
并发由持有者统一同步,与 flush / compaction / SSTable 无关。engine.go 与 skiplist.go
的职责因此各自单一(436 / 175 行)。

不改的:配置项(MaxMemTableSize、MemTableMaxInflightBytes、MaxMemTableLevel、
MaxMemTableP)与指标名(SetMemTableGauges、memtable_inflight_bytes)。它们对外可见,
且描述的确实是 memtable 那一部分,改名只会破坏既有配置与观测。

拆分中两处文档注释被错误地挂到了相邻类型上(Engine 顶着 SkipList 的说明),已重写为各自
准确的描述,并补上双表为何能让 flush 在锁外进行。测试文件随之更名 engine_test.go /
engine_semantics_test.go。

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

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ec0fe9e4-54a8-495d-a86e-a3446fbbd225

📥 Commits

Reviewing files that changed from the base of the PR and between a204e25 and e79222a.

📒 Files selected for processing (12)
  • cmd/ban-ingest/main.go
  • service/fsm.go
  • storage/bench_test.go
  • storage/compaction_bench_test.go
  • storage/engine.go
  • storage/engine_semantics_test.go
  • storage/engine_test.go
  • storage/errors_test.go
  • storage/reload_recover_test.go
  • storage/scan_test.go
  • storage/skiplist.go
  • storage/testhelpers_test.go

📝 Walkthrough

Walkthrough

Changes

The storage implementation is renamed from MemTable to Engine. A standalone ordered SkipList now provides in-memory table operations. Application wiring, benchmarks, recovery tests, and storage tests now use NewEngine().

Storage engine migration

Layer / File(s) Summary
SkipList storage structure
storage/skiplist.go
Adds ordered lookup, insertion, deletion, size accounting, and LogEntry traversal without internal locking.
Engine storage lifecycle
storage/engine.go
Renames storage methods to Engine receivers and retains reads, writes, tombstones, flushing, SSTable lookup, snapshots, shutdown, and compaction.
Application storage construction
service/fsm.go, cmd/ban-ingest/main.go
KVServer and ingest tooling now construct and monitor storage.Engine instances.
Engine test and benchmark coverage
storage/*_test.go, storage/*bench_test.go
Tests and benchmarks now initialize Engine and exercise its lifecycle and storage behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

  • NeverENG/BanFlux issue 264: The rename addresses the issue’s objective to represent the full LSM storage implementation as Engine.

Possibly related PRs

✨ 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 refactor/name-the-engine

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:把原 MemTable(实为持有 active/dirty 跳表、整组 SSTable、字节信用背压与 flush/compaction 后台协程的完整 LSM 引擎)更名为 EngineNewMemTableNewEngine),并把真正的无锁跳表 SkipList(含 insert/delete/search/firstGTE/collectAllEntry/randomLevel 等)拆到独立文件 skiplist.go。改名同时覆盖服务层(fsm.go、ingest 工具)与全部测试/bench。文件中同步修正了错误挂在相邻类型上的文档注释,并重写了 Engine 的架构说明(双表为何让 flush 能在锁外进行等)。

从架构看,此 PR 不改变任何运行时行为——跳表方法只是原样搬移、类型名机械替换,未动同步语义、磁盘格式、WAL 或隔离级别。结论是「纯结构重组」,逻辑正确性可通过既有测试(含 -race)覆盖。风险集中在改名可能遗留的引用未同步(如注释/局部变量仍叫 MemTable)、skiplist.goprobability 等包级变量跨文件继续被引用(依赖文件编译顺序之外的包级初始化,属于既有耦合,本次搬迁未改变它)等一致性细节上。

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

架构问题(共 2 项)

普通问题(共 3 项)

💡 [建议 · 命名一致性] storage/bench_test.go:70 bench 函数仍叫 BenchmarkMemTable_Put/Get

  • renamed Engine 后,BenchmarkMemTable_PutBenchmarkMemTable_Get 函数名未更名(其余已改 BenchmarkEngine_*),与新命名体系不一致,容易让人误以为仍测的是旧的 MemTable 类型。且 bench 内变量 mt 也沿用旧名。
  • 建议:将 BenchmarkMemTable_PutBenchmarkEngine_Put_MemTable 或统一改为 BenchmarkEngine_*,并把局部变量 mt 改名为 engine,与文件内其它已改名 bench 对齐。

💡 [建议 · 命名一致性] storage/engine.go:55 NewEngine 局部变量仍叫 mt

  • NewEngine() 内部局部变量名为 mt(原 MemTable 缩写),而该类型现为 Engine;mt := &Engine{...} 读起来像 memtable。同文件 NewEngine doc 里「NewEngine 创建新的 MemTable」这句也残留旧语义。
  • 建议:局部变量改名为 enge;doc 注释「NewEngine 创建新的 MemTable」改为「NewEngine 创建新的存储引擎(Engine)」。

💡 [建议 · 命名一致性] service/fsm.go:139 fsm.go 多处注释与日志仍称 MemTable

  • replayWAL 注释「重放进 memtable」、Scan doc「扫描 MemTable 热数据」、setupEngine 返回的仍是旧型名等——文件里 storage *storage.Engine 已改名,但注释仍沿用 memtable 指代引擎整体,未随改名同步。
  • 建议:将这些注释里的「memtable」统一指代内存表部分(若确实指 SkipList),或改称「engine」/「存储引擎」以对准新类型名。至少保证注释与类型名不自相矛盾。

本次评审消耗 token:共 208745 tokens(输入 192375,输出 4338,缓存命中 12032,缓存写入 0)|维度 [concurrency, memory, lock, storage, performance]|补充阅读周边文件 [storage/sstable.go, storage/wal.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