This is an automated email from the ASF dual-hosted git repository.
Alanxtl pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/dubbo-go.git
The following commit(s) were added to refs/heads/develop by this push:
new c150ca471 perf(remoting): share one backend client per ZK/Nacos
cluster across registry, config-center and metadata-report (#3573) (#3635)
c150ca471 is described below
commit c150ca471ffe4003f76079e39bd2fdc55db1e37b
Author: 超級の新人 <[email protected]>
AuthorDate: Sun Aug 23 21:47:53 2026 +0800
perf(remoting): share one backend client per ZK/Nacos cluster across
registry, config-center and metadata-report (#3573) (#3635)
The gost client pool is keyed by name, but each role passed a different
key, so a single application held three sessions to the same cluster:
- zookeeper: metadata-report used a fixed name with share=false while
registry/config-center already share via url.Location; align it to
the same pool key and enable sharing (3 sessions -> 1).
- nacos: all roles set share=true but role-prefixed client names never
matched; derive the pool key from the connection identity (endpoint,
address, namespace, credentials) instead (3 clients -> 2, naming and
config clients are distinct SDK types).
etcd is left as a follow-up: gost has no pool for it and callers hold
the concrete client type.
Signed-off-by: chaojixinren <[email protected]>
---
metadata/report/zookeeper/report.go | 15 +++-
remoting/nacos/builder.go | 80 ++++++++++++++++++--
remoting/nacos/builder_test.go | 145 ++++++++++++++++++++++++++++++++++++
3 files changed, 231 insertions(+), 9 deletions(-)
diff --git a/metadata/report/zookeeper/report.go
b/metadata/report/zookeeper/report.go
index f2d02b8ed..0b35bfa7d 100644
--- a/metadata/report/zookeeper/report.go
+++ b/metadata/report/zookeeper/report.go
@@ -239,11 +239,20 @@ 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 with
+ // DefaultRegTimeout as the ultimate default (consistent with registry
and config-center).
+ timeout := url.GetParamDuration(constant.ConfigTimeoutKey,
url.GetParam(constant.TimeoutKey, constant.DefaultRegTimeout))
client, err := gxzookeeper.NewZookeeperClient(
- "zookeeperMetadataReport",
+ url.Location,
strings.Split(url.Location, ","),
- false,
-
gxzookeeper.WithZkTimeOut(url.GetParamDuration(constant.TimeoutKey, "15s")),
+ true,
+ gxzookeeper.WithZkTimeOut(timeout),
)
if err != nil {
panic(err)
diff --git a/remoting/nacos/builder.go b/remoting/nacos/builder.go
index 52082d634..7fb597682 100644
--- a/remoting/nacos/builder.go
+++ b/remoting/nacos/builder.go
@@ -21,6 +21,7 @@ import (
"net"
"strconv"
"strings"
+ "sync"
"time"
)
@@ -43,6 +44,77 @@ 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.RWMutex
+ 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")
+
+ // Try read lock first for the common case (credential already exists)
+ credentialIDsMu.RLock()
+ id, ok := credentialIDs[tuple]
+ credentialIDsMu.RUnlock()
+ if ok {
+ return id
+ }
+
+ // Need to create new credential ID, acquire write lock
+ credentialIDsMu.Lock()
+ defer credentialIDsMu.Unlock()
+ // Double-check in case another goroutine created it while we waited
+ 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),
+// path, 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.
+//
+// Note on url.Location with multiple addresses: url.Location may contain
+// multiple comma-separated addresses (e.g., "host1:8848,host2:8848") which
+// are parsed into separate ServerConfig entries. The full Location string
+// participates in the pool key, so different orderings or different server
+// lists will create separate pool keys. This is intentional: the order and
+// composition of servers affects client behavior, and configurations should
+// be consistent across components that intend to share a client.
+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, "")
+ if server == "" {
+ server = url.Location
+ }
+ // Clients authenticated differently must never collapse into one pool
+ // entry, so the full credential set participates via its opaque id.
+ // Include url.Path so that nacos://host:port/pathA and
nacos://host:port/pathB
+ // create separate clients (path becomes ContextPath in ServerConfig).
+ return strings.Join([]string{
+ "dubbo-nacos", kind,
+ server,
+ url.Path,
+ url.GetParam(constant.NacosNamespaceID, ""),
+ credentialID(url),
+ }, "|")
+}
+
// NewNacosConfigClientByUrl read the config from url and build an instance
func NewNacosConfigClientByUrl(url *common.URL)
(*nacosClient.NacosConfigClient, error) {
sc, cc, err := GetNacosConfig(url)
@@ -53,7 +125,7 @@ func NewNacosConfigClientByUrl(url *common.URL)
(*nacosClient.NacosConfigClient,
if len(clientName) <= 0 {
return nil, perrors.New("nacos client name must set")
}
- return newNacosConfigClient(clientName, true, sc, cc)
+ return newNacosConfigClient(nacosClientPoolKey("config", url), true,
sc, cc)
}
// GetNacosConfig will return the nacos config
@@ -130,9 +202,5 @@ func NewNacosClientByURL(url *common.URL)
(*nacosClient.NacosNamingClient, error
return nil, perrors.New("nacos client name must set")
}
logger.Infof("[Remoting][Nacos] new nacos client, config=%+v", scs)
- namespaceID := url.GetParam(constant.NacosNamespaceID, "")
- if len(namespaceID) > 0 {
- clientName += namespaceID
- }
- return newNacosNamingClient(clientName, true, scs, cc)
+ return newNacosNamingClient(nacosClientPoolKey("naming", url), true,
scs, cc)
}
diff --git a/remoting/nacos/builder_test.go b/remoting/nacos/builder_test.go
index 03702401f..1e8d213c1 100644
--- a/remoting/nacos/builder_test.go
+++ b/remoting/nacos/builder_test.go
@@ -29,6 +29,7 @@ import (
nacosConstant "github.com/nacos-group/nacos-sdk-go/v2/common/constant"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
)
import (
@@ -247,3 +248,147 @@ func TestNewNacosConfigClientByUrlMissingClientName(t
*testing.T) {
assert.Nil(t, client)
assert.Error(t, err)
}
+
+func TestNacosClientPoolKeySharedAcrossRoles(t *testing.T) {
+ var captured []string
+ oldNewNacosConfigClient := newNacosConfigClient
+ newNacosConfigClient = func(name string, share bool, sc
[]nacosConstant.ServerConfig,
+ cc nacosConstant.ClientConfig) (*nacosClient.NacosConfigClient,
error) {
+ captured = append(captured, name)
+ assert.True(t, share)
+ return &nacosClient.NacosConfigClient{}, nil
+ }
+ t.Cleanup(func() {
+ newNacosConfigClient = oldNewNacosConfigClient
+ })
+
+ mkURL := func(clientName, namespace string) *common.URL {
+ m := url.Values{}
+ m.Set(constant.NacosNamespaceID, namespace)
+ m.Set(constant.ClientNameKey, clientName)
+ u, _ := common.NewURL("registry://test.nacos.io:80",
common.WithParams(m))
+ return u
+ }
+
+ // config-center and metadata-report carry role-scoped client names, but
+ // point at the same cluster: they must resolve to one pool key (#3573).
+ _, err :=
NewNacosConfigClientByUrl(mkURL("dubbo.config-center-nacos-test.nacos.io:80",
"ns1"))
+ require.NoError(t, err)
+ _, err =
NewNacosConfigClientByUrl(mkURL("dubbo.metadata-report-nacos-test.nacos.io:80",
"ns1"))
+ require.NoError(t, err)
+ assert.Equal(t, captured[0], captured[1])
+
+ // A different namespace is a different connection identity and must not
+ // share the client.
+ _, err =
NewNacosConfigClientByUrl(mkURL("dubbo.config-center-nacos-test.nacos.io:80",
"ns2"))
+ require.NoError(t, err)
+ assert.NotEqual(t, captured[0], captured[2])
+}
+
+func TestNacosClientPoolKeyCredentialAndEndpointIdentity(t *testing.T) {
+ // Clean up global credential IDs to avoid race conditions with other
tests
+ credentialIDsMu.Lock()
+ oldCredentialIDs := credentialIDs
+ credentialIDs = make(map[string]string)
+ credentialIDsMu.Unlock()
+ t.Cleanup(func() {
+ credentialIDsMu.Lock()
+ credentialIDs = oldCredentialIDs
+ credentialIDsMu.Unlock()
+ })
+
+ key := func(params map[string]string) string {
+ m := url.Values{}
+ for k, v := range params {
+ m.Set(k, v)
+ }
+ u, _ := common.NewURL("registry://test.nacos.io:80",
common.WithParams(m))
+ return nacosClientPoolKey("config", u)
+ }
+
+ userPass := map[string]string{constant.NacosUsername: "alice",
constant.NacosPassword: "s3cret-A"}
+
+ // Same server and same full credential set share one key.
+ assert.Equal(t, key(userPass),
key(map[string]string{constant.NacosUsername: "alice", constant.NacosPassword:
"s3cret-A"}))
+
+ // A different password (or secretKey) is a differently authenticated
+ // client and must never collapse into the same pool entry.
+ assert.NotEqual(t, key(userPass),
key(map[string]string{constant.NacosUsername: "alice", constant.NacosPassword:
"s3cret-B"}))
+ assert.NotEqual(t,
+ key(map[string]string{constant.NacosAccessKey: "AKID",
constant.NacosSecretKey: "SK-1"}),
+ key(map[string]string{constant.NacosAccessKey: "AKID",
constant.NacosSecretKey: "SK-2"}))
+
+ // Credentials must not appear in the key verbatim (keys can be logged).
+ assert.NotContains(t, key(userPass), "alice")
+ assert.NotContains(t, key(userPass), "s3cret-A")
+
+ // When an endpoint is set, GetNacosConfig ignores url.Location; the
pool
+ // key mirrors that, so differing (ignored) locations still share.
+ withEndpoint := func(location string) string {
+ m := url.Values{}
+ m.Set(constant.NacosEndpoint, "acm.aliyun.com")
+ u, _ := common.NewURL("registry://"+location,
common.WithParams(m))
+ return nacosClientPoolKey("config", u)
+ }
+ assert.Equal(t, withEndpoint("a.nacos.io:80"),
withEndpoint("b.nacos.io:80"))
+}
+
+func TestNacosNamingAndConfigPoolKeysDistinct(t *testing.T) {
+ var namingKey, configKey string
+ oldNaming := newNacosNamingClient
+ oldConfig := newNacosConfigClient
+ newNacosNamingClient = func(name string, share bool, sc
[]nacosConstant.ServerConfig,
+ cc nacosConstant.ClientConfig) (*nacosClient.NacosNamingClient,
error) {
+ namingKey = name
+ return &nacosClient.NacosNamingClient{}, nil
+ }
+ newNacosConfigClient = func(name string, share bool, sc
[]nacosConstant.ServerConfig,
+ cc nacosConstant.ClientConfig) (*nacosClient.NacosConfigClient,
error) {
+ configKey = name
+ return &nacosClient.NacosConfigClient{}, nil
+ }
+ t.Cleanup(func() {
+ newNacosNamingClient = oldNaming
+ newNacosConfigClient = oldConfig
+ })
+
+ regURL := getRegURL()
+ _, err := NewNacosClientByURL(regURL)
+ require.NoError(t, err)
+ _, err = NewNacosConfigClientByUrl(regURL)
+ require.NoError(t, err)
+ assert.NotEqual(t, namingKey, configKey)
+}
+
+func TestNacosClientPoolKeyDistinguishesByPath(t *testing.T) {
+ // Clean up global credential IDs to avoid race conditions with other
tests
+ credentialIDsMu.Lock()
+ oldCredentialIDs := credentialIDs
+ credentialIDs = make(map[string]string)
+ credentialIDsMu.Unlock()
+ t.Cleanup(func() {
+ credentialIDsMu.Lock()
+ credentialIDs = oldCredentialIDs
+ credentialIDsMu.Unlock()
+ })
+
+ key := func(path string) string {
+ m := url.Values{}
+ m.Set(constant.ClientNameKey, "test-client")
+ m.Set(constant.NacosNamespaceID, "default")
+ u, _ := common.NewURL("registry://localhost:8848"+path,
common.WithParams(m))
+ return nacosClientPoolKey("config", u)
+ }
+
+ // URLs with different paths must generate different pool keys (#3635)
+ keyNacos := key("/nacos")
+ keyCustom := key("/custom")
+ keyEmpty := key("")
+
+ assert.NotEqual(t, keyNacos, keyCustom, "different paths should create
different pool keys")
+ assert.NotEqual(t, keyNacos, keyEmpty, "path /nacos should differ from
empty path")
+ assert.NotEqual(t, keyCustom, keyEmpty, "path /custom should differ
from empty path")
+
+ // Same path should generate same key
+ assert.Equal(t, keyNacos, key("/nacos"), "same path should create same
pool key")
+}
