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 b6dc2352d fix: subscribe provided-by applications initially (#3623)
b6dc2352d is described below
commit b6dc2352d3b98fe4ed38932ed9b95f29f472a3a1
Author: Lcos <[email protected]>
AuthorDate: Tue Aug 11 11:44:38 2026 +0800
fix: subscribe provided-by applications initially (#3623)
* fix: subscribe provided-by applications initially
Subscribe the initial application set directly instead of routing it
through the mapping change listener. This prevents provided-by from being
treated as an unchanged mapping and skipping instance discovery.\n\nAdd a
regression test covering the initial instance lookup and listener
installation.\n\nFixes: #3617
Signed-off-by: Lcos <[email protected]>
* fix: narrow provided-by fix, keep metadata-report OnEvent baseline
The initial fix routed both provided-by and metadata-report mapping through
a direct SubscribeURL, which dropped the mapping listener baseline update on
the metadata-report path. Replaying an identical mapping set then went through
the 'old is empty' branch and called ServiceDiscovery.AddListener a second time
(re-registered the listener).
Narrow the fix: only the provided-by path subscribes directly; the
metadata-report path keeps the initial subscription on OnEvent so
oldServiceNames is updated and later A->A is a no-op.
Add AddListener call counting to mockServiceDiscovery and two regression
tests:
- TestServiceDiscoveryRegistrySubscribeWithProvidedBy asserts AddListener
fires exactly once for provided-by.
- TestServiceDiscoveryRegistrySubscribeMetadataReportA2A asserts that
replaying identical mapping to the installed mapping listener does not trigger
another AddListener.
Addresses P1 review feedback on #3623.
Signed-off-by: user.email <[email protected]>
* test: use require for error assertion to satisfy testifylint
Signed-off-by: user.email <[email protected]>
---------
Signed-off-by: Lcos <[email protected]>
Signed-off-by: user.email <[email protected]>
---
.../servicediscovery/service_discovery_registry.go | 17 +++-
.../service_discovery_registry_test.go | 102 +++++++++++++++++++++
2 files changed, 115 insertions(+), 4 deletions(-)
diff --git a/registry/servicediscovery/service_discovery_registry.go
b/registry/servicediscovery/service_discovery_registry.go
index 52c0c6850..0e4c7d9af 100644
--- a/registry/servicediscovery/service_discovery_registry.go
+++ b/registry/servicediscovery/service_discovery_registry.go
@@ -514,10 +514,19 @@ func (s *serviceDiscoveryRegistry) Subscribe(url
*common.URL, notify registry.No
" either specify 'provided-by' for reference or enable
metadata-report center subscription url:%s", url.String())
} else {
logger.Infof("[Registry][ServiceDiscovery] find initial mapping
applications %q for service %s", services, url.ServiceKey())
- // first notify
- err :=
mappingListener.OnEvent(registry.NewServiceMappingChangedEvent(url.ServiceKey(),
services))
- if err != nil {
- logger.Errorf("[Registry][ServiceDiscovery]
ServiceInstancesChangedListenerImpl handle error, err=%v", err)
+ if _, ok := url.GetNonDefaultParam(constant.ProvidedBy); ok {
+ // provided-by is an explicit, unchanging initial
target set, so it is
+ // subscribed directly. Routing it through the mapping
change listener
+ // treats it as an unchanged mapping and skips
SubscribeURL entirely.
+ s.SubscribeURL(url, notify, services)
+ } else {
+ // metadata-report mapping is dynamic: keep the initial
subscription on
+ // OnEvent so the listener baseline (oldServiceNames)
is updated and later
+ // mapping updates diff against it instead of
re-subscribing.
+ err :=
mappingListener.OnEvent(registry.NewServiceMappingChangedEvent(url.ServiceKey(),
services))
+ if err != nil {
+ logger.Errorf("[Registry][ServiceDiscovery]
ServiceInstancesChangedListenerImpl handle error, err=%v", err)
+ }
}
}
return nil
diff --git a/registry/servicediscovery/service_discovery_registry_test.go
b/registry/servicediscovery/service_discovery_registry_test.go
index 15e4d8b04..b4965f5fa 100644
--- a/registry/servicediscovery/service_discovery_registry_test.go
+++ b/registry/servicediscovery/service_discovery_registry_test.go
@@ -125,6 +125,91 @@ func TestServiceDiscoveryRegistrySubscribe(t *testing.T) {
assert.True(t, mockSD.listenerAdded)
}
+// TestServiceDiscoveryRegistrySubscribeWithProvidedBy verifies that the
initial
+// provided-by application is subscribed and its instances listener is
installed
+// exactly once (issue #3617).
+func TestServiceDiscoveryRegistrySubscribeWithProvidedBy(t *testing.T) {
+ mockSD, _ := setupEnvironment(t)
+
+ registryURL, _ := common.NewURL(testRegistryURL,
+ common.WithParamsValue(constant.RegistryKey, "mock"))
+
+ reg, err := newServiceDiscoveryRegistry(registryURL)
+ require.NoError(t, err)
+
+ consumerURL, _ := common.NewURL("dubbo://127.0.0.1:20000/",
+ common.WithInterface(testInterface),
+ common.WithParamsValue(constant.SideKey, constant.SideConsumer),
+ common.WithParamsValue(constant.ProvidedBy, testApp),
+ )
+
+ mockSD.wg.Add(1)
+ err = reg.Subscribe(consumerURL, &mockNotifyListener{})
+ require.NoError(t, err)
+
+ assert.Equal(t, testApp, mockSD.capturedAppName)
+
+ done := make(chan struct{})
+ go func() {
+ mockSD.wg.Wait()
+ close(done)
+ }()
+ select {
+ case <-done:
+ case <-time.After(3 * time.Second):
+ t.Fatal("AddListener was not invoked")
+ }
+ assert.True(t, mockSD.listenerAdded)
+ assert.Equal(t, 1, mockSD.getListenerAddCount(), "AddListener must be
invoked exactly once for provided-by")
+}
+
+// TestServiceDiscoveryRegistrySubscribeMetadataReportA2A verifies that on the
+// metadata-report path, replaying an identical mapping event to the installed
+// mapping listener does not trigger a second ServiceDiscovery.AddListener
call.
+// This is the regression that the narrowed fix guards: the initial OnEvent
+// establishes the listener baseline so the same mapping is a no-op (P1 #3623).
+func TestServiceDiscoveryRegistrySubscribeMetadataReportA2A(t *testing.T) {
+ mockSD, mockMapping := setupEnvironment(t)
+ mockMapping.data[testInterface] = gxset.NewSet(testApp)
+
+ registryURL, _ := common.NewURL(testRegistryURL,
+ common.WithParamsValue(constant.RegistryKey, "mock"))
+
+ reg, err := newServiceDiscoveryRegistry(registryURL)
+ require.NoError(t, err)
+ sdReg, ok := reg.(*serviceDiscoveryRegistry)
+ require.True(t, ok)
+
+ consumerURL, _ := common.NewURL("dubbo://127.0.0.1:20000/",
+ common.WithInterface(testInterface),
+ common.WithParamsValue(constant.GroupKey, testGroup),
+ common.WithParamsValue(constant.SideKey, constant.SideConsumer),
+ )
+
+ mockSD.wg.Add(1)
+ err = reg.Subscribe(consumerURL, &mockNotifyListener{})
+ require.NoError(t, err)
+
+ done := make(chan struct{})
+ go func() { mockSD.wg.Wait(); close(done) }()
+ select {
+ case <-done:
+ case <-time.After(3 * time.Second):
+ t.Fatal("initial AddListener was not invoked")
+ }
+ require.Equal(t, 1, mockSD.getListenerAddCount(), "AddListener must
fire once for the initial mapping")
+
+ protocolServiceKey := consumerURL.ServiceKey() + ":" +
consumerURL.Protocol
+ sdReg.lock.Lock()
+ mappingListener := sdReg.serviceMappingListeners[protocolServiceKey]
+ sdReg.lock.Unlock()
+ require.NotNil(t, mappingListener, "mapping listener must be registered
on the metadata-report path")
+
+ err =
mappingListener.OnEvent(registry.NewServiceMappingChangedEvent(consumerURL.ServiceKey(),
gxset.NewSet(testApp)))
+ require.NoError(t, err)
+ assert.Equal(t, 1, mockSD.getListenerAddCount(), "an identical mapping
replay must not trigger another AddListener")
+}
+
// TestServiceDiscoveryRegistryUnSubscribe verifies the unsubscription logic.
func TestServiceDiscoveryRegistryUnSubscribe(t *testing.T) {
mockSD, mockMapping := setupEnvironment(t)
@@ -380,6 +465,12 @@ type mockServiceDiscovery struct {
capturedAppName string
capturedInstance registry.ServiceInstance
+ // AddListener invocation tracking. Counts every AddListener call so
tests
+ // can assert that the underlying ServiceDiscovery is not re-registered
+ // (see PR #3623 review feedback on metadata-report path baseline).
+ listenerMu sync.Mutex
+ listenerAddCount int
+
// for Unregister tests
unregisterCalled bool
unregisterIDs []string
@@ -431,10 +522,21 @@ func (m *mockServiceDiscovery)
GetRequestInstances([]string, int, int) map[strin
func (m *mockServiceDiscovery)
AddListener(registry.ServiceInstancesChangedListener) error {
defer m.wg.Done()
+ m.listenerMu.Lock()
m.listenerAdded = true
+ m.listenerAddCount++
+ m.listenerMu.Unlock()
return nil
}
+// getListenerAddCount returns the number of AddListener calls observed by the
+// mock under the listener lock.
+func (m *mockServiceDiscovery) getListenerAddCount() int {
+ m.listenerMu.Lock()
+ defer m.listenerMu.Unlock()
+ return m.listenerAddCount
+}
+
type mockServiceNameMapping struct {
data map[string]*gxset.HashSet
mapCalled bool
