refactor(cluster): 删死代码与已失真的桩，上提为顶层包

[NeverENG]

起因是一个问题：service/cluster 这层逻辑到底有什么用？逐符号查了外部引用后，答案是一部分在真跑，一部分从落地起就没产生过任何行为。

一、它到底在做什么

已在生产路径上运行（有调用方、有行为）

组件	谁在用
HashRing / ShardOf / ShardReplicas	service/shardkv 的分片归属、Router 的属主计算
P（P2C 两选一延迟择优）	shardkv 的转发读
PeerPool	Router 的跨节点属主转发

仍是骨架，当前不产生行为

• Registry 的存活视图：Heartbeat 无任何非测试调用方，且生产代码传入的 TTL 是 100*365*24*time.Hour（100 年）。因此 IsAlive 恒为 true，Placement.OwnerOf 与 HashRing.NodeFor 行为等价——我核对过两者的计算路径逐步相同（searchLocked(hashKey(key)) → hashToNode[sortedHashes[idx]]）。
• Placement.Failover：有行为（把节点摘出环），但没有故障检测来触发它，零调用方。

二、删掉的

完全死代码：BoundedRing（101 行 + 88 行测试）。有界负载一致性哈希，全仓零引用，连 cluster 包内部都没用到——实际承担归属计算的一直是 HashRing。

注释已与事实不符的桩（三者均零调用方）：Placement.Forward、ForwardFunc、Placement.Rebalance。它们的说明写着「跨节点数据传输未实现，属传输层重写范围」——该前提已不成立：跨节点转发早已由 Router + PeerPool 经 BanNet 落地（见 iteration-2026-08-05-shard-routing-banNet）。留一个说明失真的桩比删掉更糟：读者会据此以为该能力缺失。gateway.go 随之只剩 IsLocal，并入 placement.go 后删除。

按讨论保留 Registry 与 Failover 作为存活视图的骨架。

三、把「恒存活」这件事说清楚

生产代码原先直接写 100*365*24*time.Hour 这个魔数，等于把「尚无心跳」伪装成一个会过期的存活窗口。改为具名常量并写明理由——这里不能换成有限 TTL：没有心跳刷新，任何有限窗口都会让全部节点在窗口后被判失联，OwnerOf 返回空串、路由整体中断。

四、上提为顶层 cluster/

它只依赖 bannet、不依赖 service，且同时被 service 与 service/shardkv 使用；嵌在 service/ 下会误导归属。它是与 storage/raft/bannet 同级的分布式控制面。

包文档重写为「哪些已在生产路径上跑、哪些仍是不产生行为的骨架」，把上面这份判断固定在代码里，而不是只留在 PR 里。

五、让记录保持诚实

删掉代码后，iteration-2026-08-05-bounded-load-consistent-hashing 与 distributed-delivery-cluster-skeleton 会描述不存在的东西——这正是我们删 architecture-diagram.md 的理由。但它们与那张图不同：它们是带日期的决策记录，并没有说错当时的事。故保留原文，仅在文末追加「后续状态」说明实现已移除及原因。

验证

go build ./...（含 -tags pprof）、go vet ./...、go test -race ./...、gofmt 全绿；scripts/bench.sh 实跑通过。顺带：p2c.go 长期未通过 gofmt 的问题在本次编辑中一并修复。

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added cluster creation from peer addresses with configurable liveness tracking.
  • Added connection pooling for reliable peer-to-peer PUT, GET, and DELETE operations.
  • Added heartbeat monitoring and key ownership checks.
  • Improved routing to prefer faster backends.

• Changes

  • Removed the unused bounded-load routing capability and legacy forwarding stubs.
  • Consolidated cluster functionality under the primary cluster package.

• Documentation & Tests

  • Updated cluster documentation and expanded coverage for routing, sharding, liveness, and performance behavior.

