Alanxtl commented on code in PR #3371:
URL: https://github.com/apache/dubbo-go/pull/3371#discussion_r3362780176
##########
registry/servicediscovery/service_discovery_registry.go:
##########
@@ -298,12 +320,142 @@ func (s *serviceDiscoveryRegistry) IsAvailable() bool {
}
func (s *serviceDiscoveryRegistry) Destroy() {
+ s.stopMetadataTimers()
err := s.serviceDiscovery.Destroy()
if err != nil {
logger.Errorf("[Registry][ServiceDiscovery] destroy
serviceDiscovery catch error, err=%s", err.Error())
}
}
+func (s *serviceDiscoveryRegistry) stopMetadataTimers() {
+ s.lock.Lock()
+ defer s.lock.Unlock()
+ if s.renewAppMetadataTimer != nil {
+ s.renewAppMetadataTimer.Stop()
+ s.renewAppMetadataTimer = nil
+ }
+}
+
+// ========== renewAppMetadata: daily app-level metadata re-publish ==========
+
+func (s *serviceDiscoveryRegistry) startRenewAppMetadataTimer() {
+ if !s.url.GetParamBool(constant.CycleReportKey, true) {
+ return
+ }
+
+ // Run immediately on start
+ if s.url.GetParamBool(constant.MetadataRenewOnStartupKey, true) {
+ go s.doRenewAppMetadata()
+ }
+
+ delay := s.calculateRenewAppMetadataDelay()
+ s.renewAppMetadataTimer = time.AfterFunc(delay, func() {
+ s.doRenewAppMetadata()
+ // Reschedule for next day
+ s.lock.Lock()
+ if s.renewAppMetadataTimer != nil {
+ s.renewAppMetadataTimer.Reset(24 * time.Hour)
+ }
+ s.lock.Unlock()
+ })
+}
Review Comment:
GC/renew 开关读的是 registry URL，不是 metadata-report 配置。
新逻辑在 341 和 415 都从 `s.url` 读 `cycle.report` / `metadata.gc.*` /
`renew-on-startup`。但独立 `dubbo.metadata-report.params` 是走
config/metadata_config.go:91 生成 metadata report URL 的，service discovery
registry 拿不到这些参数。结果是用户单独配置 metadata-report 时，无法关闭 GC 或调整 window，默认
`metadata.gc.enabled=true`、`window=5` 会直接生效并删除数据。
##########
registry/servicediscovery/service_discovery_registry.go:
##########
@@ -85,6 +87,18 @@ func newServiceDiscoveryRegistry(url *common.URL)
(registry.Registry, error) {
}, nil
}
+// startMetadataTimers starts the renewAppMetadata timer if metadata type is
remote.
+// GC runs after each renew cycle inside doRenewAppMetadata.
+func (s *serviceDiscoveryRegistry) startMetadataTimers() {
+ if metadata.GetMetadataType() != constant.RemoteMetadataStorageType {
+ return
+ }
+ if s.metadataReport == nil {
+ return
+ }
+ s.startRenewAppMetadataTimer()
+}
Review Comment:
定时 renew goroutine 会无锁修改全局 `MetadataInfo`。
metadata.GetMetadataInfo /metadata/metadata.go:36 返回的是全局 map 里的指针；
PR 在 service_discovery_registry.go:348 启动后台 goroutine，并在
service_discovery_registry.go:369 直接改 `LastUpdatedTime`、随后
marshal/publish。这个对象同时会被 `AddService` / `RemoveService` / unregister 路径修改
services/exported URLs，存在数据竞争，严重时可能在 JSON marshal map 时撞上并发修改。建议 renew
时复制快照，或者给 metadata registry 引入统一锁。
##########
metadata/report/nacos/report.go:
##########
@@ -214,6 +214,70 @@ func (n *nacosMetadataReport)
RemoveServiceAppMappingListener(key string, group
return n.removeServiceMappingListener(key, group)
}
+// UnPublishAppMetadata removes metadata for a specific revision from nacos.
+// This operation is idempotent — deleting a non-existent config returns false
but no error.
+func (n *nacosMetadataReport) UnPublishAppMetadata(application, revision
string) error {
+ // Delete primary config (compatible with java impl)
+ _, err := n.client.Client().DeleteConfig(vo.ConfigParam{
+ DataId: application,
+ Group: revision,
+ })
+ if err != nil {
+ return perrors.WithMessage(err, "Could not delete the metadata")
+ }
+ // Delete legacy config (compatible with dubbo-go 3.1.x).
+ if _, err = n.client.Client().DeleteConfig(vo.ConfigParam{
+ DataId: application + constant.KeySeparator + revision,
+ Group: n.group,
+ }); err != nil {
+ logger.Warnf("[Metadata][Nacos] could not delete legacy
metadata for app=%s rev=%s: %v",
+ application, revision, err)
+ }
+ return nil
+}
+
+// ListAppRevisions lists all stored revisions for an application from nacos.
+func (n *nacosMetadataReport) ListAppRevisions(application string)
([]report.AppRevision, error) {
+ pageNo, pageSize := 1, 500
+ configs, err := n.client.Client().SearchConfig(vo.SearchConfigParam{
+ Search: "accurate",
+ DataId: application,
+ Group: "",
+ PageNo: pageNo,
+ PageSize: pageSize,
+ })
+ if err != nil {
+ return nil, perrors.WithMessage(err, "Could not search configs
for ListAppRevisions")
+ }
+ if configs == nil || len(configs.PageItems) == 0 {
+ return nil, nil
+ }
+ if int(configs.TotalCount) > len(configs.PageItems) {
+ logger.Warnf("ListAppRevisions for app=%s: total configs (%d)
exceeds page size (%d), "+
Review Comment:
这个logger也加一下前缀
--
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]
