Copilot commented on code in PR #2859:
URL: https://github.com/apache/dubbo-go/pull/2859#discussion_r2072491745
##########
registry/nacos/registry.go:
##########
@@ -177,46 +177,62 @@ func (nr *nacosRegistry) Subscribe(url *common.URL,
notifyListener registry.Noti
return nil
}
serviceName := url.GetParam(constant.InterfaceKey, "")
- var serviceNames []string
- var err error
if serviceName == constant.AnyValue {
- serviceNames, err = nr.getAllSubscribeServiceNames(url)
- if err != nil {
- return err
- }
+ nr.subscribeAll(url, notifyListener)
+ go func() {
+ // scheduled lookup for new service
+ for {
+ nr.subscribeAll(url, notifyListener)
+ time.Sleep(LookupInterval)
+ }
+ }()
+ return nil
} else {
- serviceNames = []string{getSubscribeName(url)}
+ // retry forever
+ for {
+ return nr.subscribe(getSubscribeName(url),
notifyListener)
+ }
}
- return nr.subscribe(serviceNames, notifyListener)
}
-// subscribe subscribe services
-func (nr *nacosRegistry) subscribe(serviceNames []string, notifyListener
registry.NotifyListener) error {
+func (nr *nacosRegistry) subscribeAll(url *common.URL, notifyListener
registry.NotifyListener) {
+ groupName := nr.URL.GetParam(constant.RegistryGroupKey, defaultGroup)
+ serviceNames, err := nr.getAllSubscribeServiceNames(url)
+ if err != nil {
+ logger.Warnf("getAllServices() = err:%v",
perrors.WithStack(err))
+ return
+ }
if len(serviceNames) == 0 {
logger.Warnf("No services to listen to.")
- return nil
+ return
}
- for {
- if !nr.IsAvailable() {
- logger.Warnf("event listener game over.")
- return perrors.New("nacosRegistry is not available.")
- }
- var err error
- for _, serviceName := range serviceNames {
- listener :=
NewNacosListenerWithServiceName(serviceName, nr.URL, nr.namingClient)
- err = listener.listenService(serviceName)
- metrics.Publish(metricsRegistry.NewSubscribeEvent(err
== nil))
- if err != nil {
- logger.Warnf("getAllServices() = err:%v",
perrors.WithStack(err))
- time.Sleep(time.Duration(RegistryConnDelay) *
time.Second)
- break
- }
- go nr.handleServiceEvents(listener, notifyListener)
- }
- if err == nil {
- break
+ for _, name := range serviceNames {
+ if _, ok := listenerCache.Load(name + groupName); ok {
+ continue
}
+ err = nr.subscribe(name, notifyListener)
+ logger.Warnf("subscribe service %s err:%v", name,
perrors.WithStack(err))
Review Comment:
The code logs a warning for every subscription attempt regardless of whether
an error occurred. It is recommended to log only when 'err' is not nil to avoid
misleading log messages.
```suggestion
if err != nil {
logger.Warnf("subscribe service %s err:%v", name,
perrors.WithStack(err))
}
```
##########
registry/nacos/registry.go:
##########
@@ -256,14 +272,14 @@ func (nr *nacosRegistry) handleServiceEvents(listener
registry.Listener, notifyL
listener.Close()
return
}
- logger.Infof("[Nacos Registry] Update begin, service event:
%v", serviceEvent.String())
+ logger.Infof("[Nacos Registry] inUpdate beg, service event:
%v", serviceEvent.String())
Review Comment:
[nitpick] The log message '[Nacos Registry] inUpdate beg' appears to be a
typographical error. Consider reverting it to '[Nacos Registry] Update begin'
for consistency and clarity.
```suggestion
logger.Infof("[Nacos Registry] Update begin, service event:
%v", serviceEvent.String())
```
--
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]
