Aias00 opened a new issue, #3532:
URL: https://github.com/apache/dubbo-go/issues/3532
### Problem
The `ServiceDiscovery` interface exposes `AddListener` but **no
`RemoveListener`/`Unsubscribe`**:
```go
// registry/service_discovery.go
type ServiceDiscovery interface {
fmt.Stringer
Destroy() error
Register(instance ServiceInstance) error
Update(instance ServiceInstance) error
Unregister(instance ServiceInstance) error
...
AddListener(listener ServiceInstancesChangedListener) error // no
removal counterpart
}
```
`serviceDiscoveryRegistry.UnSubscribe` only removes the **inner**
`NotifyListener` from the `ServiceInstancesChangedListenerImpl`:
```go
// registry/servicediscovery/service_discovery_registry.go UnSubscribe
l := s.serviceListeners[serviceNamesKey]
if l != nil {
l.RemoveListener(url.ServiceKey()) // only the inner NotifyListener
}
s.stopListen(url)
```
The `ServiceInstancesChangedListenerImpl` itself stays registered in the
backend's `instanceListenerMap` (`zk`/`etcdv3`/`nacos`), and the per-service
watcher the backend started in `AddListener` (`zk` `csd.ListenServiceEvent`,
`nacos` `namingClient.Client().Subscribe`, `etcd`
`EventListener.ListenServiceEvent`) is **never torn down per `UnSubscribe`**.
Only `nacosServiceDiscovery.Destroy` clears the map and unsubscribes (and
`zk`/`etcdv3` `Destroy` do not clear it at all).
So every consumer subscribe creates a permanent entry that keeps receiving
`OnEvent` — rebuilding `serviceUrls`/`revisionToMetadata`/fetching metadata via
`GetMetadataInfo` even when its `listeners` map is empty — for the life of the
process.
### Current behavior
1. `SubscribeURL` → backend `AddListener` starts a per-service watcher +
adds the listener to `instanceListenerMap`.
2. `UnSubscribe` → only removes the inner `NotifyListener`; the backend
watcher goroutine and the `instanceListenerMap` entry remain.
3. Subsequent instance-change events still fire `OnEvent` for the now-empty
listener, doing wasted metadata/URL rebuild work.
4. Repeated Subscribe/UnSubscribe cycles (dynamic reference reload, config
refresh) accumulate leaked watcher goroutines + map entries.
### Expected behavior
When the last `NotifyListener` for a `serviceNamesKey` is removed by
`UnSubscribe`, the registry should also remove the
`ServiceInstancesChangedListenerImpl` from the backend (and stop its
per-service watcher), so no leaked goroutine/map entry remains and no wasted
`OnEvent` work runs for an empty listener set.
### Suggested approach
This is an API-surface decision; two options:
- **Non-breaking (preferred):** introduce an *optional* capability
interface, e.g. `ListenerRemover {
RemoveListener(ServiceInstancesChangedListener) error }`, implemented by the
backends that support per-listener removal. `UnSubscribe` type-asserts
`s.serviceDiscovery` to it; when the listener's `NotifyListener` set is empty
it calls `RemoveListener`, the backend removes the entry from
`instanceListenerMap[serviceName]`, and when the set is empty stops the
per-service watcher (nacos `Unsubscribe`; etcd cancels the watch context; zk
cancels the curator watcher) and deletes the map entry.
- **Breaking:** add `RemoveListener` directly to `ServiceDiscovery`. Simpler
call site, but breaks source compatibility for any external `ServiceDiscovery`
implementation (same concern flagged on #3439).
Either way, `UnSubscribe` needs a way to observe "the listener has no more
`NotifyListener`s" (e.g. an `IsEmpty()`/`ListenerCount()` accessor on
`ServiceInstancesChangedListenerImpl`).
### Acceptance criteria
- [ ] After `UnSubscribe` removes the last `NotifyListener` for a
`serviceNamesKey`, the backend watcher for that service is stopped and the
`instanceListenerMap` entry is removed.
- [ ] No leaked per-service watcher goroutine across Subscribe/UnSubscribe
cycles.
- [ ] No `OnEvent` metadata/URL rebuild for an empty listener set.
- [ ] External `ServiceDiscovery` implementations continue to compile (if
the non-breaking option is chosen).
- [ ] Concurrency test: Subscribe/UnSubscribe churn under `-race` does not
grow goroutines or map entries.
--
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]
