AlexStocks commented on code in PR #3270:
URL: https://github.com/apache/dubbo-go/pull/3270#discussion_r2985392776
##########
registry/directory/directory.go:
##########
@@ -187,26 +182,49 @@ func (dir *RegistryDirectory) Subscribe(url *common.URL)
error {
timeout, _ = time.ParseDuration(constant.DefaultRegTimeout)
}
- done := make(chan struct{})
+ serviceKey := url.Key()
+ // Registration is bounded by registry timeout (default 5s).
+ // On timeout, we return immediately and skip starting subscribe
goroutine.
+ if err := dir.registerConsumerWithTimeout(url, timeout, serviceKey);
err != nil {
+ return err
+ }
go func() {
- urlToReg := getConsumerUrlToRegistry(url)
- err := dir.registry.Register(urlToReg)
- if err != nil {
- logger.Errorf("consumer service %v register registry %v
error, error message is %v",
- url.String(), dir.registry.GetURL().String(),
err)
+ if err := dir.registry.Subscribe(url, dir); err != nil {
+ logger.Error("registry.Subscribe(url:%v, dir:%v) =
error:%v", url, dir, err)
}
+ }()
- close(done)
+ logger.Infof("register completed successfully for service: %s",
serviceKey)
+ return nil
+}
+
+func (dir *RegistryDirectory) registerConsumerWithTimeout(url *common.URL,
timeout time.Duration, serviceKey string) error {
+ registerErrCh := make(chan error, 1)
+ go func() {
+ urlToReg := getConsumerUrlToRegistry(url.Clone())
+ registerErrCh <- dir.registry.Register(urlToReg)
}()
+ timer := time.NewTimer(timeout)
+ defer timer.Stop()
+
select {
- case <-done:
- logger.Infof("register completed successfully for service: %s",
url.Key())
+ case err := <-registerErrCh:
+ if err != nil {
+ registryURL := dir.registry.GetURL()
+ registryURLString := ""
+ if registryURL != nil {
+ registryURLString = registryURL.String()
+ }
+ logger.Errorf("consumer service %v register registry %v
error, error message is %v",
+ url.String(), registryURLString, err)
+ return err
+ }
return nil
- case <-time.After(timeout):
- logger.Errorf("register timed out for service: %s", url.Key())
- return fmt.Errorf("register timed out for service: %s",
url.Key())
+ case <-timer.C:
Review Comment:
这里的超时只是让调用方提前返回，后台 `Register` goroutine 还会继续跑。如果它在超时之后成功，consumer 实际上已经注册了，但
`Subscribe` 不会启动，后续也没有补偿 `UnRegister`，状态会变成“返回失败但部分初始化成功”。建议把取消能力下推到
`registry.Register`，或者在超时后处理 late success 并做回滚。
##########
registry/directory/directory.go:
##########
@@ -523,9 +541,7 @@ func (dir *RegistryDirectory) List(invocation
protocolbase.Invocation) []protoco
routerChain := dir.RouterChain()
if routerChain == nil {
- dir.invokersLock.RLock()
- defer dir.invokersLock.RUnlock()
- return dir.cacheInvokers
+ return dir.snapshotCacheInvokers()
}
return routerChain.Route(dir.consumerURL, invocation)
Review Comment:
这里虽然把 `routerChain == nil` 的分支换成了快照，但 `routerChain.Route()` 仍然会无锁读
`RouterChain.invokers`，同时 `setNewInvokers()` 又会在另一个 goroutine 里通过
`SetInvokers()` 写这份切片。当前 HEAD 直接跑 `go test -race ./registry/directory
./registry/nacos` 仍会在 `Test_List` 报 race。建议在 `RouterChain` 内部先做带锁 snapshot
再路由，只修 `cacheInvokers` 读取还不够。
--
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]
