Skip to content

refactor: PeerPool 改用 SDK,收敛重复的客户端与线格式实现 - #268

Merged
NeverENG merged 2 commits into
mainfrom
refactor/peerpool-on-sdk
Aug 13, 2026
Merged

refactor: PeerPool 改用 SDK,收敛重复的客户端与线格式实现#268
NeverENG merged 2 commits into
mainfrom
refactor/peerpool-on-sdk

Conversation

@NeverENG

Copy link
Copy Markdown
Owner

把节点间转发收敛到 SDK 上。净减约 300 行,并顺带修掉一处会把「远端故障」误报成「key 不存在」的缺陷。

一、PeerPool 改用 client SDK

PeerPool 此前自建 bannet.Client,并用一把锁把对同一 peer 的全部转发串行化在单条连接上。它的注释本就写着:

需要更高并发时可扩为每 peer 一个连接池,此处先保正确。

SDK 正是那个连接池。每个 peer 改持一个 client.Client 后,对同一属主的并发转发由多条连接承担,不再排队。

二、顺带修掉一处真实缺陷

bannet.Client.Get 对任何非 OK 状态一律返回 (nil, false, nil)

if status != proto.StatusOK {
    return nil, false, nil // 未命中/远端错误:按未命中处理
}

于是属主节点的真实故障会被上报成「这个 key 不存在」,上游据此返回空结果——与我此前在存储层、协议层修的是同一类缺陷。新实现只把 ErrKeyNotFound 记为未命中,其余错误原样上抛。这个区分正依赖前面为 GET 新增的 notfound 状态。

三、转发不重试

MaxRetries < 0。转发是「客户端 → 入口节点 → 属主节点」的第二跳,入口侧客户端 SDK 已带重试;此处再重试会在过载时两级相乘放大请求量,正好在最不该加压的时刻加压。网络瞬时故障由客户端那层覆盖。

四、删掉的重复实现

文件 说明
bannet/client.go(176 行) 迁走 PeerPool 与三节点集成测试后已无调用方
pkg/utils/message.go(59 行) Frame 接口的第二个实现;压测改用 bannet.NewMessageByteBuilder 仍在用,保留)
Frame 接口 见下

线格式实现由 5 份收敛为 3 份:服务端 bannet/datapack.go、SDK client/conn.go、压测自有的一份(压测刻意保留裸往返,避免 SDK 的连接池与重试污染测量)。

Frame 接口随之删除

它当初存在的唯一理由是有两个实现(bannet.Messageutils.Message)。后者删掉后只剩一个实现,按 Go 惯例就不该再是接口。

和删 Server 接口时一样,接口消失后三处仅为绕过它而写的类型断言也随之消失msg.(*Message)、两处 tempMsg.(*bannet.Message))。这类断言正是「接口只有一个实现」的典型症状——调用方总得把它转回具体类型才能干活。

验证

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

三节点转发集成测试已改用 SDK 并通过:写经入口转发到属主、数据只落属主、从第三个节点读仍能转发命中、删除经转发生效。

🤖 Generated with Claude Code

NeverENG and others added 2 commits August 13, 2026 15:20
PeerPool 此前自建 bannet.Client 并用一把锁把「对同一 peer 的全部转发」串行化在单条连接
上——其注释本就写着「需要更高并发时可扩为每 peer 一个连接池,此处先保正确」。SDK 正是
那个连接池,故每个 peer 改持一个 client.Client:对同一属主的并发转发由多条连接承担,
不再排队。

顺带修掉一处真实缺陷。bannet.Client.Get 对任何非 OK 状态一律返回 (nil, false, nil),
注释写的是「未命中/远端错误:按未命中处理」——于是属主节点的真实故障会被上报为「这个
key 不存在」,上游据此返回空结果。新实现只把 ErrKeyNotFound 记为未命中,其余错误原样
上抛(这一区分正依赖此前为 GET 新增的 notfound 状态)。

转发不重试(MaxRetries<0):转发是「客户端 → 入口节点 → 属主节点」的第二跳,入口侧的
客户端 SDK 已带重试,此处再重试会在过载时两级相乘放大请求量。

随之删除两份冗余实现:
- bannet/client.go(176 行):迁走 PeerPool 与三节点集成测试后已无调用方。
- pkg/utils/message.go(59 行):Frame 接口的第二个实现,仅 bannet/client.go 与压测在
  用;压测改用 bannet.NewMessage。ByteBuilder 仍被压测使用,保留。

