chenBright commented on code in PR #2842: URL: https://github.com/apache/brpc/pull/2842#discussion_r1910305644
########## 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: 跟原来的_current_fail一样,达成就取消其他call。应该是不好判断哪些是剩下的,所以得全都取消一遍吧。 ########## src/brpc/parallel_channel.cpp: ########## @@ -655,9 +665,21 @@ void ParallelChannel::CallMethod( fail_limit = ndone; } } - - d = ParallelChannelDone::Create(fail_limit, ndone, aps, nchan, - cntl, done); + + // `success_limit' is only valid when `fail_limit' is not set. Review Comment: `success_limit = ndone;`要求全部成功,相当于不起作用。 ########## 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 && Review Comment: error_code为EPCHANFINISH,是达成成功数后取消的call,不算在_current_fail和_current_success上。 -- 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