fix AbstractServiceDiscovery.update when doRegister first failed (#14427

)
apache · Jul 16, 2024 · c0fec56 · c0fec56
1 parent eca57d3
commit c0fec56
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 2 deletions.
diff --git a/...registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java b/...registry-api/src/main/java/org/apache/dubbo/registry/client/AbstractServiceDiscovery.java
@@ -157,8 +157,13 @@ public synchronized void register() throws RuntimeException {
         }
         boolean revisionUpdated = calOrUpdateInstanceRevision(this.serviceInstance);
         if (revisionUpdated) {
-            reportMetadata(this.metadataInfo);
-            doRegister(this.serviceInstance);
+            try {
+                reportMetadata(this.metadataInfo);
+                doRegister(this.serviceInstance);
+            } catch (Exception e) {
+                this.serviceInstance = null;
+                throw e;
+            }
         }
     }
 

diff --git a/...egistry-api/src/test/java/org/apache/dubbo/registry/client/ServiceDiscoveryCacheTest.java b/...egistry-api/src/test/java/org/apache/dubbo/registry/client/ServiceDiscoveryCacheTest.java
@@ -91,4 +91,35 @@ void test() throws InterruptedException {
 
         applicationModel.destroy();
     }
+
+    /**
+     * to fix #14126
+     */
+    @Test
+    void testUpdateWhenFirstDoRegisterFail() throws InterruptedException {
+        ApplicationModel applicationModel = FrameworkModel.defaultModel().newApplication();
+        applicationModel.getApplicationConfigManager().setApplication(new ApplicationConfig("Test"));
+
+        URL registryUrl = URL.valueOf("mock://127.0.0.1:12345").addParameter(METADATA_INFO_CACHE_EXPIRE_KEY, 10);
+        MockServiceDiscovery mockServiceDiscovery =
+                Mockito.spy(new MockServiceDiscovery(applicationModel, registryUrl));
+
+        mockServiceDiscovery.register(URL.valueOf("mock://127.0.0.1:12345")
+                .setServiceInterface("org.apache.dubbo.registry.service.DemoService"));
+
+        Thread.sleep(100);
+        Mockito.doThrow(new RuntimeException())
+                .when(mockServiceDiscovery)
+                .doRegister(Mockito.any(ServiceInstance.class));
+        Assertions.assertThrows(RuntimeException.class, mockServiceDiscovery::update);
+
+        Thread.sleep(100);
+        Mockito.doNothing().when(mockServiceDiscovery).doRegister(Mockito.any(ServiceInstance.class));
+        Assertions.assertDoesNotThrow(mockServiceDiscovery::update);
+
+        Thread.sleep(100);
+        Assertions.assertDoesNotThrow(mockServiceDiscovery::update);
+
+        applicationModel.destroy();
+    }
 }