[GitHub] [dubbo] BurningCN commented on a change in pull request #8532: Fix bug of BroadcastClusterInvoker and add test cases

[GitBox <gi...@apache.org>]

BurningCN commented on a change in pull request #8532:
URL: https://github.com/apache/dubbo/pull/8532#discussion_r690862475



##########
File path: dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/BroadcastClusterInvoker.java
##########
@@ -72,20 +72,18 @@ public Result doInvoke(final Invocation invocation, List<Invoker<T>> invokers, L
                     if (null != resultException) {
                         exception = getRpcException(result.getException());
                         logger.warn(exception.getMessage(), exception);
+                        failIndex++;

Review comment:
       
   The accumulation of failIndex should be placed above the judgment of failIndex. Suppose 4 invokers (their invoke methods will throw exceptions), and the failure percentage is configured to 50/100. It is reasonable to throw exceptions after 2 invokers fail. If it is the original code method, it will throw exception after 3 invokers fail.
   
   failIndex的累加应该放到对failIndex判断的上面。假设4个invoker（他们的invoke方法都会抛异常），配置了失败百分比50/100，按道理是2个invoker失败后上抛异常，如果是原有代码方式的话，需要3个invoker调用失败才会抛异常




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org