[ISSUE #6873] If dns resolve controller address exception will update…

… controllerAddresses to null (#8180) * Adding null does not update * rolling back * dns resolution failure not updating controllerAddresses
apache · May 24, 2024 · bdc7c0a · bdc7c0a
1 parent b58eefc
commit bdc7c0a
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 7 deletions.
diff --git a/broker/src/main/java/org/apache/rocketmq/broker/controller/ReplicasManager.java b/broker/src/main/java/org/apache/rocketmq/broker/controller/ReplicasManager.java
@@ -30,7 +30,7 @@
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
-
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.rocketmq.broker.BrokerController;
 import org.apache.rocketmq.broker.out.BrokerOuterAPI;
@@ -803,7 +803,10 @@ private void scanAvailableControllerAddresses() {
 
     private void updateControllerAddr() {
         if (brokerConfig.isFetchControllerAddrByDnsLookup()) {
-            this.controllerAddresses = brokerOuterAPI.dnsLookupAddressByDomain(this.brokerConfig.getControllerAddr());
+            List<String> adders = brokerOuterAPI.dnsLookupAddressByDomain(this.brokerConfig.getControllerAddr());
+            if (CollectionUtils.isNotEmpty(adders)) {
+                this.controllerAddresses = adders;
+            }
         } else {
             final String controllerPaths = this.brokerConfig.getControllerAddr();
             final String[] controllers = controllerPaths.split(";");

diff --git a/broker/src/test/java/org/apache/rocketmq/broker/controller/ReplicasManagerTest.java b/broker/src/test/java/org/apache/rocketmq/broker/controller/ReplicasManagerTest.java
@@ -17,12 +17,15 @@
 
 package org.apache.rocketmq.broker.controller;
 
+import com.google.common.collect.Lists;
 import java.io.File;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.List;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
-
 import org.apache.rocketmq.broker.BrokerController;
 import org.apache.rocketmq.broker.out.BrokerOuterAPI;
 import org.apache.rocketmq.broker.slave.SlaveSynchronize;
@@ -31,11 +34,11 @@
 import org.apache.rocketmq.common.Pair;
 import org.apache.rocketmq.common.UtilAll;
 import org.apache.rocketmq.remoting.protocol.body.SyncStateSet;
+import org.apache.rocketmq.remoting.protocol.header.controller.ElectMasterResponseHeader;
 import org.apache.rocketmq.remoting.protocol.header.controller.GetMetaDataResponseHeader;
 import org.apache.rocketmq.remoting.protocol.header.controller.GetReplicaInfoResponseHeader;
 import org.apache.rocketmq.remoting.protocol.header.controller.register.ApplyBrokerIdResponseHeader;
 import org.apache.rocketmq.remoting.protocol.header.controller.register.GetNextBrokerIdResponseHeader;
-import org.apache.rocketmq.remoting.protocol.header.controller.ElectMasterResponseHeader;
 import org.apache.rocketmq.remoting.protocol.header.controller.register.RegisterBrokerToControllerResponseHeader;
 import org.apache.rocketmq.store.DefaultMessageStore;
 import org.apache.rocketmq.store.RunningFlags;
@@ -52,6 +55,7 @@
 
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -189,11 +193,11 @@ public void changeBrokerRoleTest() {
         syncStateSetA.add(BROKER_ID_2);
         // not equal to localAddress
         Assertions.assertThatCode(() -> replicasManager.changeBrokerRole(BROKER_ID_2, NEW_MASTER_ADDRESS, NEW_MASTER_EPOCH, OLD_MASTER_EPOCH, syncStateSetB))
-                .doesNotThrowAnyException();
+            .doesNotThrowAnyException();
 
         // equal to localAddress
         Assertions.assertThatCode(() -> replicasManager.changeBrokerRole(BROKER_ID_1, OLD_MASTER_ADDRESS, NEW_MASTER_EPOCH, OLD_MASTER_EPOCH, syncStateSetA))
-                .doesNotThrowAnyException();
+            .doesNotThrowAnyException();
     }
 
     @Test
@@ -206,6 +210,28 @@ public void changeToMasterTest() {
     @Test
     public void changeToSlaveTest() {
         Assertions.assertThatCode(() -> replicasManager.changeToSlave(NEW_MASTER_ADDRESS, NEW_MASTER_EPOCH, BROKER_ID_2))
-                .doesNotThrowAnyException();
+            .doesNotThrowAnyException();
     }
+
+    @Test
+    public void testUpdateControllerAddr() throws Exception {
+        final String controllerAddr = "192.168.1.1";
+        brokerConfig.setFetchControllerAddrByDnsLookup(true);
+        when(brokerOuterAPI.dnsLookupAddressByDomain(anyString())).thenReturn(Lists.newArrayList(controllerAddr));
+        Method method = ReplicasManager.class.getDeclaredMethod("updateControllerAddr");
+        method.setAccessible(true);
+        method.invoke(replicasManager);
+
+        List<String> addresses = replicasManager.getControllerAddresses();
+        Assertions.assertThat(addresses).contains(controllerAddr);
+
+        // Simulating dns resolution exceptions
+        when(brokerOuterAPI.dnsLookupAddressByDomain(anyString())).thenReturn(new ArrayList<>());
+
+        method.invoke(replicasManager);
+        addresses = replicasManager.getControllerAddresses();
+        Assertions.assertThat(addresses).contains(controllerAddr);
+
+    }
+
 }