refactor: 参数改注入，切断存储层与热路径对全局配置的依赖

[NeverENG]

最后一项结构问题：config.G 这个全局可变单例。

为什么它是问题（证据，不是偏好）

生产代码已经在为它打补丁。 sstable.go 与 engine.go 都写着「构造时从 config 快照一份」，注释直言是为了避开「与（测试中）并发修改全局配置形成的数据竞争」——生产代码在为测试的副作用做防御，这是最强的信号。

它已经造成过故障。 本仓库此前的 TestEngine_* 偶发失败，正是全局配置 + 各用例后台协程共享造成的。

两个包级变量甚至是静默失效的：maxLevel / probability 在 import 时取自全局配置，于是测试里设 MaxMemTableLevel 一直是空操作。

它还挡住了集成测试：同进程内无法并存两套不同配置的引擎，而多节点测试正需要。

改法

新增 storage.Options，由 NewEngine / NewSSTable 构造时接收。DefaultOptions() 是全局配置进入存储层的唯一入口——调用方读一次，引擎此后只认自己那份参数。

	改前	改后
storage 生产代码读 config.G	8 处，散布 2 个文件	7 处，全在 options.go
storage 测试改 config.G	74 处	0
storage 包 import config	engine.go / sstable.go	仅 options.go

跳表也不再依赖包级变量：SkipList 自带层高与升层概率，构造时固定。测试全部改为传入独立 Options，连带删掉大量 old := config.G.X; defer 还原 的样板。

热路径上的全局读取

bannet 的测试一处都没改过全局配置，所以「测试污染」这条论据对它不成立。但它有另一个问题：每读一帧都在访问可变全局状态——UnPack 读两次 MaxPackageSize，StartReader 再读一次 WorkerPoolSize。

帧长上限本就是策略而非编解码：移到连接侧、在读取负载之前执行，编解码器因而保持无状态；worker 池选择改为构造时快照。bannet 剩余的配置读取全部在构造期或每次 accept 时。

移动执行点必须有测试守着——否则一个损坏或恶意的帧头就能让服务端按对端声称的长度分配内存。新增用例：只发一个声称 64MiB 负载的帧头、不发负载，断言服务端在读取负载前即断开、且该帧未进入业务处理。已变异验证：去掉上限判断后用例失败（服务端果然一直等着那 64MiB）。

未改动的部分及理由

raft 只有 2 处读取且都在构造期，测试也不改全局配置——收益不足以抵改动面。

service 有 24 处读取、其测试 34 处改配置。但它是编排层、紧邻程序入口，读取进程配置正是其职责；参数化它需要一路改到 NewKVServer 签名与全部集成测试，属独立一轮工作。

验证

go build ./...（含 -tags pprof）、go vet ./...、gofmt 全绿；go test -race ./... 连跑多次通过；scripts/bench.sh 实跑通过。

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Oversized network frames are now rejected before payload memory is allocated, improving resilience against malformed or excessive requests.
  • Connections consistently use their configured packet-size and worker settings throughout their lifetime.

• Improvements

  • Storage engines and tables now support instance-specific configuration, improving isolation and predictability when running multiple storage instances.
  • Default storage settings are applied automatically when configuration values are omitted or invalid.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 142ab08d-a67c-41cd-bcc7-1e892b004b4d

📥 Commits

Reviewing files that changed from the base of the PR and between e84b39e and 9c167d1.

📒 Files selected for processing (24)

• bannet/connection.go
• bannet/datapack.go
• bannet/oversized_frame_test.go
• cmd/ban-ingest/main.go
• service/fsm.go
• storage/bench_test.go
• storage/compaction_bench_test.go
• storage/compaction_failure_test.go
• storage/compaction_level_test.go
• storage/engine.go
• storage/engine_semantics_test.go
• storage/engine_test.go
• storage/errors_test.go
• storage/merge_test.go
• storage/options.go
• storage/recency_fuzz_test.go
• storage/recency_test.go
• storage/reload_recover_test.go
• storage/scan_test.go
• storage/skiplist.go
• storage/sstable.go
• storage/sstable_bloom_test.go
• storage/sstable_truncated_test.go
• storage/testhelpers_test.go

---

📝 Walkthrough

Walkthrough

The PR adds per-instance storage options and updates storage constructors, production callers, tests, and benchmarks. It also snapshots connection settings and rejects oversized frames before payload allocation.

Changes

Storage configuration isolation

