You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by ji...@apache.org on 2022/09/14 03:35:24 UTC

[rocketmq] branch develop updated: [ISSUE #5059] Remove redundant try&catch in RouteInfoManager

This is an automated email from the ASF dual-hosted git repository.

jinrongtong pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/rocketmq.git


The following commit(s) were added to refs/heads/develop by this push:
     new 49e2367b9 [ISSUE #5059] Remove redundant try&catch in RouteInfoManager
49e2367b9 is described below

commit 49e2367b94a0b6f32218903bcdec8c5b11bc3a72
Author: zhiliatom <87...@users.noreply.github.com>
AuthorDate: Wed Sep 14 11:35:17 2022 +0800

    [ISSUE #5059] Remove redundant try&catch in RouteInfoManager
---
 .../namesrv/routeinfo/RouteInfoManager.java        | 136 ++++++++++-----------
 1 file changed, 68 insertions(+), 68 deletions(-)

diff --git a/namesrv/src/main/java/org/apache/rocketmq/namesrv/routeinfo/RouteInfoManager.java b/namesrv/src/main/java/org/apache/rocketmq/namesrv/routeinfo/RouteInfoManager.java
index 91ec4ca1f..99c9975a2 100644
--- a/namesrv/src/main/java/org/apache/rocketmq/namesrv/routeinfo/RouteInfoManager.java
+++ b/namesrv/src/main/java/org/apache/rocketmq/namesrv/routeinfo/RouteInfoManager.java
@@ -18,6 +18,7 @@ package org.apache.rocketmq.namesrv.routeinfo;
 
 import com.google.common.collect.Sets;
 import io.netty.channel.Channel;
+
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -32,6 +33,7 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
+
 import org.apache.commons.lang3.StringUtils;
 import org.apache.rocketmq.common.BrokerAddrInfo;
 import org.apache.rocketmq.common.DataVersion;
@@ -394,13 +396,13 @@ public class RouteInfoManager {
     }
 
     public boolean isBrokerTopicConfigChanged(final String clusterName, final String brokerAddr,
-        final DataVersion dataVersion) {
+                                              final DataVersion dataVersion) {
         DataVersion prev = queryBrokerTopicConfig(clusterName, brokerAddr);
         return null == prev || !prev.equals(dataVersion);
     }
 
     public boolean isTopicConfigChanged(final String clusterName, final String brokerAddr,
-        final DataVersion dataVersion, String brokerName, String topic) {
+                                        final DataVersion dataVersion, String brokerName, String topic) {
         boolean isChange = isBrokerTopicConfigChanged(clusterName, brokerAddr, dataVersion);
         if (isChange) {
             return true;
@@ -527,83 +529,81 @@ public class RouteInfoManager {
 
     public void unRegisterBroker(Set<UnRegisterBrokerRequestHeader> unRegisterRequests) {
         try {
-            try {
-                Set<String> removedBroker = new HashSet<>();
-                Set<String> reducedBroker = new HashSet<>();
-                Map<String, BrokerStatusChangeInfo> needNotifyBrokerMap = new HashMap<>();
+            Set<String> removedBroker = new HashSet<>();
+            Set<String> reducedBroker = new HashSet<>();
+            Map<String, BrokerStatusChangeInfo> needNotifyBrokerMap = new HashMap<>();
 
-                this.lock.writeLock().lockInterruptibly();
-                for (final UnRegisterBrokerRequestHeader unRegisterRequest : unRegisterRequests) {
-                    final String brokerName = unRegisterRequest.getBrokerName();
-                    final String clusterName = unRegisterRequest.getClusterName();
-
-                    BrokerAddrInfo brokerAddrInfo = new BrokerAddrInfo(clusterName, unRegisterRequest.getBrokerAddr());
-
-                    BrokerLiveInfo brokerLiveInfo = this.brokerLiveTable.remove(brokerAddrInfo);
-                    log.info("unregisterBroker, remove from brokerLiveTable {}, {}",
-                        brokerLiveInfo != null ? "OK" : "Failed",
+            this.lock.writeLock().lockInterruptibly();
+            for (final UnRegisterBrokerRequestHeader unRegisterRequest : unRegisterRequests) {
+                final String brokerName = unRegisterRequest.getBrokerName();
+                final String clusterName = unRegisterRequest.getClusterName();
+
+                BrokerAddrInfo brokerAddrInfo = new BrokerAddrInfo(clusterName, unRegisterRequest.getBrokerAddr());
+
+                BrokerLiveInfo brokerLiveInfo = this.brokerLiveTable.remove(brokerAddrInfo);
+                log.info("unregisterBroker, remove from brokerLiveTable {}, {}",
+                    brokerLiveInfo != null ? "OK" : "Failed",
+                    brokerAddrInfo
+                );
+
+                this.filterServerTable.remove(brokerAddrInfo);
+
+                boolean removeBrokerName = false;
+                boolean isMinBrokerIdChanged = false;
+                BrokerData brokerData = this.brokerAddrTable.get(brokerName);
+                if (null != brokerData) {
+                    if (!brokerData.getBrokerAddrs().isEmpty() &&
+                        unRegisterRequest.getBrokerId().equals(Collections.min(brokerData.getBrokerAddrs().keySet()))) {
+                        isMinBrokerIdChanged = true;
+                    }
+                    String addr = brokerData.getBrokerAddrs().remove(unRegisterRequest.getBrokerId());
+                    log.info("unregisterBroker, remove addr from brokerAddrTable {}, {}",
+                        addr != null ? "OK" : "Failed",
                         brokerAddrInfo
                     );
-
-                    this.filterServerTable.remove(brokerAddrInfo);
-
-                    boolean removeBrokerName = false;
-                    boolean isMinBrokerIdChanged = false;
-                    BrokerData brokerData = this.brokerAddrTable.get(brokerName);
-                    if (null != brokerData) {
-                        if (!brokerData.getBrokerAddrs().isEmpty() &&
-                            unRegisterRequest.getBrokerId().equals(Collections.min(brokerData.getBrokerAddrs().keySet()))) {
-                            isMinBrokerIdChanged = true;
-                        }
-                        String addr = brokerData.getBrokerAddrs().remove(unRegisterRequest.getBrokerId());
-                        log.info("unregisterBroker, remove addr from brokerAddrTable {}, {}",
-                            addr != null ? "OK" : "Failed",
-                            brokerAddrInfo
+                    if (brokerData.getBrokerAddrs().isEmpty()) {
+                        this.brokerAddrTable.remove(brokerName);
+                        log.info("unregisterBroker, remove name from brokerAddrTable OK, {}",
+                            brokerName
                         );
-                        if (brokerData.getBrokerAddrs().isEmpty()) {
-                            this.brokerAddrTable.remove(brokerName);
-                            log.info("unregisterBroker, remove name from brokerAddrTable OK, {}",
-                                brokerName
-                            );
 
-                            removeBrokerName = true;
-                        } else if (isMinBrokerIdChanged) {
-                            needNotifyBrokerMap.put(brokerName, new BrokerStatusChangeInfo(
-                                brokerData.getBrokerAddrs(), addr, null));
-                        }
+                        removeBrokerName = true;
+                    } else if (isMinBrokerIdChanged) {
+                        needNotifyBrokerMap.put(brokerName, new BrokerStatusChangeInfo(
+                            brokerData.getBrokerAddrs(), addr, null));
                     }
+                }
 
-                    if (removeBrokerName) {
-                        Set<String> nameSet = this.clusterAddrTable.get(clusterName);
-                        if (nameSet != null) {
-                            boolean removed = nameSet.remove(brokerName);
-                            log.info("unregisterBroker, remove name from clusterAddrTable {}, {}",
-                                removed ? "OK" : "Failed",
-                                brokerName);
-
-                            if (nameSet.isEmpty()) {
-                                this.clusterAddrTable.remove(clusterName);
-                                log.info("unregisterBroker, remove cluster from clusterAddrTable {}",
-                                    clusterName
-                                );
-                            }
+                if (removeBrokerName) {
+                    Set<String> nameSet = this.clusterAddrTable.get(clusterName);
+                    if (nameSet != null) {
+                        boolean removed = nameSet.remove(brokerName);
+                        log.info("unregisterBroker, remove name from clusterAddrTable {}, {}",
+                            removed ? "OK" : "Failed",
+                            brokerName);
+
+                        if (nameSet.isEmpty()) {
+                            this.clusterAddrTable.remove(clusterName);
+                            log.info("unregisterBroker, remove cluster from clusterAddrTable {}",
+                                clusterName
+                            );
                         }
-                        removedBroker.add(brokerName);
-                    } else {
-                        reducedBroker.add(brokerName);
                     }
+                    removedBroker.add(brokerName);
+                } else {
+                    reducedBroker.add(brokerName);
                 }
+            }
 
-                cleanTopicByUnRegisterRequests(removedBroker, reducedBroker);
+            cleanTopicByUnRegisterRequests(removedBroker, reducedBroker);
 
-                if (!needNotifyBrokerMap.isEmpty() && namesrvConfig.isNotifyMinBrokerIdChanged()) {
-                    notifyMinBrokerIdChanged(needNotifyBrokerMap);
-                }
-            } finally {
-                this.lock.writeLock().unlock();
+            if (!needNotifyBrokerMap.isEmpty() && namesrvConfig.isNotifyMinBrokerIdChanged()) {
+                notifyMinBrokerIdChanged(needNotifyBrokerMap);
             }
         } catch (Exception e) {
             log.error("unregisterBroker Exception", e);
+        } finally {
+            this.lock.writeLock().unlock();
         }
     }
 
@@ -839,7 +839,7 @@ public class RouteInfoManager {
     }
 
     private boolean setupUnRegisterRequest(UnRegisterBrokerRequestHeader unRegisterRequest,
-        BrokerAddrInfo brokerAddrInfo) {
+                                           BrokerAddrInfo brokerAddrInfo) {
         unRegisterRequest.setClusterName(brokerAddrInfo.getClusterName());
         unRegisterRequest.setBrokerAddr(brokerAddrInfo.getBrokerAddr());
 
@@ -877,7 +877,7 @@ public class RouteInfoManager {
     }
 
     private void notifyMinBrokerIdChanged(Map<Long, String> brokerAddrMap, String offlineBrokerAddr,
-        String haBrokerAddr)
+                                          String haBrokerAddr)
         throws InterruptedException, RemotingSendRequestException, RemotingTimeoutException,
         RemotingTooMuchRequestException, RemotingConnectException {
         if (brokerAddrMap == null || brokerAddrMap.isEmpty() || this.namesrvController == null) {
@@ -1089,8 +1089,8 @@ class BrokerLiveInfo {
     private String haServerAddr;
 
     public BrokerLiveInfo(long lastUpdateTimestamp, long heartbeatTimeoutMillis, DataVersion dataVersion,
-        Channel channel,
-        String haServerAddr) {
+                          Channel channel,
+                          String haServerAddr) {
         this.lastUpdateTimestamp = lastUpdateTimestamp;
         this.heartbeatTimeoutMillis = heartbeatTimeoutMillis;
         this.dataVersion = dataVersion;