AlexStocks commented on code in PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#discussion_r3610089431
##########
registry/servicediscovery/service_discovery_registry.go:
##########
@@ -532,23 +537,28 @@ func (s *serviceDiscoveryRegistry) SubscribeURL(url
*common.URL, notify registry
protocol = url.Protocol
}
protocolServiceKey := url.ServiceKey() + ":" + protocol
- listener := s.serviceListeners[serviceNamesKey]
- if listener == nil {
- listener =
NewServiceInstancesChangedListener(url.GetParam(constant.ApplicationKey, ""),
s.url.GetParam(constant.RegistryIdKey, constant.DefaultKey), services)
- for _, serviceNameTmp := range services.Values() {
- serviceName := serviceNameTmp.(string)
- instances :=
s.serviceDiscovery.GetInstances(serviceName)
- logger.Infof("[Registry][ServiceDiscovery] synchronized
instance notification on application %s subscription, instance list size %s",
serviceName, len(instances))
- err =
listener.OnEvent(®istry.ServiceInstancesChangedEvent{
- ServiceName: serviceName,
- Instances: instances,
- })
- if err != nil {
- logger.Warnf("[Registry][ServiceDiscovery]
ServiceInstancesChangedListenerImpl handle error, err=%v", err)
+ var listener registry.ServiceInstancesChangedListener
+ func() {
+ s.lock.Lock()
+ defer s.lock.Unlock()
+ listener = s.serviceListeners[serviceNamesKey]
+ if listener == nil {
+ listener =
NewServiceInstancesChangedListener(url.GetParam(constant.ApplicationKey, ""),
s.url.GetParam(constant.RegistryIdKey, constant.DefaultKey), services)
+ for _, serviceNameTmp := range services.Values() {
+ serviceName := serviceNameTmp.(string)
+ instances :=
s.serviceDiscovery.GetInstances(serviceName)
Review Comment:
[P1] 不要持有全局 registry 锁执行外部查询和元数据回调
当前写锁从 map 查重一直覆盖 `GetInstances` 和 `listener.OnEvent`。后者会继续执行
`GetMetadataInfo`，其路径可能访问 metadata report/RPC，并重建、通知服务 URL；任一外部调用阻塞时，所有使用同一个
`s.lock` 的注册、注销、订阅和映射更新都会被一起阻塞。这个锁只应保护 `serviceListeners` 的
check/install，不应包住无界外部工作。建议使用按 `serviceNamesKey` 的
singleflight/初始化状态，或先在锁外构造并加载 listener，再在短写锁内 double-check 后安装；同时增加一个让
`GetInstances` 或元数据获取阻塞的并发测试，验证其他 key 的订阅/注销不会被全局锁卡住。
--
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]
