yanglimingcn commented on code in PR #2842:
URL: https://github.com/apache/brpc/pull/2842#discussion_r1909963919
##########
src/brpc/parallel_channel.cpp:
##########
@@ -220,14 +216,25 @@ class ParallelChannelDone : public
google::protobuf::Closure {
if (fin != NULL) {
// [ called from SubDone::Run() ]
- // Count failed sub calls, if fail_limit is reached, cancel others.
- if (fin->cntl.FailedInline() &&
- _current_fail.fetch_add(1, butil::memory_order_relaxed) + 1
- == _fail_limit) {
+ int error_code = fin->cntl.ErrorCode();
+ // EPCHANFINISH is not an error of sub calls.
+ bool fail = 0 != error_code && EPCHANFINISH != error_code;
+ bool cancel =
+ // Count failed sub calls, if `fail_limit' is reached, cancel
others.
+ (fail && _current_fail.fetch_add(1,
butil::memory_order_relaxed) + 1
+ == _fail_limit) ||
+ // Count successful sub calls, if `success_limit' is reached,
cancel others.
+ (0 == error_code &&
+ _current_success.fetch_add(1, butil::memory_order_relaxed) + 1
+ == _success_limit);
+
+ if (cancel) {
+ // Only cancel once by `fail_limit' or `success_limit'.
for (int i = 0; i < _ndone; ++i) {
Review Comment:
这部分逻辑，为啥fin != sd就把其它的subcall给cancel掉呢？
那_current_succes这个分支来说fetch_add(1, butil::memory_order_relaxed) +
1意思是_current_success已经达成了，所以可以把所有的call都取消掉了，应该是把剩下的call取消掉？如果这个取消只能执行一次感觉也没问题。
但是这块是否有竞态条件呢？因为这里是先做原子增加操作，后做取消操作。
--
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: dev-unsubscr...@brpc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org
