xiaobaicai66695 opened a new pull request, #3600: URL: https://github.com/apache/dubbo-go/pull/3600
### 说明 Fixes #3569 本 PR 保证服务发现 `CacheManager` 在并发更新期间生成一致的缓存快照，并使定时落盘任务能够可靠、幂等地停止。 ### 问题分析 进一步检查发现，项目通过 `*lru.Cache` 使用 `github.com/hashicorp/golang-lru v0.5.4`。该类型本身是并发安全的，其 `Get`、`Add`、`Remove`、`Keys`、`Len` 和 `Purge` 均有内部锁保护。 实际问题发生在复合操作层面： 1. `GetAll` 先通过 `Keys()` 获取键快照。 2. 另一个 goroutine 可能在此时删除其中一个键。 3. 后续 `Get(key)` 返回 `(nil, false)`。 4. 原实现忽略了布尔返回值，将 `nil` 写入结果。 5. `dumpCache` 调用 `gob.Register(nil)`，最终触发 panic。 因此，可复现的问题是缓存快照不具备原子性，并在定时落盘时引发崩溃，而不是底层 LRU 双向链表发生了无锁数据竞争。 原 `StopDump` 还会在持有 `CacheManager.lock` 时向无缓冲通道发送停止信号。如果落盘 goroutine 正在等待同一把锁，停止流程存在死锁窗口。 ### 修改内容 - 使用同一把互斥锁保护所有 `CacheManager` 缓存操作。 - 保证 `GetAll` 相对于 `Get`、`Set` 和 `Delete` 是原子快照。 - 忽略已经不存在的键，避免将 `nil` 写入落盘数据。 - 缓存加载统一通过加锁后的 `Set` 方法执行。 - 文件写入前先生成一致的缓存快照。 - 使用 `sync.Once` 保证 `StopDump` 可以安全重复调用。 - 使用 `done` 通道等待落盘 goroutine 完全退出。 - goroutine 退出时停止 ticker 并关闭 `done`。 - 增加并发访问、落盘重载和并发停止回归测试。 ### 回归测试设计 `TestCacheManagerConcurrentAccess` 启动 16 个 goroutine。每个 goroutine 执行 500 轮并发 `Set`、`Get`、`Delete` 和 `GetAll`，同时启用 1ms 周期的定时落盘任务。 每轮测试包含： - 8,000 次 `Set` - 8,000 次 `Get` - 2,672 次 `Delete` - 1,152 次 `GetAll` 并发操作结束后，测试还会： - 等待定时落盘文件生成； - 停止落盘 goroutine； - 从落盘文件重新加载缓存； - 验证缓存可正常解码、值不为 nil 且类型正确。 `TestCacheManagerConcurrentStopDump` 分别覆盖开启和关闭落盘两种模式，每种模式由 64 个 goroutine 同时调用 `StopDump`，并验证重复停止不会阻塞或死锁。 ### 自测报告 测试环境：Go 1.25.5、Windows/AMD64、`CGO_ENABLED=1`、32 个逻辑处理器。 | 测试命令 | 结果 | | --- | --- | | `go test -count=1 ./registry/servicediscovery/store` | 通过，9.337s | | `go test -race -count=1 ./registry/servicediscovery/store` | 通过，10.810s | | 目标回归测试 `-count=20` | 通过，2.239s | | 目标回归测试 `-race -count=20` | 通过，4.412s | | `go test -count=1 ./registry/servicediscovery/...` | 全部通过 | | `go vet ./registry/servicediscovery/...` | 通过，无诊断信息 | | store 包语句覆盖率 | 90.2% | 20 轮 race 回归共执行 396,480 次缓存读写及快照操作，并执行 2,560 次并发 `StopDump` 调用。 ### 覆盖率明细 - `Get`、`Set`、`Delete`、`GetAll`、`StopDump`、`destroy`：100% - `runDumpTask`：90.9% - `getAllLocked`：88.9% - `loadCache`：84.6% - `dumpCache`：83.3% - `NewCacheManager`：83.3% - 总语句覆盖率：90.2% ### 性能测试 32 线程环境下，每项执行 5 次 1 秒 benchmark，取中位数： | 操作 | 修改前 | 修改后 | 变化 | | --- | ---: | ---: | ---: | | 并行 `Get` | 103.2 ns/op | 121.0 ns/op | +17.2% | | 混合读写 | 114.2 ns/op | 125.0 ns/op | +9.5% | | 并行 `GetAll` | 16.179 μs/op | 5.207 μs/op | -67.8% | 由于底层缓存本身也有锁，外层互斥锁会增加单次操作开销；但原子 `GetAll` 的性能明显提升，内存分配由 10,520 B / 12 allocs 降至约 6,104 B / 5 allocs。 ### 已知但不属于本 PR 的问题 - `go test -race ./registry/servicediscovery/...` 会在既有的 `TestServiceDiscoveryRegistryUnRegister_Concurrent` 中报告竞态。该测试明确无锁修改 `sdReg.instances`，在基线提交上同样失败，与本 PR 无关。store 范围的 race 测试全部通过。 - 显式调用 `Set(key, nil)` 仍可能在 `gob.Register(nil)` 处 panic。生产元数据路径会拒绝 nil，建议单独处理。 - 缓存文件仍直接写入最终路径，没有使用临时文件加原子重命名。本 PR 未修改该既有行为。 ### 检查清单 - [x] 确认目标分支为 `develop` - [x] 代码已通过本地测试 - [x] 已添加能够证明修复有效的回归测试 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
