Alanxtl commented on code in PR #3625:
URL: https://github.com/apache/dubbo-go/pull/3625#discussion_r3746534115
##########
registry/servicediscovery/service_discovery_registry.go:
##########
@@ -323,6 +323,15 @@ func (s *serviceDiscoveryRegistry) IsAvailable() bool {
func (s *serviceDiscoveryRegistry) Destroy() {
s.stopMetadataTimers()
+ s.lock.Lock()
+ for _, l := range s.serviceListeners {
+ // Destroy drops listeners without RemoveListener; cancel any
pending
+ // metadata retry so its timer cannot leak.
+ if impl, ok := l.(*ServiceInstancesChangedListenerImpl); ok {
+ impl.stopMetadataRetry()
+ }
+ }
+ s.lock.Unlock()
err := s.serviceDiscovery.Destroy()
if err != nil {
Review Comment:
`Destroy()` 不能保证阻止正在执行的重试重新创建 timer
如果 timer callback 已经把 `retryTimer` 置空，或 `refreshServiceURLs()` 正在进行
metadata RPC，`Destroy()` 调用 `stopMetadataRetry()` 后，刷新流程仍会执行到：
```go
lstn.scheduleMetadataRetry()
```
于是 registry 已销毁后又创建新的无限重试 timer。需要增加 `closed/destroyed` 状态或 generation
检查，并在刷新完成后再次确认 listener 未销毁。
##########
registry/servicediscovery/service_instances_changed_listener_impl.go:
##########
@@ -343,3 +409,94 @@ func GetMetadataInfo(app string, instance
registry.ServiceInstance, revision str
metaCache.Set(cacheKey, metadataInfo)
return metadataInfo, nil
}
+
+var (
+ // metadataRetryInitialDelay is the first backoff delay before retrying
a failed
+ // metadata fetch. Package-level so tests can shrink it.
+ metadataRetryInitialDelay = time.Second
+ // metadataRetryMaxDelay caps the backoff. The retry count itself is
+ // intentionally unlimited: retries only target instances the registry
still
+ // reports as alive, and a capped count would re-introduce the permanent
+ // empty-directory failure this mechanism fixes.
+ metadataRetryMaxDelay = 30 * time.Second
+ // metadataFetchFailureLogInterval throttles repeated fetch-failure
warnings
+ // for the same revision key.
+ metadataFetchFailureLogInterval = 5 * time.Minute
+)
+
+// metadataInfoFetcher resolves MetadataInfo for a revision; a package-level
+// indirection so tests can inject transient failures.
+var metadataInfoFetcher = GetMetadataInfo
+
+// stopMetadataRetry cancels any pending metadata retry. It is called when the
+// owning registry is destroyed and drops this listener without RemoveListener,
+// so the retry timer cannot leak.
+func (lstn *ServiceInstancesChangedListenerImpl) stopMetadataRetry() {
+ lstn.mutex.Lock()
+ defer lstn.mutex.Unlock()
+ if lstn.retryTimer != nil {
+ lstn.retryTimer.Stop()
+ lstn.retryTimer = nil
+ }
+}
+
+// scheduleMetadataRetry arms the shared retry timer while unresolved revisions
+// remain. Retries replay the latest instance snapshot, so revisions whose
+// instances disappeared from the registry are dropped naturally on the next
run.
+func (lstn *ServiceInstancesChangedListenerImpl) scheduleMetadataRetry() {
+ lstn.mutex.Lock()
+ defer lstn.mutex.Unlock()
+ if len(lstn.unresolvedRevisions) == 0 {
+ if lstn.retryTimer != nil {
+ lstn.retryTimer.Stop()
+ lstn.retryTimer = nil
+ }
+ lstn.retryAttempts = 0
+ return
+ }
+ if lstn.retryTimer != nil {
+ // One shared timer per listener: repeated events must not
multiply retries.
+ return
+ }
Review Comment:
订阅者移除后，重试 timer 仍可能无限重建
`scheduleMetadataRetry()` 只检查 `unresolvedRevisions`，没有检查
`len(listeners)`。因此：
- 初始 `OnEvent` 在 listener 安装前失败，会先启动 timer；
- 并发订阅中被丢弃的临时 listener 可能永久重试；
- `RemoveListener` 与 timer callback 竞争时，callback 仍会在无订阅者后重新调度。
结果是持续 metadata RPC、日志输出，并通过 timer closure 持有 listener。建议调度和 callback 中都检查
listener 是否仍存在，并为被丢弃的临时 listener 增加清理路径。
--
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]
