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