[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: f3186b0d-946c-462f-b301-0639189278af

📥 Commits

Reviewing files that changed from the base of the PR and between 2241e4f and 2f8b7b5.

📒 Files selected for processing (19)

• cluster/bootstrap.go
• cluster/p2c.go
• cluster/p2c_test.go
• cluster/peerpool.go
• cluster/placement.go
• cluster/registry.go
• cluster/registry_test.go
• cluster/routing.go
• cluster/routing_test.go
• docs/distributed-delivery-cluster-skeleton.md
• docs/iteration-2026-08-05-bounded-load-consistent-hashing.md
• service/cluster/bounded_ring.go
• service/cluster/bounded_ring_test.go
• service/cluster/gateway.go
• service/cluster_bootstrap.go
• service/router.go
• service/shard_routing_integration_test.go
• service/shardkv/read.go
• service/shardkv/shardkv.go

---

📝 Walkthrough

Walkthrough

The cluster package adds TTL-based peer liveness, peer connection pooling, and bootstrap helpers. Routing and P2C behavior gain broader tests. Service imports move to the top-level package. Legacy bounded-ring and forwarding stubs are removed.

Changes

Cluster runtime

Layer / File(s)	Summary
Liveness and placement bootstrap cluster/registry.go, cluster/bootstrap.go, cluster/placement.go, cluster/registry_test.go	Adds a concurrency-safe heartbeat registry with injectable clocks, TTL checks, sorted alive-node output, peer bootstrap, and Placement.IsLocal. Removes the unused rebalance stub.
Peer connection pooling cluster/peerpool.go	Adds lazy, per-peer BanNet client reuse with serialized operations, failure cleanup, PUT/GET/DELETE forwarding, and shutdown.
Routing and balancer validation cluster/routing_test.go, cluster/p2c.go, cluster/p2c_test.go, cluster/routing.go	Adds coverage for hash-ring stability, distribution, node changes, shard bounds, replicas, P2C latency preference, and empty or single-backend behavior.
Package migration and legacy removal service/cluster_bootstrap.go, service/router.go, service/shard_routing_integration_test.go, service/shardkv/read.go, service/shardkv/shardkv.go, service/cluster/bounded_ring.go, service/cluster/gateway.go, docs/*	Moves service imports to cluster, names the assumed-alive TTL, removes BoundedRing and forwarding stubs, and updates design-status documentation.

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

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant PeerPool
  participant BanNetClient
  Client->>PeerPool: Submit PUT, GET, or DELETE
  PeerPool->>BanNetClient: Dial or reuse cached connection
  BanNetClient-->>PeerPool: Return operation result
  PeerPool-->>Client: Return forwarded result

Loading

✨ 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/cluster-cleanup

---

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：把 service/cluster 包上提为顶层 cluster/ 包（因它只依赖 bannet、不依赖 service，且被 service 与 shardkv 共用），删除了三类代码：1) 全仓零引用的 BoundedRing（有界负载一致性哈希，201 行代码+测试）；2) 「注释已与事实不符」的桩 Placement.Forward/ForwardFunc/Placement.Rebalance（跨节点转发已由 Router+PeerPool 经 BanNet 落地，桩的描述前提已不成立），连带合并 gateway.go 的 IsLocal 进 placement.go 后删除该文件；3) 把生产代码里 100*365*24*time.Hour 这个「恒活」魔数提为具名常量 assumeAliveTTL 并写明理由。同时在 package 文档和两份决策记录（iteration docs）里追加「后续状态」说明，让记录与代码保持一致。此 PR 不改变任何运行行为——被删的组件本来就没有调用方，被移的包纯搬运。核心判断（Registry/Failover 无故障检测触发、恒活视图不产生行为）已在 package 文档中显式固定。

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

架构问题（共 2 项）

• [并发] IsLocal 在归属节点停机时返回 false 且无降级 cluster/placement.go → 🐯 [并发] IsLocal 在归属节点停机时返回 false 且无降级 · placement.go #266
• [兼容] 恒活 TTL 是信用债，接入心跳前无到期 service/cluster_bootstrap.go → 🐯 [兼容] 恒活 TTL 是信用债，接入心跳前无到期 · cluster_bootstrap.go #267

普通问题（共 1 项）

⚠️ [重要 · 逻辑错误] service/cluster_bootstrap.go:23 恒活 TTL 与 package 文档对 Heartbeat 无调用方 的判断依赖线性演化

• 本 PR 在文档中论证 IsAlive 恒为 true 的依据是「Heartbeat 无任何非测试调用方」。这个论证在当前代码成立，但它是一个运行时断言而非编译时约束：一旦未来某处接入心跳但调用方仍沿用 assumeAliveTTL（比如接入方不知道要改 TTL），就会得到「心跳在刷、TTL 是 100 年」的矛盾状态——心跳刷新最后一个 lastSeen，但恒活 TTL 让失效判定永远不会触发，代码会「看起来在判活」实则恒活。注释已写明「接入心跳后应改为真实的判活窗口」，但这个约束没有以类型/结构强制，靠的是维护者的阅读自觉。建议在 NewClusterFromPeers 或 NewRegistry 调用处加一处 ttl >= 24*365*time.Hour 的断言或 log.Warn，让「恒活 TTL 被用于真实心跳场景」在启动日志里被立即暴露，而非静默地留着。
• 建议：在 EnableShardRoutingFromConfig 传入 assumeAliveTTL 后、或 cluster.NewClusterFromPeers 内部，若 TTL 超过阈值（如 365 天）且后续引入的心跳刷新存在，输出一条 slog.Warn 说明「Assume-alive TTL in effect; heartbeat will not expire nodes」。低成本、防回溯。

---

_{本次评审消耗 token：共 113534 tokens（输入 100074，输出 3860，缓存命中 9600，缓存写入 0）｜维度 [concurrency, memory, lock, storage, schema]｜补充阅读周边文件 [cluster/registry.go, cluster/peerpool.go]｜对抗式复核 3 票/条，过滤疑似误报 0 条}