From 4f0c8ff2b96c97153a35a1955c4f7a9457d7669e Mon Sep 17 00:00:00 2001
From: asapple <960099622@qq.com>
Date: Tue, 10 Dec 2024 17:06:55 +0800
Subject: [PATCH 1/2] Refactor name server address update logic

- Simplified the logic for checking if the name server address list needs to be updated.
- Moved channel closure logic into a new method `handleChannelClosureIfNeeded`.
- Improved maintainability and readability of the `updateNameServerAddressList` method.

No functional changes, just refactoring for better clarity and maintainability.
---
 .../remoting/netty/NettyRemotingClient.java   | 89 ++++++++++++-------
 1 file changed, 56 insertions(+), 33 deletions(-)

diff --git a/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java b/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java
index 6ac54aed6d2..609f59ac95d 100644
--- a/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java
+++ b/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java
@@ -500,41 +500,64 @@ public void closeChannel(final Channel channel) {
     }
 
     @Override
-    public void updateNameServerAddressList(List<String> addrs) {
-        List<String> old = this.namesrvAddrList.get();
-        boolean update = false;
-
-        if (!addrs.isEmpty()) {
-            if (null == old) {
-                update = true;
-            } else if (addrs.size() != old.size()) {
-                update = true;
-            } else {
-                for (String addr : addrs) {
-                    if (!old.contains(addr)) {
-                        update = true;
-                        break;
-                    }
-                }
+    public void updateNameServerAddressList(List<String> newAddresses) {
+        List<String> oldAddresses = this.namesrvAddrList.get();
+
+        // Check if the address list needs to be updated
+        if (shouldUpdateAddressList(newAddresses, oldAddresses)) {
+            Collections.shuffle(newAddresses);
+            LOGGER.info("name server address updated. NEW : {} , OLD: {}", newAddresses, oldAddresses);
+            this.namesrvAddrList.set(newAddresses);
+
+            // Handle channel closure if the chosen address is not in the new list
+            handleChannelClosureIfNeeded(newAddresses);
+        }
+    }
+
+    /**
+     * Check if the address list should be updated
+     */
+    private static boolean shouldUpdateAddressList(List<String> newAddresses, List<String> oldAddresses) {
+        if (newAddresses.isEmpty()) {
+            return false;
+        }
+
+        if (oldAddresses == null || newAddresses.size() != oldAddresses.size()) {
+            return true;
+        }
+
+        for (String addr : newAddresses) {
+            if (!oldAddresses.contains(addr)) {
+                return true;
             }
+        }
 
-            if (update) {
-                Collections.shuffle(addrs);
-                LOGGER.info("name server address updated. NEW : {} , OLD: {}", addrs, old);
-                this.namesrvAddrList.set(addrs);
-
-                // should close the channel if choosed addr is not exist.
-                String chosenNameServerAddr = this.namesrvAddrChoosed.get();
-                if (chosenNameServerAddr != null && !addrs.contains(chosenNameServerAddr)) {
-                    namesrvAddrChoosed.compareAndSet(chosenNameServerAddr, null);
-                    for (String addr : this.channelTables.keySet()) {
-                        if (addr.contains(chosenNameServerAddr)) {
-                            ChannelWrapper channelWrapper = this.channelTables.get(addr);
-                            if (channelWrapper != null) {
-                                channelWrapper.close();
-                            }
-                        }
-                    }
+        return false;
+    }
+
+    /**
+     * Handle channel closure if the chosen address is no longer in the new list
+     */
+    private void handleChannelClosureIfNeeded(List<String> newAddresses) {
+        String chosenNameServerAddr = this.namesrvAddrChoosed.get();
+        if (chosenNameServerAddr != null && !newAddresses.contains(chosenNameServerAddr)) {
+            // Set the chosen address to null
+            namesrvAddrChoosed.compareAndSet(chosenNameServerAddr, null);
+
+            // Close the channels associated with the chosen address
+            closeChannelsForAddress(chosenNameServerAddr);
+        }
+    }
+
+    /**
+     * Close channels associated with the given address
+     */
+    private void closeChannelsForAddress(String chosenNameServerAddr) {
+        for (String addr : this.channelTables.keySet()) {
+            if (addr.contains(chosenNameServerAddr)) {
+                ChannelWrapper channelWrapper = this.channelTables.get(addr);
+                if (channelWrapper != null) {
+                    channelWrapper.close();
                 }
             }
         }

From 2152ae8c21d1a3ce232c04c5a2c6ccd72bfe9204 Mon Sep 17 00:00:00 2001
From: asapple <960099622@qq.com>
Date: Tue, 10 Dec 2024 17:14:33 +0800
Subject: [PATCH 2/2] Optimize shouldUpdateAddressList method using containsAll

---
 .../rocketmq/remoting/netty/NettyRemotingClient.java      | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java b/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java
index 609f59ac95d..8179341d80c 100644
--- a/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java
+++ b/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java
@@ -526,13 +526,7 @@ private static boolean shouldUpdateAddressList(List<String> newAddresses, List<S
             return true;
         }
 
-        for (String addr : newAddresses) {
-            if (!oldAddresses.contains(addr)) {
-                return true;
-            }
-        }
-
-        return false;
+        return !new HashSet<>(newAddresses).containsAll(oldAddresses);
     }
 
     /**