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);
}
/**