线格式实现由 5 份收敛为 3 份:服务端 bannet/datapack.go、SDK client/conn.go、以及压测
自有的一份(压测刻意保留裸往返,避免 SDK 的连接池与重试污染测量)。

三节点转发集成测试已改用 SDK 并通过:写经入口转发到属主、数据只落属主、从第三节点读仍
能转发命中、删除经转发生效。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Frame 当初之所以存在,是因为它有两个实现:bannet.Message 与 pkg/utils.Message。上一步
删掉后者后,它只剩一个实现,按 Go 的惯例就不该再是接口(不要在需要之前定义接口)。
Codec.Pack/UnPack 与 request.msg 改用具体的 *Message。

与删 Server 接口时一样,接口消失后三处仅为绕过它而写的类型断言也随之消失:
connection.go 的 msg.(*Message)、压测与 wire_scan_test 中的 tempMsg.(*bannet.Message)。
这类断言正是「接口只有一个实现」的典型症状——调用方总要把它转回具体类型才能干活。

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

Copy link
Copy Markdown

🐯 BanGD 数据库内核评审

整体风险:🟡 中

变更总结:这个 PR 把集群节点间转发(PeerPool)从自建的多余 bannet.Client 实现收敛到统一的 client SDK(bandb/client)。具体做了四件事:(1) PeerPool 从「每 peer 单连接 + 一把锁串行化全部转发」改为「每 peer 一个 SDK 客户端,SDK 内部连接池承担并发」,把转发路径从串行排队提升为多连接并行;(2) 修掉一处把「属主节点故障」误报成「key 不存在」的缺陷——GET 转发路径改为仅把 ErrKeyNotFound 记为未命中、其余错误上抛;(3) 转发层不再重试(MaxRetries=-1),因为入口侧 SDK 已带重试,避免过载时两级重试相乘放大请求量;(4) 删除 176 行的 bannet/client.go 与 59 行的 utils.Message 两套重复实现,随之删除 Frame 接口及三处仅为绕过它的类型断言,把线格式实现从 5 份收敛到 3 份。

改动触及的层级与不变量:这是网络协议层(线格式 + Frame 接口收敛)与集群转发层(并发模型重构)的改动。收敛的方向(单一实现、删借口)是健康的;核心架构风险集中在转发路径「不再带错误态」的挂起/失败语义,以及 ctx 生命周期与每个请求的 timeout 分配。

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

架构问题(共 3 项)

普通问题(共 3 项)

💡 [建议 · 错误处理] cluster/peerpool.go:45 bandb.New 错误转发被吞

  • peerMaxRetries 注释写「取 0(不重试)」但实际值为 -1,且 -1 在 SDK 的 applyDefaults 里被夹到 0。注释与行为不一致,属于误导性注释。
  • 建议:把常量改为 0,或把 applyDefaults 对负值的处理改为显式保留并更新 SDK 注释;确保注释与行为严格一致。

💡 [建议 · 边界条件] cmd/ban-bench/runner.go:270 recv 对大帧只读一次但不校验

  • recv 中 dp.UnPack(header) 在 MaxPackageSize 超过 0 时会对 DataLen 做上限校验,但 cmd/ban-bench 的 recv 在 UnPack 之后直接按 dataLen 一次性 make + ReadFull。若服务端返回的 dataLen 超过压测可分配大小(内存约束场景),会触发大分配。这个分支在压测大 value(如 ValueSize 1MB)时可能触发。
  • 建议:在 make(data) 前对 dataLen 加一个基于压测 ValueSize 的上限判断,避免意外大分配。

💡 [建议 · 逻辑错误] bannet/wire_scan_test.go:54 测试断言仅比较长度不比较内容

  • TestScanResponseSurvivesWire 里 len(got) != len(entries) 只在数量不符时报错,未校验每条 entry 的 key/value 是否逐字节一致。若服务端编码时数据内容错乱、但条数恰好一致,测试会假绿。
  • 建议:补充逐条比较的断言(如 reflect.DeepEqual 或逐 key 比对),避免测试只验证了数量。

本次评审消耗 token:共 209945 tokens(输入 195321,输出 6176,缓存命中 8448,缓存写入 0)|维度 [concurrency, memory, lock, performance, resource]|补充阅读周边文件 [client/client.go, service/router.go, bannet/server.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