MergeableClusterInvoker处理超时会抛出异常而不是忽略超时结果

[feelwing1314]

源码部分：

for (Map.Entry<String, Future<Result>> entry : results.entrySet()) {
    Future<Result> future = entry.getValue();
    try {
        Result r = future.get(timeout, TimeUnit.MILLISECONDS);
        if (r.hasException()) {
            log.error("Invoke " + getGroupDescFromServiceKey(entry.getKey()) + 
                            " failed: " + r.getException().getMessage(), 
                    r.getException());
        } else {
            resultList.add(r);
        }
    } catch (Exception e) {
        throw new RpcException("Failed to invoke service " + entry.getKey() + ": " + e.getMessage(), e);
    }
}

future.get(timeout, TimeUnit.MILLISECONDS);如果执行超时会抛出TimeoutException, 那么逻辑会走到throw new RpcException(.....)，按照合并特性，这里应该是忽略部分Provider的异常；

修改建议：
将throw new RpcException(... ...);改为日志输出，并增加continue；如下所示：

for (Map.Entry<String, Future<Result>> entry : results.entrySet()) {
    Future<Result> future = entry.getValue();
    try {
        Result r = future.get(timeout, TimeUnit.MILLISECONDS);
        if (r.hasException()) {
            log.error("Invoke " + getGroupDescFromServiceKey(entry.getKey()) + 
                            " failed: " + r.getException().getMessage(), 
                    r.getException());
        } else {
            resultList.add(r);
        }
    } catch (Exception e) {
        log.error("Failed to invoke service " + entry.getKey() + ": " + e.getMessage(), e);
        continue;
    }
}

[beiwei30]

@feelwing1314 interested in firing a pull request to fix it?

[feelwing1314]

@feelwing1314 interested in firing a pull request to fix it?

of course! I'll commit a pull request ASAP !

[lixiaojiee]

I don't think so. If I understand correctly, the grouping aggregation function is to combine the results of different groups (different implementations) according to certain rules[1]. Is it a problem if the execution results of a group are ignored? I think it's a reasonable implementation to throw an exception to make the user aware of the service problem as soon as possible.
[1]:http://dubbo.apache.org/en-us/docs/user/demos/group-merger.html

[lexburner]

I am confused that r.hasException() will be ignored while other exceptions: InterruptedException, ExecutionException, TimeoutException will throw new RpcException.
In my opinion, there are three behaviors:

1. stay the same as before
2. r.hasException() and other exception(InterruptedException, ExecutionException, TimeoutException) all should throw an exception
3. r.hasException() and other exception(InterruptedException, ExecutionException, TimeoutException) all should ignore.

They are all generally ok. This feature is not often be used, take it easy.