You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by GitBox <gi...@apache.org> on 2020/07/13 14:42:38 UTC
[GitHub] [incubator-brpc] heyuyi0906 opened a new pull request #1161: fix h2_req check failed when retry after ELIMIT error
heyuyi0906 opened a new pull request #1161:
URL: https://github.com/apache/incubator-brpc/pull/1161
#1114
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] heyuyi0906 commented on a change in pull request #1161: fix h2_req check failed when retry after ELIMIT error
Posted by GitBox <gi...@apache.org>.
heyuyi0906 commented on a change in pull request #1161:
URL: https://github.com/apache/incubator-brpc/pull/1161#discussion_r454411025
##########
File path: src/brpc/channel.cpp
##########
@@ -464,6 +464,8 @@ void Channel::CallMethod(const google::protobuf::MethodDescriptor* method,
// Share the lb with controller.
cntl->_lb = _lb;
+ // serialize_request must be done before pack_request
+ _serialize_request(&cntl->_request_buf, cntl, request);
Review comment:
已经改进了
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] jamesge commented on a change in pull request #1161: fix h2_req check failed when retry after ELIMIT error
Posted by GitBox <gi...@apache.org>.
jamesge commented on a change in pull request #1161:
URL: https://github.com/apache/incubator-brpc/pull/1161#discussion_r454811965
##########
File path: src/brpc/channel.cpp
##########
@@ -476,7 +480,6 @@ void Channel::CallMethod(const google::protobuf::MethodDescriptor* method,
// parameters in `cntl' are set.
Review comment:
这两个分支判断重复了，需要保留一个且移动到_serialize_request后。
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] heyuyi0906 commented on a change in pull request #1161: fix h2_req check failed when retry after ELIMIT error
Posted by GitBox <gi...@apache.org>.
heyuyi0906 commented on a change in pull request #1161:
URL: https://github.com/apache/incubator-brpc/pull/1161#discussion_r454411025
##########
File path: src/brpc/channel.cpp
##########
@@ -464,6 +464,8 @@ void Channel::CallMethod(const google::protobuf::MethodDescriptor* method,
// Share the lb with controller.
cntl->_lb = _lb;
+ // serialize_request must be done before pack_request
+ _serialize_request(&cntl->_request_buf, cntl, request);
Review comment:
已经改进了下
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] jamesge merged pull request #1161: fix h2_req check failed when retry after ELIMIT error
Posted by GitBox <gi...@apache.org>.
jamesge merged pull request #1161:
URL: https://github.com/apache/incubator-brpc/pull/1161
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] jamesge commented on pull request #1161: fix h2_req check failed when retry after ELIMIT error
Posted by GitBox <gi...@apache.org>.
jamesge commented on pull request #1161:
URL: https://github.com/apache/incubator-brpc/pull/1161#issuecomment-661956917
Thanks for the fix 👍
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] heyuyi0906 commented on a change in pull request #1161: fix h2_req check failed when retry after ELIMIT error
Posted by GitBox <gi...@apache.org>.
heyuyi0906 commented on a change in pull request #1161:
URL: https://github.com/apache/incubator-brpc/pull/1161#discussion_r454960528
##########
File path: src/brpc/channel.cpp
##########
@@ -476,7 +480,6 @@ void Channel::CallMethod(const google::protobuf::MethodDescriptor* method,
// parameters in `cntl' are set.
Review comment:
确实，之前修改的时候疏忽了，已修改
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
[GitHub] [incubator-brpc] zyearn commented on a change in pull request #1161: fix h2_req check failed when retry after ELIMIT error
Posted by GitBox <gi...@apache.org>.
zyearn commented on a change in pull request #1161:
URL: https://github.com/apache/incubator-brpc/pull/1161#discussion_r454211947
##########
File path: src/brpc/channel.cpp
##########
@@ -464,6 +464,8 @@ void Channel::CallMethod(const google::protobuf::MethodDescriptor* method,
// Share the lb with controller.
cntl->_lb = _lb;
+ // serialize_request must be done before pack_request
+ _serialize_request(&cntl->_request_buf, cntl, request);
Review comment:
我觉得这个注释可以优化一下，“serialize_request must be done before pack_request”是对代码本身的复述，而这里最好写明意图，即为什么要把这行放这里，为什么必须要在HandleSendFailed之前，否则过段时间，维护者还是有可能在这之前调用HandleSendFailed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org