Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
sonarqubecloud[bot] commented on PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371#issuecomment-4806026623 ## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) **Quality Gate passed** Issues  [4 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3371&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3371&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3371&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3371&metric=new_coverage&view=list)  [16.6% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3371&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) -- 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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
Alanxtl merged PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371 -- 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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
eye-gu commented on PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371#issuecomment-4797789491 > help resolve confilct Resolved. -- 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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
sonarqubecloud[bot] commented on PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371#issuecomment-4797774416 ## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) **Quality Gate passed** Issues  [4 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3371&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3371&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3371&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3371&metric=new_coverage&view=list)  [16.6% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3371&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) -- 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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
Alanxtl commented on PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371#issuecomment-4797494578 help resolve confilct -- 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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
eye-gu commented on PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371#issuecomment-4665921148 > @eye-gu fix file confliction 已经rebase develop @Alanxtl 添加了测试 -- 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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
sonarqubecloud[bot] commented on PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371#issuecomment-4665919718 ## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) **Quality Gate passed** Issues  [4 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3371&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3371&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3371&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3371&metric=new_coverage&view=list)  [18.3% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3371&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) -- 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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
AlexStocks commented on PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371#issuecomment-4665587552 @eye-gu fix file confliction -- 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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
Alanxtl commented on PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371#issuecomment-4655880459 合并顺序 #3360、#3362 先合，低耦合。 #3367 作为并发安全底座。 #3369 合入，建立 registryId/report/cache 作用域。 #3370 rebase 到 #3367 + #3369 之后，特别检查 revision 计算不要绕开锁。 #3371 再合，接受 MetadataReport 接口扩展，并补确认 Snapshot() 与 #3367 锁语义一致。 #3373 最后 rebase，因为它和 #3371 同改 report backends；语义不重复，但文件冲突概率高。 -- 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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
Alanxtl commented on PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371#issuecomment-4655867589 1. 当前测试覆盖率偏低，建议至少补一两个 timer/GC 核心分支测试，尤其是 stale revision 被清理、alive revision 被保留这两个路径。 2. doRenewAppMetadata 里依赖 MetadataInfo.Snapshot()，合并 #3367 后要确认 Snapshot 是带锁/深拷贝语义，避免 renew 时读写 metadata 产生 race -- 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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
eye-gu commented on code in PR #3371:
URL: https://github.com/apache/dubbo-go/pull/3371#discussion_r3367383566
##
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()
+ })
+}
+
+func (s *serviceDiscoveryRegistry) doRenewAppMetadata() {
+ registryID := s.url.GetParam(constant.RegistryIdKey, "")
+ metaInfo := metadata.GetMetadataInfo(registryID)
+ if metaInfo == nil || metaInfo.Revision == "0" {
+ return
+ }
+ metaInfo.LastUpdatedTime = time.Now().UnixMilli()
+ if err := s.metadataReport.PublishAppMetadata(metaInfo.App,
metaInfo.Revision, metaInfo); err != nil {
+ logger.Errorf("[Metadata][renewAppMetadata] failed to
re-publish metadata for app=%s revision=%s: %v", metaInfo.App,
metaInfo.Revision, err)
+ } else {
+ logger.Infof("[Metadata][renewAppMetadata] refreshed metadata
for app=%s revision=%s", metaInfo.App, metaInfo.Revision)
+ }
+
+ // Run garbage collection if enabled, after each renew cycle
+ if s.url.GetParamBool(constant.MetadataGCEnabledKey, true) {
+ s.doGarbageCollect()
+ }
+}
+
+func (s *serviceDiscoveryRegistry) calculateRenewAppMetadataDelay()
time.Duration {
+ now := time.Now()
+ // Next day 2:00 AM
+ nextDay2AM := time.Date(now.Year(), now.Month(), now.Day()+1, 2, 0, 0,
0, now.Location())
+ // Add random offset 0~4 hours to avoid thundering herd
+ randomOffset := time.Duration(rand.Int64N(int64(4 * time.Hour)))
+ return time.Until(nextDay2AM) + randomOffset
+}
+
+// == GC: stale revision cleanup ==
+
+func (s *serviceDiscoveryRegistry) doGarbageCollect() {
+ registryID := s.url.GetParam(constant.RegistryIdKey, "")
+ metaInfo := metadata.GetMetadataInfo(registryID)
+ if metaInfo == nil {
+ return
+ }
+ app := metaInfo.App
+ if app == "" {
+ return
+ }
+
+ // Step 1: List all revisions for this app
+ revisions, err := s.metadataReport.ListAppRevisions(app)
+ if err != nil {
+ logger.Warnf("[Metadata][GC] failed to list app revisions: %v",
err)
+ return
+ }
+ if len(revisions) == 0 {
+ return
+ }
+
+ // Step 2: Filter stale candidates (exceed GC window in days)
+ gcWindowDays := s.url.GetParamByIntValue(constant.MetadataGCWindowKey,
5)
+ if gcWindowDays <= 0 || gcWindowDays > 365 {
+ gcWindowDays = 5
+ }
+ cutoff := time.Now().AddDate(0, 0, -gcWindowDays).UnixMilli()
+ candidates := make(map[string]bool)
+ for _, rev := range revisions {
+ // Skip special revisions
+ if rev.Revision == "0" || rev.Revision == "N/A" || rev.Revision
== "" || rev.Revision == metaInfo.Revision {
+ continue
+ }
+ // ModifyTime == 0 means old metadata produced by a version
that does not set
+ // lastUpdatedTime — never garbage-collect such entries. Only
delete when the
+ // revision is older than gcWindow and no alive instance
references it.
+ if rev.ModifyTime > 0 && rev.ModifyTime < cutoff {
+ candidates[rev.Revision] = true
+ }
+ }
+ if len(candidates) == 0 {
+ return
+ }
+
+ // Step 3: Get alive instances and their revisions
+ instances := s.serviceDiscovery.GetInstances(app)
+ a
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
sonarqubecloud[bot] commented on PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371#issuecomment-4638732463 ## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) **Quality Gate passed** Issues  [4 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3371&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3371&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3371&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3371&metric=new_coverage&view=list)  [14.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3371&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) -- 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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
eye-gu commented on code in PR #3371:
URL: https://github.com/apache/dubbo-go/pull/3371#discussion_r3367378479
##
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:
copilot要求for循环分页查询,所以该日志已经删除了
--
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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
eye-gu commented on code in PR #3371:
URL: https://github.com/apache/dubbo-go/pull/3371#discussion_r3367376069
##
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:
已添加复制快照
--
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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
eye-gu commented on code in PR #3371:
URL: https://github.com/apache/dubbo-go/pull/3371#discussion_r3367374823
##
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:
已通过metadata report URL获取
--
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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
sonarqubecloud[bot] commented on PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371#issuecomment-4638686173 ## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) **Quality Gate passed** Issues  [4 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3371&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3371&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3371&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3371&metric=new_coverage&view=list)  [14.9% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3371&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) -- 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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
AlexStocks commented on code in PR #3371:
URL: https://github.com/apache/dubbo-go/pull/3371#discussion_r3367357889
##
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()
+ })
+}
+
+func (s *serviceDiscoveryRegistry) doRenewAppMetadata() {
+ registryID := s.url.GetParam(constant.RegistryIdKey, "")
+ metaInfo := metadata.GetMetadataInfo(registryID)
+ if metaInfo == nil || metaInfo.Revision == "0" {
+ return
+ }
+ metaInfo.LastUpdatedTime = time.Now().UnixMilli()
+ if err := s.metadataReport.PublishAppMetadata(metaInfo.App,
metaInfo.Revision, metaInfo); err != nil {
+ logger.Errorf("[Metadata][renewAppMetadata] failed to
re-publish metadata for app=%s revision=%s: %v", metaInfo.App,
metaInfo.Revision, err)
+ } else {
+ logger.Infof("[Metadata][renewAppMetadata] refreshed metadata
for app=%s revision=%s", metaInfo.App, metaInfo.Revision)
+ }
+
+ // Run garbage collection if enabled, after each renew cycle
+ if s.url.GetParamBool(constant.MetadataGCEnabledKey, true) {
+ s.doGarbageCollect()
+ }
+}
+
+func (s *serviceDiscoveryRegistry) calculateRenewAppMetadataDelay()
time.Duration {
+ now := time.Now()
+ // Next day 2:00 AM
+ nextDay2AM := time.Date(now.Year(), now.Month(), now.Day()+1, 2, 0, 0,
0, now.Location())
+ // Add random offset 0~4 hours to avoid thundering herd
+ randomOffset := time.Duration(rand.Int64N(int64(4 * time.Hour)))
+ return time.Until(nextDay2AM) + randomOffset
+}
+
+// == GC: stale revision cleanup ==
+
+func (s *serviceDiscoveryRegistry) doGarbageCollect() {
+ registryID := s.url.GetParam(constant.RegistryIdKey, "")
+ metaInfo := metadata.GetMetadataInfo(registryID)
+ if metaInfo == nil {
+ return
+ }
+ app := metaInfo.App
+ if app == "" {
+ return
+ }
+
+ // Step 1: List all revisions for this app
+ revisions, err := s.metadataReport.ListAppRevisions(app)
+ if err != nil {
+ logger.Warnf("[Metadata][GC] failed to list app revisions: %v",
err)
+ return
+ }
+ if len(revisions) == 0 {
+ return
+ }
+
+ // Step 2: Filter stale candidates (exceed GC window in days)
+ gcWindowDays := s.url.GetParamByIntValue(constant.MetadataGCWindowKey,
5)
+ if gcWindowDays <= 0 || gcWindowDays > 365 {
+ gcWindowDays = 5
+ }
+ cutoff := time.Now().AddDate(0, 0, -gcWindowDays).UnixMilli()
+ candidates := make(map[string]bool)
+ for _, rev := range revisions {
+ // Skip special revisions
+ if rev.Revision == "0" || rev.Revision == "N/A" || rev.Revision
== "" || rev.Revision == metaInfo.Revision {
+ continue
+ }
+ // ModifyTime == 0 means old metadata produced by a version
that does not set
+ // lastUpdatedTime — never garbage-collect such entries. Only
delete when the
+ // revision is older than gcWindow and no alive instance
references it.
+ if rev.ModifyTime > 0 && rev.ModifyTime < cutoff {
+ candidates[rev.Revision] = true
+ }
+ }
+ if len(candidates) == 0 {
+ return
+ }
+
+ // Step 3: Get alive instances and their revisions
+ instances := s.serviceDiscovery.GetInstances(app)
+
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
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")
+
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
Copilot commented on code in PR #3371:
URL: https://github.com/apache/dubbo-go/pull/3371#discussion_r3367021937
##
metadata/report/zookeeper/report.go:
##
@@ -82,13 +108,48 @@ func (m *zookeeperMetadataReport)
PublishAppMetadata(application, revision strin
return err
}
err = m.client.CreateWithValue(k, data)
- if perrors.Is(err, zk.ErrNodeExists) {
- logger.Debug("[Metadata][Zookeeper] try to create the node data
failed. In most cases, it's not a problem. ")
+ if err == zk.ErrNodeExists {
+ _, err = m.client.SetContent(k, data, -1)
+ }
Review Comment:
`CreateWithValue` may return a wrapped `zk.ErrNodeExists` (other codepaths
in this repo handle it via `perrors.Is`). Using direct equality here risks
skipping the update path and leaving existing app-metadata nodes stale when
republishing.
Consider switching to `errors.Is(err, zk.ErrNodeExists)` (or `perrors.Is`),
and similarly for `zk.ErrNoNode` checks if the client wraps errors.
##
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,
+ })
Review Comment:
`ListAppRevisions` only fetches the first page (500) and then proceeds with
a partial set of revisions. This can make GC incomplete for long-lived apps
(revisions beyond the first page will never be considered for cleanup). The
warning log mentions silent drops, but GC correctness still depends on seeing
all revisions.
##
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()
+ })
+}
+
+func (s *serviceDiscoveryRegistry) doRenewAppMetadata() {
+ registryID := s.url.GetParam(constant.RegistryIdKey, "")
+ metaInfo := metadata.GetMetadataInfo(registryID)
+ if metaInfo == nil || metaInfo.Revision == "0" {
+ return
+ }
+ metaInfo.LastUpdatedTime = time.
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
sonarqubecloud[bot] commented on PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371#issuecomment-4630643126 ## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) **Quality Gate passed** Issues  [3 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3371&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3371&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3371&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3371&metric=new_coverage&view=list)  [14.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3371&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) -- 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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
github-advanced-security[bot] commented on code in PR #3371:
URL: https://github.com/apache/dubbo-go/pull/3371#discussion_r3361963178
##
registry/servicediscovery/service_discovery_registry.go:
##
@@ -298,12 +320,143 @@
}
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()
+ })
+}
+
+func (s *serviceDiscoveryRegistry) doRenewAppMetadata() {
+ registryID := s.url.GetParam(constant.RegistryIdKey, "")
+ metaInfo := metadata.GetMetadataInfo(registryID)
+ if metaInfo == nil || metaInfo.Revision == "0" {
+ return
+ }
+ metaInfo.LastUpdatedTime = time.Now().UnixMilli()
+ if err := s.metadataReport.PublishAppMetadata(metaInfo.App,
metaInfo.Revision, metaInfo); err != nil {
+ logger.Errorf("[Metadata][renewAppMetadata] failed to
re-publish metadata for app=%s revision=%s: %v", metaInfo.App,
metaInfo.Revision, err)
+ } else {
+ logger.Infof("[Metadata][renewAppMetadata] refreshed metadata
for app=%s revision=%s", metaInfo.App, metaInfo.Revision)
+ }
+
+ // Run garbage collection if enabled, after each renew cycle
+ if s.url.GetParamBool(constant.MetadataGCEnabledKey, true) {
+ s.doGarbageCollect()
+ }
+}
+
+func (s *serviceDiscoveryRegistry) calculateRenewAppMetadataDelay()
time.Duration {
+ now := time.Now()
+ // Next day 2:00 AM
+ nextDay2AM := time.Date(now.Year(), now.Month(), now.Day()+1, 2, 0, 0,
0, now.Location())
+ // Add random offset 0~4 hours to avoid thundering herd
+ randomOffset := time.Duration(rand.Int64N(int64(4 * time.Hour)))
+ return time.Until(nextDay2AM) + randomOffset
+}
+
+// == GC: stale revision cleanup ==
+
+func (s *serviceDiscoveryRegistry) doGarbageCollect() {
+ registryID := s.url.GetParam(constant.RegistryIdKey, "")
+ metaInfo := metadata.GetMetadataInfo(registryID)
+ if metaInfo == nil {
+ return
+ }
+ app := metaInfo.App
+ if app == "" {
+ return
+ }
+
+ // Step 1: List all revisions for this app
+ revisions, err := s.metadataReport.ListAppRevisions(app)
+ if err != nil {
+ logger.Warnf("[Metadata][GC] failed to list app revisions: %v",
err)
+ return
+ }
+ if len(revisions) == 0 {
+ return
+ }
+
+ // Step 2: Filter stale candidates (exceed GC window in days)
+ gcWindowRaw := s.url.GetParamInt(constant.MetadataGCWindowKey, 5)
+ if gcWindowRaw <= 0 || gcWindowRaw > 365 {
+ gcWindowRaw = 5
+ }
+ gcWindowDays := int(gcWindowRaw)
Review Comment:
## CodeQL / Incorrect conversion between integer types
Incorrect conversion of a signed 64-bit integer from [strconv.ParseInt](1)
to a lower bit size type int without an upper bound check.
[Show more
details](https://github.com/apache/dubbo-go/security/code-scanning/71)
--
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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
sonarqubecloud[bot] commented on PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371#issuecomment-4630287545 ## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) **Quality Gate passed** Issues  [3 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3371&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3371&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3371&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3371&metric=new_coverage&view=list)  [14.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3371&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) -- 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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
codecov-commenter commented on PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371#issuecomment-4629401650 ## [Codecov](https://app.codecov.io/gh/apache/dubbo-go/pull/3371?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report :x: Patch coverage is `67.01031%` with `64 lines` in your changes missing coverage. Please review. :white_check_mark: Project coverage is 52.79%. Comparing base ([`60d1c2a`](https://app.codecov.io/gh/apache/dubbo-go/commit/60d1c2a949f0ee0da4be3fe09fb79d295491e040?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`52b64b0`](https://app.codecov.io/gh/apache/dubbo-go/commit/52b64b08c4a6f88d968512e719ce0508314c0e2a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)). :warning: Report is 816 commits behind head on develop. | [Files with missing lines](https://app.codecov.io/gh/apache/dubbo-go/pull/3371?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines | |---|---|---| | [...try/servicediscovery/service\_discovery\_registry.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3371?src=pr&el=tree&filepath=registry%2Fservicediscovery%2Fservice_discovery_registry.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cmVnaXN0cnkvc2VydmljZWRpc2NvdmVyeS9zZXJ2aWNlX2Rpc2NvdmVyeV9yZWdpc3RyeS5nbw==) | 61.44% | [24 Missing and 8 partials :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3371?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | | [metadata/report/zookeeper/report.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3371?src=pr&el=tree&filepath=metadata%2Freport%2Fzookeeper%2Freport.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bWV0YWRhdGEvcmVwb3J0L3pvb2tlZXBlci9yZXBvcnQuZ28=) | 62.50% | [10 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3371?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | | [metadata/report/nacos/report.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3371?src=pr&el=tree&filepath=metadata%2Freport%2Fnacos%2Freport.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bWV0YWRhdGEvcmVwb3J0L25hY29zL3JlcG9ydC5nbw==) | 79.54% | [6 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3371?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | | [metadata/report/etcd/report.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3371?src=pr&el=tree&filepath=metadata%2Freport%2Fetcd%2Freport.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bWV0YWRhdGEvcmVwb3J0L2V0Y2QvcmVwb3J0Lmdv) | 76.92% | [5 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3371?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | | [metadata/report/report.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3371?src=pr&el=tree&filepath=metadata%2Freport%2Freport.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bWV0YWRhdGEvcmVwb3J0L3JlcG9ydC5nbw==) | 0.00% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3371?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop#3371 +/- ## === + Coverage46.76% 52.79% +6.02% === Files 295 493 +198 Lines1717238059 +20887 === + Hits 803120093 +12062 - Misses828716339+8052 - Partials 854 1627 +773 ``` [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/dubbo-go/pull/3371?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+c
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
github-advanced-security[bot] commented on code in PR #3371:
URL: https://github.com/apache/dubbo-go/pull/3371#discussion_r3361238099
##
registry/servicediscovery/service_discovery_registry.go:
##
@@ -298,12 +320,139 @@
}
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()
+ })
+}
+
+func (s *serviceDiscoveryRegistry) doRenewAppMetadata() {
+ registryID := s.url.GetParam(constant.RegistryIdKey, "")
+ metaInfo := metadata.GetMetadataInfo(registryID)
+ if metaInfo == nil || metaInfo.Revision == "0" {
+ return
+ }
+ metaInfo.LastUpdatedTime = time.Now().UnixMilli()
+ if err := s.metadataReport.PublishAppMetadata(metaInfo.App,
metaInfo.Revision, metaInfo); err != nil {
+ logger.Errorf("[Metadata][renewAppMetadata] failed to
re-publish metadata for app=%s revision=%s: %v", metaInfo.App,
metaInfo.Revision, err)
+ } else {
+ logger.Infof("[Metadata][renewAppMetadata] refreshed metadata
for app=%s revision=%s", metaInfo.App, metaInfo.Revision)
+ }
+
+ // Run garbage collection if enabled, after each renew cycle
+ if s.url.GetParamBool(constant.MetadataGCEnabledKey, true) {
+ s.doGarbageCollect()
+ }
+}
+
+func (s *serviceDiscoveryRegistry) calculateRenewAppMetadataDelay()
time.Duration {
+ now := time.Now()
+ // Next day 2:00 AM
+ nextDay2AM := time.Date(now.Year(), now.Month(), now.Day()+1, 2, 0, 0,
0, now.Location())
+ // Add random offset 0~4 hours to avoid thundering herd
+ randomOffset := time.Duration(rand.Int63n(int64(4 * time.Hour)))
+ return time.Until(nextDay2AM) + randomOffset
+}
+
+// == GC: stale revision cleanup ==
+
+func (s *serviceDiscoveryRegistry) doGarbageCollect() {
+ registryID := s.url.GetParam(constant.RegistryIdKey, "")
+ metaInfo := metadata.GetMetadataInfo(registryID)
+ if metaInfo == nil {
+ return
+ }
+ app := metaInfo.App
+ if app == "" {
+ return
+ }
+
+ // Step 1: List all revisions for this app
+ revisions, err := s.metadataReport.ListAppRevisions(app)
+ if err != nil {
+ logger.Warnf("[Metadata][GC] failed to list app revisions: %v",
err)
+ return
+ }
+ if len(revisions) == 0 {
+ return
+ }
+
+ // Step 2: Filter stale candidates (exceed GC window in days)
+ gcWindowDays := int(s.url.GetParamInt(constant.MetadataGCWindowKey, 5))
Review Comment:
## CodeQL / Incorrect conversion between integer types
Incorrect conversion of a signed 64-bit integer from [strconv.ParseInt](1)
to a lower bit size type int without an upper bound check.
[Show more
details](https://github.com/apache/dubbo-go/security/code-scanning/70)
--
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]
Re: [PR] feat(metadata): add renew and GC policy for app-level metadata [dubbo-go]
sonarqubecloud[bot] commented on PR #3371: URL: https://github.com/apache/dubbo-go/pull/3371#issuecomment-4629361496 ## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) **Quality Gate failed** Failed conditions  [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3371&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3371) -- 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]
