chenBright commented on code in PR #2734:
URL: https://github.com/apache/brpc/pull/2734#discussion_r1712626585


##########
src/brpc/channel.cpp:
##########
@@ -495,8 +496,10 @@ void Channel::CallMethod(const 
google::protobuf::MethodDescriptor* method,
     // overriding connect_timeout_ms does not make sense, just use the
     // one in ChannelOptions
     cntl->_connect_timeout_ms = _options.connect_timeout_ms;
-    if (cntl->backup_request_ms() == UNSET_MAGIC_NUM) {
+    if (cntl->backup_request_ms() == UNSET_MAGIC_NUM &&
+        NULL != cntl->_backup_request_policy) {
         cntl->set_backup_request_ms(_options.backup_request_ms);
+        cntl->_backup_request_policy = _options.backup_request_policy;
     }

Review Comment:
   写错了,应该是`NULL == 
cntl->_backup_request_policy`。只要Controller设置了其中一项,就不会使用ChannelOptions的配置了。
   
   > 要不去掉policy里的GetBackupRequestMs接口吧
   
   
还是需要保留的,用户使用BackupRequestPolicy,就统一在BackupRequestPolicy里实现所有策略,只要设置好backup_request_policy就行了,而不是还得额外设置一个backup_request_ms。
   
   将这两者看做两个配置,应该就比较自然了。
   优先级应该也挺清晰的:
   1. backup_request_policy > backup_request_ms;
   2. Controller > ChannelOptions。



-- 
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

Reply via email to