Do not instantiate load balance if there is no invokers (#1297)

[liketic]

Currently, we'll instantiate the load balance with a default instance if no invokers found. However, we don't need to use it.

Closes #1297

[codecov-io]

Codecov Report

Merging #1376 into master will decrease coverage by 0.17%.
The diff coverage is 40%.

@@            Coverage Diff             @@
##           master    #1376      +/-   ##
==========================================
- Coverage   32.58%   32.41%   -0.18%     
==========================================
  Files         691      691              
  Lines       34537    34539       +2     
  Branches     6812     6813       +1     
==========================================
- Hits        11255    11196      -59     
- Misses      21356    21410      +54     
- Partials     1926     1933       +7

Impacted Files	Coverage Δ
...bo/rpc/cluster/support/AbstractClusterInvoker.java	67.3% <40%> (-1.32%)	⬇️
...bo/rpc/cluster/support/FailbackClusterInvoker.java	71.79% <0%> (-12.83%)	⬇️
...ba/dubbo/remoting/transport/netty/NettyClient.java	72.58% <0%> (-11.3%)	⬇️
...mon/serialize/support/dubbo/GenericDataOutput.java	67.71% <0%> (-7.72%)	⬇️
...om/alibaba/dubbo/rpc/filter/ActiveLimitFilter.java	83.33% <0%> (-5.56%)	⬇️
...a/dubbo/remoting/transport/netty/NettyChannel.java	61.25% <0%> (-5%)	⬇️
...mmon/serialize/support/dubbo/GenericDataInput.java	59.45% <0%> (-4.25%)	⬇️
...rpc/protocol/dubbo/telnet/InvokeTelnetHandler.java	51.89% <0%> (-3.8%)	⬇️
.../com/alibaba/dubbo/monitor/dubbo/DubboMonitor.java	73.14% <0%> (-2.78%)	⬇️
...main/java/com/alibaba/dubbo/rpc/RpcInvocation.java	15.11% <0%> (-2.33%)	⬇️
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c20d99...758b551. Read the comment docs.

[chickenlj]

How about remove this else statement and move loadbalance = ExtensionLoader.getExtensionLoader(LoadBalance.class).getExtension(Constants.DEFAULT_LOADBALANCE); to doSelect()?

[liketic]

You mean something like:

    private Invoker<T> doselect(LoadBalance loadbalance, Invocation invocation, List<Invoker<T>> invokers, List<Invoker<T>> selected) throws RpcException {
        if (invokers == null || invokers.isEmpty())
            return null;
        if (invokers.size() == 1)
            return invokers.get(0);
        // If we only have two invokers, use round-robin instead.
        if (invokers.size() == 2 && selected != null && !selected.isEmpty()) {
            return selected.get(0) == invokers.get(0) ? invokers.get(1) : invokers.get(0);
        }
        if (loadbalance == null) {
            loadbalance = ExtensionLoader.getExtensionLoader(LoadBalance.class).getExtension(Constants.DEFAULT_LOADBALANCE);
        }
        Invoker<T> invoker = loadbalance.select(invokers, getUrl(), invocation);

        //If the `invoker` is in the  `selected` or invoker is unavailable && availablecheck is true, reselect.
        if ((selected != null && selected.contains(invoker))
                || (!invoker.isAvailable() && getUrl() != null && availablecheck)) {
            try {
                Invoker<T> rinvoker = reselect(loadbalance, invocation, invokers, selected, availablecheck);
                if (rinvoker != null) {
                    invoker = rinvoker;
                } else {
                    //Check the index of current selected invoker, if it's not the last one, choose the one at index+1.
                    int index = invokers.indexOf(invoker);
                    try {
                        //Avoid collision
                        invoker = index < invokers.size() - 1 ? invokers.get(index + 1) : invoker;
                    } catch (Exception e) {
                        logger.warn(e.getMessage() + " may because invokers list dynamic change, ignore.", e);
                    }
                }
            } catch (Throwable t) {
                logger.error("cluster reselect fail reason is :" + t.getMessage() + " if can not solve, you can set cluster.availablecheck=false in url", t);
            }
        }
        return invoker;
    }

    public Result invoke(final Invocation invocation) throws RpcException {

        checkWhetherDestroyed();

        LoadBalance loadbalance = null;
        List<Invoker<T>> invokers = list(invocation);
        if (invokers != null && !invokers.isEmpty()) {
            loadbalance = ExtensionLoader.getExtensionLoader(LoadBalance.class).getExtension(invokers.get(0).getUrl()
                    .getMethodParameter(invocation.getMethodName(), Constants.LOADBALANCE_KEY, Constants.DEFAULT_LOADBALANCE));
        }
        RpcUtils.attachInvocationIdIfAsync(getUrl(), invocation);
        return doInvoke(invocation, invokers, loadbalance);
    }

If that, I'm not sure why we need to expose loadbalance to doInvoke and select, unless we allow subclass to pass a custom loadbalance to select. Thank you so much!

[chickenlj]

checkInvokers(copyinvokers, invocation); should always be called every time before we actually invoke.

But what if developers extending AbstractClusterInvoker don't do this check? The loadbalance will be null.

[liketic]

Thanks! I pushed 758b551