NeverENG commented on code in PR #3635:
URL: https://github.com/apache/dubbo-go/pull/3635#discussion_r3836260930
##########
remoting/nacos/builder.go:
##########
@@ -43,6 +44,55 @@ var (
newNacosConfigClient = nacosClient.NewNacosConfigClient
)
+// credentialIDs maps each distinct credential set to a small opaque id used
+// in pool keys. The credentials themselves stay in this process-local map and
+// never become part of the key, which may end up in logs.
+var (
+ credentialIDsMu sync.Mutex
+ credentialIDs = make(map[string]string)
+)
+
+func credentialID(url *common.URL) string {
+ tuple := strings.Join([]string{
+ url.GetParam(constant.NacosUsername, ""),
+ url.GetParam(constant.NacosPassword, ""),
+ url.GetParam(constant.NacosAccessKey, ""),
+ url.GetParam(constant.NacosSecretKey, ""),
+ }, "\n")
+ credentialIDsMu.Lock()
+ defer credentialIDsMu.Unlock()
+ id, ok := credentialIDs[tuple]
+ if !ok {
+ id = "cred" + strconv.Itoa(len(credentialIDs))
+ credentialIDs[tuple] = id
+ }
+ return id
+}
+
+// nacosClientPoolKey derives the gost client-pool key from the fields that
+// distinguish one nacos connection from another: server (endpoint/address),
+// namespace and the full credential set. Components pointing at the same
+// cluster (registry, config-center, metadata-report) resolve to the same key
+// and share one SDK client session instead of each opening its own.
+// Role-scoped client names must not be used as the key — they would defeat
+// the sharing.
+func nacosClientPoolKey(kind string, url *common.URL) string {
+ // GetNacosConfig ignores url.Location when an endpoint is set; mirror
+ // that here so URLs resolving to the same server set share one client.
+ server := url.GetParam(constant.NacosEndpoint, "")
Review Comment:
这里 poolkey 没有区分 path，主体是 url.location 。如果相同的 location 可能打到同一个对象 比如
nacos://localhost:8080/nacos 和 nacos://localhost:8080/custom会打到同一个client对象，建议加入
url.Path 作为 key 的一员,并补充对应单元测试
##########
metadata/report/zookeeper/report.go:
##########
@@ -213,11 +213,19 @@ type zookeeperMetadataReportFactory struct{}
// CreateMetadataReport creates the zookeeper-based metadata report
implementation.
func (mf *zookeeperMetadataReportFactory) CreateMetadataReport(url
*common.URL) report.MetadataReport {
+ // Join the gost shared-client pool under the same key (url.Location)
that
+ // registry and config-center use via ValidateZookeeperClient, so all
roles
+ // pointing at the same cluster reuse one ZooKeeper session. The pool is
+ // reference-counted: Close only disconnects when the last user is gone.
+ // Prefer the timeout key the other pool users read (ConfigTimeoutKey)
so
+ // the pooled client's timeout does not depend on which role creates it
+ // first; fall back to the metadata-report's historical TimeoutKey.
+ timeout := url.GetParamDuration(constant.ConfigTimeoutKey,
url.GetParam(constant.TimeoutKey, "15s"))
Review Comment:
registry 和 config-center 创建共享 ZK client 时， DefaultRegTimeout =
"5s"，这里默认值是15s。
gost 创建 zookeeper client 是 frist-win
，如果默认超时不一致语义不清晰，建议修改。建议统一成url.GetParamDuration(constant.ConfigTimeoutKey,
constant.DefaultRegTimeout)吗，与另外两个角色完全一致。
##########
remoting/nacos/builder.go:
##########
@@ -43,6 +44,55 @@ var (
newNacosConfigClient = nacosClient.NewNacosConfigClient
)
+// credentialIDs maps each distinct credential set to a small opaque id used
+// in pool keys. The credentials themselves stay in this process-local map and
+// never become part of the key, which may end up in logs.
+var (
+ credentialIDsMu sync.Mutex
+ credentialIDs = make(map[string]string)
+)
+
+func credentialID(url *common.URL) string {
+ tuple := strings.Join([]string{
+ url.GetParam(constant.NacosUsername, ""),
+ url.GetParam(constant.NacosPassword, ""),
+ url.GetParam(constant.NacosAccessKey, ""),
+ url.GetParam(constant.NacosSecretKey, ""),
+ }, "\n")
+ credentialIDsMu.Lock()
+ defer credentialIDsMu.Unlock()
+ id, ok := credentialIDs[tuple]
+ if !ok {
+ id = "cred" + strconv.Itoa(len(credentialIDs))
+ credentialIDs[tuple] = id
+ }
+ return id
+}
+
+// nacosClientPoolKey derives the gost client-pool key from the fields that
+// distinguish one nacos connection from another: server (endpoint/address),
+// namespace and the full credential set. Components pointing at the same
+// cluster (registry, config-center, metadata-report) resolve to the same key
+// and share one SDK client session instead of each opening its own.
+// Role-scoped client names must not be used as the key — they would defeat
+// the sharing.
+func nacosClientPoolKey(kind string, url *common.URL) string {
+ // GetNacosConfig ignores url.Location when an endpoint is set; mirror
+ // that here so URLs resolving to the same server set share one client.
+ server := url.GetParam(constant.NacosEndpoint, "")
Review Comment:
https://github.com/apache/dubbo-go/issues/3330
有点像这个
--
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]
