AlexStocks commented on code in PR #3625:
URL: https://github.com/apache/dubbo-go/pull/3625#discussion_r3749843553
##########
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:
[P1] 当前公共 `UnSubscribe` 入口仍没有真正覆盖这个生命周期出口：`SubscribeURL` 以
`ServiceKey:protocol` 保存通知器，但 `UnSubscribe` 仍用 `url.ServiceKey()` 调
`RemoveListener`，所以最后一个订阅者离开后 `len(listeners)` 仍非零，新增的 timer
取消分支不会执行。exact-Head 公开入口探针在 `UnSubscribe` 后 metadata fetch 仍从 4 增至 8。请统一两端
key（最好共用 helper），并用 `SubscribeURL -> UnSubscribe` 的回归测试验证 fetch/timer
停止，而不是只直接测试 `RemoveListener`。
##########
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:
[P1] `closed` 修复了已安装 listener 的在途 refresh，但 `Destroy` 仍看不到尚在初始
`GetInstances`/metadata 阶段的 `SubscribeURL`。该调用在 `Destroy` 返回后完成时仍会安装
listener、`AddListenerAndNotify` 并启动新的无限 timer；exact-Head 阻塞探针中 fetch 在 Destroy
后从 3 增至 7。这里需要 registry 级 `destroyed`/generation 状态，并在安装/附加订阅者前复检、丢弃晚到
listener；请补 `SubscribeURL` 与 `Destroy` 的确定性交错测试。
--
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]
