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 b3adb3362 fix(registry): publish app metadata only once after
registering all instances (#3590)
b3adb3362 is described below
commit b3adb3362ae7a1f90db43f8575c1664203c1f134
Author: Li Zining <[email protected]>
AuthorDate: Fri Aug 14 20:55:51 2026 +0800
fix(registry): publish app metadata only once after registering all
instances (#3590)
* fix(registry): publish app metadata only once after registering all
instances
RegisterService previously published the full app metadata inside the
per-URL loop, writing an identical payload N times for N exported services.
Move the publish out of the loop so the metadata center is written exactly
once, reducing startup write amplification and metadata center pressure.
Fixes: #3570
Signed-off-by: lizining <[email protected]>
* test(registry): verify app metadata is published exactly once
Add TestServiceDiscoveryRegistryRegisterPublishesMetadataOnce which
registers two services and asserts that app metadata is published exactly
once. The test fails on the previous per-URL publish implementation.
Signed-off-by: lizining <[email protected]>
* test(registry): cover RegisterService publish error paths
Signed-off-by: lizining <[email protected]>
* test(registry): fix testifylint require-error in publish test
Signed-off-by: lizining <[email protected]>
* fix(registry): publish app metadata before registering service instances
RegisterService registered every instance first and only published the app
metadata afterwards. If the publish failed, the already-registered instances
stayed routable while the server only rolled back after a fully successful
startup, leaving orphan instances. It also entered the publish path with an
empty exported URL list (e.g. pure consumers), writing empty keys or
hitting a
nil report dereference, and started the renew timer while the revision was
still "0", spinning as a no-op.
Build all instances and compute the revision first, publish app metadata
exactly once, then register each instance. Return early when there are no
exported URLs, keep the metadata report nil check before any dereference,
and
skip starting the renew timer until a revision exists.
Fixes: #3570
Signed-off-by: lizining <[email protected]>
* test(registry): cover publish failure, retry and metadata edge cases
Add tests for the reworked RegisterService: a failed publish registers no
instances and leaves none behind, a retry then registers them exactly once,
pure consumers with remote metadata never publish or register, a nil
metadata
report returns an error instead of panicking, non-remote metadata mode
publishes zero times, and the renew timer is not started while the revision
is empty.
Signed-off-by: lizining <[email protected]>
* fix(registry): skip starting the renew timer for empty revisions
The renew timer should not start while the revision is empty, and a
revision is empty when nothing real has been published yet: it starts as
"", becomes "0" after exported URLs are cleared, and falls back to "N/A"
in the customizer. startMetadataTimers previously did not check the
revision at all and doRenewAppMetadata only skipped "0", so "" and "N/A"
could still start the timer or trigger a renew. isPublishableRevision now
treats all three as empty, so the timer stays off until a real revision
exists.
Signed-off-by: lizining <[email protected]>
* test(registry): verify the renew timer is not started for empty revision
markers
Turn TestServiceDiscoveryRegistry_StartMetadataTimersSkipsEmptyRevision into
a table-driven test covering "", "0" and "N/A". All three cases keep the
renew timer from starting.
Signed-off-by: lizining <[email protected]>
* test(server): return an error from ServeContext when metadata publish
fails
Add TestServeContextReturnsErrorWhenMetadataPublishFails. The registry mock
calls PublishAppMetadata while the metadata report is mocked to fail, so
ServeContext returns an error for the publish failure.
Signed-off-by: lizining <[email protected]>
* style(server): replace context.WithCancel with t.Context() in test
Signed-off-by: lizining <[email protected]>
* style(server): fix gost import order to satisfy imports-formatter
Signed-off-by: lizining <[email protected]>
* test(registry): disable the startup renew in the publish-once test
RegisterService starts a background renew goroutine when the metadata
report enables metadata.renew.on.startup. That goroutine also calls
PublishAppMetadata, so the count asserted by
TestServiceDiscoveryRegistryRegisterPublishesMetadataOnce can be raced
and is not reliable proof of exactly-once. Give the mock report a URL
that disables the startup renew and stop the timer at the end of the
test, leaving the counter updated only by the synchronous publish.
Signed-off-by: lizining <[email protected]>
* ci(registry): gate servicediscovery tests with the race detector
RegisterService starts a background metadata renew goroutine that can
race the assertions of tests counting app-metadata publishes, and plain
go test does not detect that locally. Add a test-race target that runs
go test -race on the servicediscovery package and wire it into the CI
workflow so any newly introduced race is caught before merge, and the
intentionally racy TestServiceDiscoveryRegistryUnRegister_Concurrent is
excluded via the -skip flag.
Signed-off-by: lizining <[email protected]>
* ci(race): run the race detector across the whole repo
The test-race target previously only ran on the servicediscovery package,
so races in failback, chain, apollo, dubbo, grpc, getty and server tests
went undetected. Extend it to 'go test -race ./...' and skip the tests
with known issues via a RACE_SKIP_TESTS variable so the whole-repo race
run can pass; each skipped test should be fixed and removed from the list
over time.
Signed-off-by: lizining <[email protected]>
---------
Signed-off-by: lizining <[email protected]>
---
.github/workflows/github-actions.yml | 3 +
Makefile | 17 +-
.../servicediscovery/service_discovery_registry.go | 39 ++-
.../service_discovery_registry_test.go | 337 ++++++++++++++++++++-
server/server_test.go | 146 +++++++++
5 files changed, 527 insertions(+), 15 deletions(-)
diff --git a/.github/workflows/github-actions.yml
b/.github/workflows/github-actions.yml
index 28ed006ed..8fec237c7 100644
--- a/.github/workflows/github-actions.yml
+++ b/.github/workflows/github-actions.yml
@@ -51,6 +51,9 @@ jobs:
- name: Run unit tests
run: make test
+ - name: Race Test
+ run: make test-race
+
- name: Codecov
uses: codecov/codecov-action@v7 #NOSONAR
with:
diff --git a/Makefile b/Makefile
index 5a49ce83d..ac04fa3c2 100644
--- a/Makefile
+++ b/Makefile
@@ -40,6 +40,21 @@ else
BIN_EXT :=
endif
+# Tests with known issues (data races or test design defects) tracked in the
+# race-detector issue. Skipped via -skip so the whole-repo race run can pass;
+# each test should be fixed and removed from this list over time.
+RACE_SKIP_TESTS := TestFailbackRetryFailed \
+TestFailbackOutOfLimit \
+TestRouteCacheGenerationRace \
+TestListener \
+TestDubboProtocol_Refer \
+TestGrpcHealthWatchEmitsClosingEvent \
+TestServiceDiscoveryRegistryUnRegister_Concurrent \
+TestCfgAPI_Export \
+TestCfgAPI_Call \
+TestTCPPackageHandle
+space := $(subst x, ,x)
+
GOLANGCI_LINT := $(TOOLS_BIN)/golangci-lint$(BIN_EXT)
IMPORTS_FORMATTER := $(TOOLS_BIN)/imports-formatter$(BIN_EXT)
MODERNIZE := $(TOOLS_BIN)/modernize$(BIN_EXT)
@@ -65,7 +80,7 @@ test: clean ## Run unit tests and write the root coverage
profile
cd $(CLI_DIR) && $(GO_RUN) test ./...
test-race: clean ## Run unit tests with the race detector
- $(GO_RUN) test ./... -race -coverprofile=$(CURDIR)/$(COVERAGE_FILE)
-covermode=atomic
+ $(GO_RUN) test ./... -race -skip '^($(subst
$(space),|,$(RACE_SKIP_TESTS)))$$' -coverprofile=$(CURDIR)/$(COVERAGE_FILE)
-covermode=atomic
cd $(CLI_DIR) && $(GO_RUN) test ./... -race
fmt: $(MODERNIZE_STAMP) $(IMPORTS_FORMATTER_STAMP) ## Format Go code and
modernize syntax
diff --git a/registry/servicediscovery/service_discovery_registry.go
b/registry/servicediscovery/service_discovery_registry.go
index 0e4c7d9af..cd65c1ecf 100644
--- a/registry/servicediscovery/service_discovery_registry.go
+++ b/registry/servicediscovery/service_discovery_registry.go
@@ -88,6 +88,10 @@ func newServiceDiscoveryRegistry(url *common.URL)
(registry.Registry, error) {
}, nil
}
+func isPublishableRevision(revision string) bool {
+ return len(revision) > 0 && revision != "0" && revision != "N/A"
+}
+
// startMetadataTimers starts the renewAppMetadata timer if metadata type is
remote.
// GC runs after each renew cycle inside doRenewAppMetadata.
func (s *serviceDiscoveryRegistry) startMetadataTimers() {
@@ -97,6 +101,10 @@ func (s *serviceDiscoveryRegistry) startMetadataTimers() {
if s.metadataReport == nil {
return
}
+ metaInfo :=
metadata.GetMetadataInfo(s.url.GetParam(constant.RegistryIdKey, ""))
+ if metaInfo == nil || !isPublishableRevision(metaInfo.Revision) {
+ return
+ }
s.startRenewAppMetadataTimer()
}
@@ -107,25 +115,36 @@ func (s *serviceDiscoveryRegistry) RegisterService()
error {
panic("no metada info found of registry id " + registryId)
}
urls := metaInfo.GetExportedServiceURLs()
+ if len(urls) == 0 {
+ return nil
+ }
+
+ instances := make([]registry.ServiceInstance, 0, len(urls))
+ instanceURLs := make(map[registry.ServiceInstance]*common.URL)
for _, url := range urls {
instance := createInstance(metaInfo, url, registryId)
metaInfo.Revision =
instance.GetMetadata()[constant.ExportedServicesRevisionPropertyName]
- if metadata.GetMetadataType() ==
constant.RemoteMetadataStorageType {
- if s.metadataReport == nil {
- return perrors.New("can not publish app
metadata cause report instance not found")
- }
- err :=
s.metadataReport.PublishAppMetadata(metaInfo.App, metaInfo.Revision, metaInfo)
- if err != nil {
- return err
- }
+ instances = append(instances, instance)
+ instanceURLs[instance] = url
+ }
+
+ if metadata.GetMetadataType() == constant.RemoteMetadataStorageType {
+ if s.metadataReport == nil {
+ return perrors.New("can not publish app metadata cause
report instance not found")
}
+ if err := s.metadataReport.PublishAppMetadata(metaInfo.App,
metaInfo.Revision, metaInfo); err != nil {
+ return err
+ }
+ }
+
+ for _, instance := range instances {
err := s.serviceDiscovery.Register(instance)
if err != nil {
return perrors.WithMessage(err, "Register service
failed")
}
s.lock.Lock()
s.instances = append(s.instances, instance)
- s.instanceURLs[instance] = url
+ s.instanceURLs[instance] = instanceURLs[instance]
s.lock.Unlock()
}
@@ -374,7 +393,7 @@ func (s *serviceDiscoveryRegistry)
startRenewAppMetadataTimer() {
func (s *serviceDiscoveryRegistry) doRenewAppMetadata() {
registryID := s.url.GetParam(constant.RegistryIdKey, "")
metaInfo := metadata.GetMetadataInfo(registryID)
- if metaInfo == nil || metaInfo.Revision == "0" {
+ if metaInfo == nil || !isPublishableRevision(metaInfo.Revision) {
return
}
diff --git a/registry/servicediscovery/service_discovery_registry_test.go
b/registry/servicediscovery/service_discovery_registry_test.go
index b4965f5fa..608e9b29f 100644
--- a/registry/servicediscovery/service_discovery_registry_test.go
+++ b/registry/servicediscovery/service_discovery_registry_test.go
@@ -97,6 +97,280 @@ func TestServiceDiscoveryRegistryRegister(t *testing.T) {
}
}
+// TestServiceDiscoveryRegistryRegisterPublishesMetadataOnce verifies that
+// app metadata is published exactly once.
+func TestServiceDiscoveryRegistryRegisterPublishesMetadataOnce(t *testing.T) {
+ mockSD, mockMapping := setupEnvironment(t)
+ regID := fmt.Sprintf("mock-reg-%s-%d", t.Name(), time.Now().UnixNano())
+ prevType := metadata.GetMetadataType()
+ opts :=
metadata.NewOptions(metadata.WithMetadataType(constant.RemoteMetadataStorageType))
+ _ = opts.Init()
+ defer func() {
+ restoreOpts :=
metadata.NewOptions(metadata.WithMetadataType(prevType))
+ _ = restoreOpts.Init()
+ }()
+
+ registryURL, err := common.NewURL(testRegistryURL,
+ common.WithParamsValue(constant.RegistryKey, "mock"),
+ common.WithParamsValue(constant.RegistryIdKey, regID))
+ require.NoError(t, err)
+
+ reg, err := newServiceDiscoveryRegistry(registryURL)
+ require.NoError(t, err)
+
+ // Disable the startup renew so the background publish cannot race the
count below.
+ renewURL, err := common.NewURL("mock://127.0.0.1:8848",
+ common.WithParamsValue(constant.MetadataRenewOnStartupKey,
"false"))
+ require.NoError(t, err)
+ countingReport := &mockMetadataReportForGC{reportURL: renewURL}
+ sdReg, ok := reg.(*serviceDiscoveryRegistry)
+ require.True(t, ok)
+ sdReg.metadataReport = countingReport
+ defer sdReg.stopMetadataTimers()
+
+ providerURL1, err := common.NewURL("dubbo://127.0.0.1:20880/",
+ common.WithParamsValue(constant.ApplicationKey, testApp),
+ common.WithInterface(testInterface),
+ common.WithParamsValue(constant.SideKey, constant.SideProvider),
+ )
+ require.NoError(t, err)
+ providerURL2, err := common.NewURL("dubbo://127.0.0.1:20881/",
+ common.WithParamsValue(constant.ApplicationKey, testApp),
+ common.WithInterface(testInterface),
+ common.WithParamsValue(constant.SideKey, constant.SideProvider),
+ )
+ require.NoError(t, err)
+
+ err = reg.Register(providerURL1)
+ require.NoError(t, err)
+ err = reg.Register(providerURL2)
+ require.NoError(t, err)
+ assert.True(t, mockMapping.mapCalled, "ServiceNameMapping.Map should be
called")
+
+ err = sdReg.RegisterService()
+ require.NoError(t, err)
+
+ assert.True(t, mockSD.registerCalled, "ServiceDiscovery.Register should
be called")
+ assert.Len(t, sdReg.instances, 2)
+ assert.Equal(t, 1, countingReport.published)
+}
+
+// TestServiceDiscoveryRegistryRegisterReturnsPublishError verifies that
+// RegisterService propagates a PublishAppMetadata error.
+func TestServiceDiscoveryRegistryRegisterReturnsPublishError(t *testing.T) {
+ mockSD, _ := setupEnvironment(t)
+ regID := fmt.Sprintf("mock-reg-%s-%d", t.Name(), time.Now().UnixNano())
+ prevType := metadata.GetMetadataType()
+ opts :=
metadata.NewOptions(metadata.WithMetadataType(constant.RemoteMetadataStorageType))
+ _ = opts.Init()
+ defer func() {
+ restoreOpts :=
metadata.NewOptions(metadata.WithMetadataType(prevType))
+ _ = restoreOpts.Init()
+ }()
+
+ registryURL, err := common.NewURL(testRegistryURL,
+ common.WithParamsValue(constant.RegistryKey, "mock"),
+ common.WithParamsValue(constant.RegistryIdKey, regID))
+ require.NoError(t, err)
+
+ reg, err := newServiceDiscoveryRegistry(registryURL)
+ require.NoError(t, err)
+
+ failingReport := &mockMetadataReportForGC{publishErr: errors.New("mock
publish failed")}
+ sdReg, ok := reg.(*serviceDiscoveryRegistry)
+ require.True(t, ok)
+ sdReg.metadataReport = failingReport
+
+ providerURL, err := common.NewURL("dubbo://127.0.0.1:20880/",
+ common.WithParamsValue(constant.ApplicationKey, testApp),
+ common.WithInterface(testInterface),
+ common.WithParamsValue(constant.SideKey, constant.SideProvider),
+ )
+ require.NoError(t, err)
+
+ err = reg.Register(providerURL)
+ require.NoError(t, err)
+
+ err = sdReg.RegisterService()
+ require.EqualError(t, err, "mock publish failed")
+ assert.False(t, mockSD.registerCalled, "ServiceDiscovery.Register
should not be called when publish fails")
+ assert.Empty(t, sdReg.instances, "no instance should remain after a
failed publish")
+}
+
+// TestServiceDiscoveryRegistryRegisterRetryAfterPublishFailure verifies that
retrying
+// after a failed publish starts clean: no instances left behind, then
registers normally.
+func TestServiceDiscoveryRegistryRegisterRetryAfterPublishFailure(t
*testing.T) {
+ mockSD, _ := setupEnvironment(t)
+ regID := fmt.Sprintf("mock-reg-%s-%d", t.Name(), time.Now().UnixNano())
+ prevType := metadata.GetMetadataType()
+ opts :=
metadata.NewOptions(metadata.WithMetadataType(constant.RemoteMetadataStorageType))
+ _ = opts.Init()
+ defer func() {
+ restoreOpts :=
metadata.NewOptions(metadata.WithMetadataType(prevType))
+ _ = restoreOpts.Init()
+ }()
+
+ registryURL, err := common.NewURL(testRegistryURL,
+ common.WithParamsValue(constant.RegistryKey, "mock"),
+ common.WithParamsValue(constant.RegistryIdKey, regID))
+ require.NoError(t, err)
+
+ reg, err := newServiceDiscoveryRegistry(registryURL)
+ require.NoError(t, err)
+
+ report := &mockMetadataReportForGC{publishErr: errors.New("mock publish
failed")}
+ sdReg, ok := reg.(*serviceDiscoveryRegistry)
+ require.True(t, ok)
+ sdReg.metadataReport = report
+
+ providerURL, err := common.NewURL("dubbo://127.0.0.1:20880/",
+ common.WithParamsValue(constant.ApplicationKey, testApp),
+ common.WithInterface(testInterface),
+ common.WithParamsValue(constant.SideKey, constant.SideProvider),
+ )
+ require.NoError(t, err)
+
+ err = reg.Register(providerURL)
+ require.NoError(t, err)
+
+ // First call: publish fails, so nothing is registered and no state is
left behind.
+ err = sdReg.RegisterService()
+ require.EqualError(t, err, "mock publish failed")
+ assert.False(t, mockSD.registerCalled, "ServiceDiscovery.Register
should not be called when publish fails")
+ assert.Empty(t, sdReg.instances, "no instance should remain after a
failed publish")
+
+ // Retry: publish succeeds, the instance is registered and recorded
exactly once.
+ report.publishErr = nil
+ err = sdReg.RegisterService()
+ require.NoError(t, err)
+ assert.True(t, mockSD.registerCalled, "ServiceDiscovery.Register should
be called on retry")
+ assert.Len(t, sdReg.instances, 1)
+}
+
+// TestServiceDiscoveryRegistryRegisterPureConsumerDoesNotPublish verifies
that a pure
+// consumer (no exported URLs, only subscriptions) never publishes or
registers.
+func TestServiceDiscoveryRegistryRegisterPureConsumerDoesNotPublish(t
*testing.T) {
+ mockSD, _ := setupEnvironment(t)
+ regID := fmt.Sprintf("mock-reg-%s-%d", t.Name(), time.Now().UnixNano())
+ prevType := metadata.GetMetadataType()
+ opts :=
metadata.NewOptions(metadata.WithMetadataType(constant.RemoteMetadataStorageType))
+ _ = opts.Init()
+ defer func() {
+ restoreOpts :=
metadata.NewOptions(metadata.WithMetadataType(prevType))
+ _ = restoreOpts.Init()
+ }()
+
+ registryURL, err := common.NewURL(testRegistryURL,
+ common.WithParamsValue(constant.RegistryKey, "mock"),
+ common.WithParamsValue(constant.RegistryIdKey, regID))
+ require.NoError(t, err)
+
+ reg, err := newServiceDiscoveryRegistry(registryURL)
+ require.NoError(t, err)
+
+ sdReg, ok := reg.(*serviceDiscoveryRegistry)
+ require.True(t, ok)
+
+ countingReport := &mockMetadataReportForGC{}
+ sdReg.metadataReport = countingReport
+
+ // A pure consumer only subscribes; the consumer URL still creates a
+ // MetadataInfo whose exported URLs are empty.
+ consumerURL, err := common.NewURL("dubbo://127.0.0.1:20000/",
+ common.WithParamsValue(constant.ApplicationKey, testApp),
+ common.WithInterface(testInterface),
+ common.WithParamsValue(constant.SideKey, constant.SideConsumer),
+ )
+ require.NoError(t, err)
+ metadata.AddSubscribeURL(regID, consumerURL)
+
+ err = sdReg.RegisterService()
+ require.NoError(t, err)
+
+ assert.False(t, mockSD.registerCalled, "ServiceDiscovery.Register
should not be called for a pure consumer")
+ assert.Equal(t, 0, countingReport.published, "app metadata must not be
published when exported URLs are empty")
+}
+
+// TestServiceDiscoveryRegistryRegisterNilReportReturnsError verifies that a
nil
+// metadata report is checked before any dereference instead of panicking.
+func TestServiceDiscoveryRegistryRegisterNilReportReturnsError(t *testing.T) {
+ mockSD, _ := setupEnvironment(t)
+ regID := fmt.Sprintf("mock-reg-%s-%d", t.Name(), time.Now().UnixNano())
+ prevType := metadata.GetMetadataType()
+ opts :=
metadata.NewOptions(metadata.WithMetadataType(constant.RemoteMetadataStorageType))
+ _ = opts.Init()
+ defer func() {
+ restoreOpts :=
metadata.NewOptions(metadata.WithMetadataType(prevType))
+ _ = restoreOpts.Init()
+ }()
+
+ registryURL, err := common.NewURL(testRegistryURL,
+ common.WithParamsValue(constant.RegistryKey, "mock"),
+ common.WithParamsValue(constant.RegistryIdKey, regID))
+ require.NoError(t, err)
+
+ reg, err := newServiceDiscoveryRegistry(registryURL)
+ require.NoError(t, err)
+
+ sdReg, ok := reg.(*serviceDiscoveryRegistry)
+ require.True(t, ok)
+ // Remote mode without a configured metadata report instance.
+ sdReg.metadataReport = nil
+
+ providerURL, err := common.NewURL("dubbo://127.0.0.1:20880/",
+ common.WithParamsValue(constant.ApplicationKey, testApp),
+ common.WithInterface(testInterface),
+ common.WithParamsValue(constant.SideKey, constant.SideProvider),
+ )
+ require.NoError(t, err)
+
+ err = reg.Register(providerURL)
+ require.NoError(t, err)
+
+ err = sdReg.RegisterService()
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "report instance not found")
+ assert.False(t, mockSD.registerCalled, "no instance should be
registered when the metadata report is nil")
+}
+
+// TestServiceDiscoveryRegistryRegisterLocalMetadataDoesNotPublish verifies
that
+// non-remote metadata mode publishes app metadata zero times.
+func TestServiceDiscoveryRegistryRegisterLocalMetadataDoesNotPublish(t
*testing.T) {
+ mockSD, _ := setupEnvironment(t)
+ regID := fmt.Sprintf("mock-reg-%s-%d", t.Name(), time.Now().UnixNano())
+ // setupEnvironment leaves the metadata type as "mock", i.e. not remote.
+
+ registryURL, err := common.NewURL(testRegistryURL,
+ common.WithParamsValue(constant.RegistryKey, "mock"),
+ common.WithParamsValue(constant.RegistryIdKey, regID))
+ require.NoError(t, err)
+
+ reg, err := newServiceDiscoveryRegistry(registryURL)
+ require.NoError(t, err)
+
+ sdReg, ok := reg.(*serviceDiscoveryRegistry)
+ require.True(t, ok)
+
+ countingReport := &mockMetadataReportForGC{}
+ sdReg.metadataReport = countingReport
+
+ providerURL, err := common.NewURL("dubbo://127.0.0.1:20880/",
+ common.WithParamsValue(constant.ApplicationKey, testApp),
+ common.WithInterface(testInterface),
+ common.WithParamsValue(constant.SideKey, constant.SideProvider),
+ )
+ require.NoError(t, err)
+
+ err = reg.Register(providerURL)
+ require.NoError(t, err)
+
+ err = sdReg.RegisterService()
+ require.NoError(t, err)
+
+ assert.True(t, mockSD.registerCalled, "ServiceDiscovery.Register should
be called")
+ assert.Equal(t, 0, countingReport.published, "app metadata must not be
published in non-remote metadata mode")
+}
+
// TestServiceDiscoveryRegistrySubscribe verifies the subscription flow.
func TestServiceDiscoveryRegistrySubscribe(t *testing.T) {
mockSD, mockMapping := setupEnvironment(t)
@@ -613,10 +887,11 @@ func (m *mockProxyFactory) GetInvoker(url *common.URL)
protocol.Invoker { return
// mockMetadataReportForGC is a lightweight mock for testing GC logic
type mockMetadataReportForGC struct {
- revisions []report.AppRevision
- deleted []string // tracks deleted revisions
- published int // tracks publish calls
- reportURL *common.URL
+ revisions []report.AppRevision
+ deleted []string // tracks deleted revisions
+ published int // tracks publish calls
+ reportURL *common.URL
+ publishErr error // optional error returned by PublishAppMetadata
}
func (m *mockMetadataReportForGC) GetAppMetadata(string, string)
(*info.MetadataInfo, error) {
@@ -624,6 +899,9 @@ func (m *mockMetadataReportForGC) GetAppMetadata(string,
string) (*info.Metadata
}
func (m *mockMetadataReportForGC) PublishAppMetadata(string, string,
*info.MetadataInfo) error {
m.published++
+ if m.publishErr != nil {
+ return m.publishErr
+ }
return nil
}
func (m *mockMetadataReportForGC) RegisterServiceAppMapping(string, string,
string) error {
@@ -816,6 +1094,57 @@ func TestServiceDiscoveryRegistry_DoRenewAppMetadata(t
*testing.T) {
assert.Equal(t, 1, mockReport.published)
}
+// TestServiceDiscoveryRegistry_StartMetadataTimersSkipsEmptyRevision verifies
that the
+// renewAppMetadata timer is not started while the metadata revision is an
empty
+// marker ("" initial state, "0" after exports are cleared, or "N/A" customizer
+// default), otherwise doRenewAppMetadata would spin as a no-op.
+func TestServiceDiscoveryRegistry_StartMetadataTimersSkipsEmptyRevision(t
*testing.T) {
+ prevType := metadata.GetMetadataType()
+ opts :=
metadata.NewOptions(metadata.WithMetadataType(constant.RemoteMetadataStorageType))
+ _ = opts.Init()
+ defer func() {
+ restoreOpts :=
metadata.NewOptions(metadata.WithMetadataType(prevType))
+ _ = restoreOpts.Init()
+ }()
+
+ mockReport := &mockMetadataReportForGC{}
+
+ tests := []struct {
+ name string
+ revision string
+ }{
+ {"initial empty revision", ""},
+ {"cleared revision", "0"},
+ {"customizer default revision", "N/A"},
+ }
+ for i, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ regID := fmt.Sprintf("timer-empty-rev-reg-%d-%d", i,
time.Now().UnixNano())
+ url := common.NewURLWithOptions(
+ common.WithParamsValue(constant.RegistryIdKey,
regID),
+ )
+
+ reg := &serviceDiscoveryRegistry{
+ url: url,
+ metadataReport: mockReport,
+ }
+
+ serviceURL, _ :=
common.NewURL("dubbo://127.0.0.1:20880/org.test.EmptyRevision",
+ common.WithParamsValue(constant.ApplicationKey,
"test-app"),
+ common.WithParamsValue(constant.SideKey,
constant.SideProvider),
+ )
+ metadata.AddService(regID, serviceURL)
+ metaInfo := metadata.GetMetadataInfo(regID)
+ require.NotNil(t, metaInfo)
+ metaInfo.Revision = tt.revision
+
+ reg.startMetadataTimers()
+
+ assert.Nil(t, reg.renewAppMetadataTimer)
+ })
+ }
+}
+
func TestServiceDiscoveryRegistry_Destroy_StopsTimers(t *testing.T) {
url := common.NewURLWithOptions()
sd := &mockServiceDiscovery{}
diff --git a/server/server_test.go b/server/server_test.go
index d27229de2..6217d4c5f 100644
--- a/server/server_test.go
+++ b/server/server_test.go
@@ -19,6 +19,7 @@ package server
import (
"context"
+ "errors"
"fmt"
"os"
"os/signal"
@@ -32,6 +33,7 @@ import (
)
import (
+ gxset "github.com/dubbogo/gost/container/set"
gostlogger "github.com/dubbogo/gost/log/logger"
"github.com/stretchr/testify/assert"
@@ -44,6 +46,9 @@ import (
"dubbo.apache.org/dubbo-go/v3/common/extension"
"dubbo.apache.org/dubbo-go/v3/global"
"dubbo.apache.org/dubbo-go/v3/graceful_shutdown"
+ "dubbo.apache.org/dubbo-go/v3/metadata/info"
+ "dubbo.apache.org/dubbo-go/v3/metadata/mapping"
+ "dubbo.apache.org/dubbo-go/v3/metadata/report"
"dubbo.apache.org/dubbo-go/v3/protocol/base"
"dubbo.apache.org/dubbo-go/v3/registry"
)
@@ -104,12 +109,20 @@ type mockServeRegistryFactoryProtocol struct {
base.BaseProtocol
}
+type failingServeRegistryFactoryProtocol struct {
+ base.BaseProtocol
+}
+
type mockServeRegistry struct{}
func (p *mockServeRegistryFactoryProtocol) GetRegistries() []registry.Registry
{
return []registry.Registry{&mockServeRegistry{}}
}
+func (p *failingServeRegistryFactoryProtocol) GetRegistries()
[]registry.Registry {
+ return []registry.Registry{&failingServeRegistry{metadataReport:
&publishFailingMetadataReport{}}}
+}
+
func (r *mockServeRegistry) GetURL() *common.URL {
return &common.URL{}
}
@@ -148,6 +161,86 @@ func (r *mockServeRegistry) UnRegisterService() error {
return nil
}
+// failingServeRegistry is a registry mock whose RegisterService calls
+// PublishAppMetadata and returns the error when the report cannot publish.
+type failingServeRegistry struct {
+ metadataReport report.MetadataReport
+}
+
+func (r *failingServeRegistry) GetURL() *common.URL {
+ return &common.URL{}
+}
+
+func (r *failingServeRegistry) IsAvailable() bool {
+ return true
+}
+
+func (r *failingServeRegistry) Destroy() {}
+
+func (r *failingServeRegistry) Register(*common.URL) error {
+ return nil
+}
+
+func (r *failingServeRegistry) UnRegister(*common.URL) error {
+ return nil
+}
+
+func (r *failingServeRegistry) Subscribe(*common.URL, registry.NotifyListener)
error {
+ return nil
+}
+
+func (r *failingServeRegistry) UnSubscribe(*common.URL,
registry.NotifyListener) error {
+ return nil
+}
+
+func (r *failingServeRegistry) LoadSubscribeInstances(*common.URL,
registry.NotifyListener) error {
+ return nil
+}
+
+func (r *failingServeRegistry) RegisterService() error {
+ return r.metadataReport.PublishAppMetadata("", "", nil)
+}
+
+func (r *failingServeRegistry) UnRegisterService() error {
+ return nil
+}
+
+// publishFailingMetadataReport is a metadata report mock whose publish always
+// fails, so a registry using it hits the real publish path and returns the
error.
+type publishFailingMetadataReport struct{}
+
+func (m *publishFailingMetadataReport) GetAppMetadata(_, _ string)
(*info.MetadataInfo, error) {
+ return nil, nil
+}
+
+func (m *publishFailingMetadataReport) PublishAppMetadata(_, _ string, _
*info.MetadataInfo) error {
+ return errors.New("mock publish app metadata failed")
+}
+
+func (m *publishFailingMetadataReport) RegisterServiceAppMapping(_, _, _
string) error {
+ return nil
+}
+
+func (m *publishFailingMetadataReport) GetServiceAppMapping(_ string, _
string, _ mapping.MappingListener) (*gxset.HashSet, error) {
+ return nil, nil
+}
+
+func (m *publishFailingMetadataReport) RemoveServiceAppMappingListener(_, _
string) error {
+ return nil
+}
+
+func (m *publishFailingMetadataReport) UnPublishAppMetadata(_, _ string) error
{
+ return nil
+}
+
+func (m *publishFailingMetadataReport) ListAppRevisions(_ string)
([]report.AppRevision, error) {
+ return nil, nil
+}
+
+func (m *publishFailingMetadataReport) URL() *common.URL {
+ return nil
+}
+
type countingServeExporter struct {
invoker base.Invoker
unexportCount *atomic.Int32
@@ -267,6 +360,29 @@ func registerServeTestProtocols(t *testing.T) {
})
}
+func registerFailingServeTestProtocols(t *testing.T) {
+ t.Helper()
+
+ originalProtocols := extensionProtocols.Snapshot()
+ extension.SetProtocol("dubbo", func() base.Protocol {
+ return &mockServeProtocol{BaseProtocol: base.NewBaseProtocol()}
+ })
+ extension.SetProtocol(constant.RegistryKey, func() base.Protocol {
+ return &failingServeRegistryFactoryProtocol{BaseProtocol:
base.NewBaseProtocol()}
+ })
+ t.Cleanup(func() {
+ for name, factory := range originalProtocols {
+ extension.SetProtocol(name, factory)
+ }
+ if _, ok := originalProtocols["dubbo"]; !ok {
+ extension.UnregisterProtocol("dubbo")
+ }
+ if _, ok := originalProtocols[constant.RegistryKey]; !ok {
+ extension.UnregisterProtocol(constant.RegistryKey)
+ }
+ })
+}
+
func registerCountingServeTestProtocols(
t *testing.T,
exportCount, unexportCount, registerCount, unregisterCount
*atomic.Int32,
@@ -443,6 +559,36 @@ func TestServeContextRollsBackWhenCanceledDuringStartup(t
*testing.T) {
assert.Equal(t, int32(1), unregisterCount.Load())
}
+// TestServeContextReturnsErrorWhenMetadataPublishFails verifies that a
metadata
+// publish failure is propagated back to the caller of ServeContext. The
registry
+// mock calls PublishAppMetadata while the metadata report is mocked to fail.
+func TestServeContextReturnsErrorWhenMetadataPublishFails(t *testing.T) {
+ resetGracefulShutdownStateForTest(t)
+ t.Cleanup(func() {
+ resetGracefulShutdownStateForTest(t)
+ })
+ resetInternalProviderServicesForTest(t)
+ registerFailingServeTestProtocols(t)
+
+ internalSignal := false
+ shutdownCfg := global.DefaultShutdownConfig()
+ shutdownCfg.InternalSignal = &internalSignal
+ shutdownCfg.ConsumerUpdateWaitTime = "0s"
+ shutdownCfg.StepTimeout = "0s"
+ shutdownCfg.NotifyTimeout = "10ms"
+ shutdownCfg.OfflineRequestWindowTimeout = "0s"
+
+ srv, err := NewServer(SetServerShutdown(shutdownCfg))
+ require.NoError(t, err)
+ require.NoError(t, srv.Register(&MockServerRPCService{}, nil))
+
+ ctx := t.Context()
+
+ err = srv.ServeContext(ctx)
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "mock publish app metadata failed")
+}
+
func TestServeContextDoesNotRestartAfterGracefulShutdownCompletes(t
*testing.T) {
resetGracefulShutdownStateForTest(t)
t.Cleanup(func() {