Layer / File(s)	Summary
Storage options and constructors storage/options.go, storage/engine.go, storage/sstable.go, storage/skiplist.go, cmd/ban-ingest/main.go, service/fsm.go	Storage components now accept Options, apply defaults, and use instance-specific limits, paths, caches, and skip-list settings. Production constructors pass storage.DefaultOptions().
Storage test and benchmark migration storage/*_test.go	Tests and benchmarks now use temporary directories and explicit Options values instead of changing global configuration.
Network frame validation bannet/connection.go, bannet/datapack.go, bannet/oversized_frame_test.go	Connections snapshot packet and worker-pool settings. StartReader rejects oversized frames before reading payload data. The TCP test verifies rejection and non-dispatch.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• NeverENG/BanFlux#272: It overlaps in storage/engine.go and storage/skiplist.go, which this PR changes to use explicit configuration.

✨ 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/inject-config

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

🐯 BanGD 数据库内核评审

整体风险：🟡 中

变更总结：这是一个结构性重构 PR，目标是把存储层与 bannet 网络层从对全局可变单例 config.G 的运行时依赖中剥离出来，改为构造时通过显式参数/injection 传入。具体分两部分：(1) 存储层新增 storage.Options，NewEngine/NewSSTable 构造时一次性接收全部可调参数，跳表也把层高/升层概率从包级变量（import 时取全局配置、测试改配置为静默空操作）姿态改为实例字段 maxLevel/p，构造时固定。测试的 74 处改 config.G 全部清零。(2) bannet 侧把帧长上限校验从 DataPack.UnPack 移到连接侧 StartReader（读取负载前执行），并把 MaxPackageSize/WorkerPoolSize 两项策略在 NewConnection 构造时快照为实例字段，使编解码器无状态。PR 还新增了 TestOversizedFrameRejectedBeforeReadingPayload 守护帧长上限的执行点被移动到边界后仍生效。核心不变量：全局配置只允许在构造期被读取一次，之后引擎/连接只认自己快照的参数。

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

架构问题（共 3 项）

• [兼容] DataPack 帧长上限校验移除后无全局兜底 bannet/datapack.go → 🐯 [兼容] DataPack 帧长上限校验移除后无全局兜底 · datapack.go #289
• [兼容] 跳过默认值后 BlockCacheBytes 无默认导致缓存关闭 storage/options.go → 🐯 [兼容] 跳过默认值后 BlockCacheBytes 无默认导致缓存关闭 · options.go #290
• [并发] bannet 构造仍与连接耦合全局读竞争窗口 bannet/connection.go → 🐯 [并发] bannet 构造仍与连接耦合全局读竞争窗口 · connection.go #291

普通问题（共 2 项）

💡 [建议 · 测试可靠性] bannet/oversized_frame_test.go:67 测试用 Retry-Dial 循环掩盖服务端未就绪

• for i := 0; i < 100; i++ { DialTimeout; sleep 20ms } 在服务端启动慢时盲目重试，若 2s 内始终连不上则 conn==nil 且 t.Fatalf 打印的是最后一次 err（在循环外该变量已被赋为 nil 路径）。更关键的是测试先 ln.Close() 再用原端口，存在端口被 OS 复用的竞态——虽然概率极低，但若被复用一个与测试预期不符的服务会假绿。
• 建议：用 bannet 提供的就绪机制或循环内保留 dial err 变量并在超时后 t.Fatalf 输出具体 err；端口复用竞态可接受（极罕见），但建议在 srv.Start() 前后轮询端口就绪而非 100 次盲等。

💡 [建议 · 资源使用] storage/engine.go:318 Flush 时每次 newSkipList 按 maxLevel 分配 update 数组

• Flush() 里 m.active = newSkipList(m.opts.SkipListMaxLevel, m.opts.SkipListP) 且 insert 每次 make([]*SkipNode, sl.maxLevel)（默认 32 个指针）。flush 是高频路径，每次 flush 都重新分配一个 32 元素指针数组。虽非泄露，但属于可复用的暂存分配——原本 newSkipList() 隐式共享包级 maxLevel 时也存在同样分配，故本 PR 未回归；但用单例池或固定上限数组可消除每次 flush 的堆分配。仅影响微性能，不算缺陷。
• 建议：可选优化：insert 的 update 数组可按 maxLevel 上限用 sync.Pool 复用，顺带消除 flush 路径上每节点一次的对象更替压力。

---

_{本次评审消耗 token：共 294198 tokens（输入 274985，输出 6029，缓存命中 13184，缓存写入 0）｜维度 [concurrency, memory, lock, storage, performance]｜补充阅读周边文件 [bannet/message.go]｜对抗式复核 3 票/条，过滤疑似误报 1 条}