From b140588c6738f3e27a5841e1263291e57c4e359c Mon Sep 17 00:00:00 2001 From: asapple <960099622@qq.ocm> Date: Mon, 9 Dec 2024 17:28:12 +0800 Subject: [PATCH 1/4] 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..588023bdfa6 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 addrs) { - List 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 newAddresses) { + List 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 newAddresses, List oldAddresses) { + if (newAddresses == null || 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 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 04bf21e02d9dc70c526c7ed0a1091f5b8780f90b Mon Sep 17 00:00:00 2001 From: asapple <960099622@qq.com> Date: Mon, 9 Dec 2024 23:02:36 +0800 Subject: [PATCH 2/4] Remove the null value check for the new address. --- .../org/apache/rocketmq/remoting/netty/NettyRemotingClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 588023bdfa6..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 @@ -518,7 +518,7 @@ public void updateNameServerAddressList(List newAddresses) { * Check if the address list should be updated */ private static boolean shouldUpdateAddressList(List newAddresses, List oldAddresses) { - if (newAddresses == null || newAddresses.isEmpty()) { + if (newAddresses.isEmpty()) { return false; } From 26c46eb1d72276135a22575aca299ba8f2933b12 Mon Sep 17 00:00:00 2001 From: asapple <960099622@qq.ocm> Date: Mon, 9 Dec 2024 17:28:12 +0800 Subject: [PATCH 3/4] 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..588023bdfa6 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 addrs) { - List 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 newAddresses) { + List 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 newAddresses, List oldAddresses) { + if (newAddresses == null || 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 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 dbbb1fe2ef83bf047cea30b2704696aa8a31df01 Mon Sep 17 00:00:00 2001 From: asapple <960099622@qq.com> Date: Mon, 9 Dec 2024 23:02:36 +0800 Subject: [PATCH 4/4] Remove the null value check for the new address. --- .../org/apache/rocketmq/remoting/netty/NettyRemotingClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 588023bdfa6..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 @@ -518,7 +518,7 @@ public void updateNameServerAddressList(List newAddresses) { * Check if the address list should be updated */ private static boolean shouldUpdateAddressList(List newAddresses, List oldAddresses) { - if (newAddresses == null || newAddresses.isEmpty()) { + if (newAddresses.isEmpty()) { return false; }