From 5b000a389c92778c92931cae3bbe604ea797e0b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=94=B0=E5=B0=8F=E6=B3=A2?= Date: Fri, 7 Dec 2018 16:48:57 +0800 Subject: [PATCH] Simplify the code logic of the method AbstractClusterInvoker#reselect. (#2826) * Simplify the code logic of the method AbstractClusterInvoker#reselect. * Minor modification for code style. --- .../support/AbstractClusterInvoker.java | 66 +++++++++---------- .../support/FailbackClusterInvoker.java | 6 +- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/AbstractClusterInvoker.java b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/AbstractClusterInvoker.java index e8b80595e7d..1c24aef1ac7 100644 --- a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/AbstractClusterInvoker.java +++ b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/AbstractClusterInvoker.java @@ -24,6 +24,7 @@ import org.apache.dubbo.common.logger.LoggerFactory; import org.apache.dubbo.common.utils.CollectionUtils; import org.apache.dubbo.common.utils.NetUtils; +import org.apache.dubbo.common.utils.StringUtils; import org.apache.dubbo.rpc.Invocation; import org.apache.dubbo.rpc.Invoker; import org.apache.dubbo.rpc.Result; @@ -44,8 +45,8 @@ */ public abstract class AbstractClusterInvoker implements Invoker { - private static final Logger logger = LoggerFactory - .getLogger(AbstractClusterInvoker.class); + private static final Logger logger = LoggerFactory.getLogger(AbstractClusterInvoker.class); + protected final Directory directory; protected final boolean availablecheck; @@ -110,13 +111,16 @@ public void destroy() { * @return the invoker which will final to do invoke. * @throws RpcException */ - protected Invoker select(LoadBalance loadbalance, Invocation invocation, List> invokers, List> selected) throws RpcException { - if (invokers == null || invokers.isEmpty()) { + protected Invoker select(LoadBalance loadbalance, Invocation invocation, + List> invokers, List> selected) throws RpcException { + + if (CollectionUtils.isEmpty(invokers)) { return null; } - String methodName = invocation == null ? "" : invocation.getMethodName(); + String methodName = invocation == null ? StringUtils.EMPTY : invocation.getMethodName(); - boolean sticky = invokers.get(0).getUrl().getMethodParameter(methodName, Constants.CLUSTER_STICKY_KEY, Constants.DEFAULT_CLUSTER_STICKY); + boolean sticky = invokers.get(0).getUrl() + .getMethodParameter(methodName, Constants.CLUSTER_STICKY_KEY, Constants.DEFAULT_CLUSTER_STICKY); { //ignore overloaded method if (stickyInvoker != null && !invokers.contains(stickyInvoker)) { @@ -137,8 +141,10 @@ protected Invoker select(LoadBalance loadbalance, Invocation invocation, List return invoker; } - private Invoker doSelect(LoadBalance loadbalance, Invocation invocation, List> invokers, List> selected) throws RpcException { - if (invokers == null || invokers.isEmpty()) { + private Invoker doSelect(LoadBalance loadbalance, Invocation invocation, + List> invokers, List> selected) throws RpcException { + + if (CollectionUtils.isEmpty(invokers)) { return null; } if (invokers.size() == 1) { @@ -158,7 +164,7 @@ private Invoker doSelect(LoadBalance loadbalance, Invocation invocation, List int index = invokers.indexOf(invoker); try { //Avoid collision - invoker = index < invokers.size() - 1 ? invokers.get(index + 1) : invokers.get(0); + invoker = invokers.get((index + 1) % invokers.size()); } catch (Exception e) { logger.warn(e.getMessage() + " may because invokers list dynamic change, ignore.", e); } @@ -171,7 +177,8 @@ private Invoker doSelect(LoadBalance loadbalance, Invocation invocation, List } /** - * Reselect, use invokers not in `selected` first, if all invokers are in `selected`, just pick an available one using loadbalance policy. + * Reselect, use invokers not in `selected` first, if all invokers are in `selected`, + * just pick an available one using loadbalance policy. * * @param loadbalance * @param invocation @@ -181,34 +188,27 @@ private Invoker doSelect(LoadBalance loadbalance, Invocation invocation, List * @throws RpcException */ private Invoker reselect(LoadBalance loadbalance, Invocation invocation, - List> invokers, List> selected, boolean availablecheck) - throws RpcException { + List> invokers, List> selected, boolean availablecheck) throws RpcException { //Allocating one in advance, this list is certain to be used. - List> reselectInvokers = new ArrayList>(invokers.size() > 1 ? (invokers.size() - 1) : invokers.size()); + List> reselectInvokers = new ArrayList<>( + invokers.size() > 1 ? (invokers.size() - 1) : invokers.size()); - //First, try picking a invoker not in `selected`. - if (availablecheck) { // invoker.isAvailable() should be checked - for (Invoker invoker : invokers) { - if (invoker.isAvailable()) { - if (selected == null || !selected.contains(invoker)) { - reselectInvokers.add(invoker); - } - } - } - if (!reselectInvokers.isEmpty()) { - return loadbalance.select(reselectInvokers, getUrl(), invocation); + // First, try picking a invoker not in `selected`. + for (Invoker invoker : invokers) { + if (availablecheck && !invoker.isAvailable()) { + continue; } - } else { // do not check invoker.isAvailable() - for (Invoker invoker : invokers) { - if (selected == null || !selected.contains(invoker)) { - reselectInvokers.add(invoker); - } - } - if (!reselectInvokers.isEmpty()) { - return loadbalance.select(reselectInvokers, getUrl(), invocation); + + if (selected == null || !selected.contains(invoker)) { + reselectInvokers.add(invoker); } } + + if (!reselectInvokers.isEmpty()) { + return loadbalance.select(reselectInvokers, getUrl(), invocation); + } + // Just pick an available invoker using loadbalance policy { if (selected != null) { @@ -257,7 +257,7 @@ public String toString() { } protected void checkInvokers(List> invokers, Invocation invocation) { - if (invokers == null || invokers.isEmpty()) { + if (CollectionUtils.isEmpty(invokers)) { throw new RpcException("Failed to invoke the method " + invocation.getMethodName() + " in the service " + getInterface().getName() + ". No provider available for the service " + directory.getUrl().getServiceKey() diff --git a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailbackClusterInvoker.java b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailbackClusterInvoker.java index 64d4b24f9b0..b8ae095af6b 100644 --- a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailbackClusterInvoker.java +++ b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/FailbackClusterInvoker.java @@ -65,7 +65,7 @@ public FailbackClusterInvoker(Directory directory) { super(directory); } - private void addFailed(Invocation invocation, AbstractClusterInvoker router) { + private void addFailed(Invocation invocation, AbstractClusterInvoker invoker) { if (retryFuture == null) { synchronized (this) { if (retryFuture == null) { @@ -84,11 +84,11 @@ public void run() { } } } - failed.put(invocation, router); + failed.put(invocation, invoker); } void retryFailed() { - if (failed.size() == 0) { + if (failed.isEmpty()) { return; } for (Map.Entry> entry : new HashMap<>(failed).entrySet()) {