You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by "yockie (via GitHub)" <gi...@apache.org> on 2023/07/25 13:41:53 UTC
[PR] response返回之后，request/response析构之前执行一些自定义逻辑 (brpc)
yockie opened a new pull request, #2328:
URL: https://github.com/apache/brpc/pull/2328
### What problem does this PR solve?
1.我们的场景希望将响应时间减少到最小，所以希望将一些类似数据上报的耗时操作（是的，特别耗时）放在response返回之后、request/response析构之前做
2.本pr借鉴了 https://github.com/apache/brpc/pull/1433，看这个pr一直没合入且测试发现有些问题&不太满足我们的需要，所以另提一个，https://github.com/apache/brpc/pull/1433 的问题主要是：
（1）太多无关topic的改动
（2）仅在SendRpcResponse成功时才调用call_after_rpc_resp函数，失败时没有调用（我们需要无论成功或失败都要调用）
（3）定义的after_resp_fn_t不包含controller指针，我们需要拿到controller对象（进而拿到session data指针，所以声明为非const指针）
3.同样在目前在常用的 baidu_std/http/http2 协议上支持。其他协议未开发。
4.将Controller与req、res的析构顺序进行了修改，依赖关系应该是controller依赖req、res，所以应该先析构controller，后析构req、res
麻烦maintainers看看有什么不妥之处~
--
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-unsubscribe@brpc.apache.org
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
Re: [PR] send response之后，request/response对象析构之前执行一些自定义逻辑 (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on code in PR #2328:
URL: https://github.com/apache/brpc/pull/2328#discussion_r1281456450
##########
src/brpc/controller.h:
##########
@@ -567,6 +568,14 @@ friend void policy::ProcessThriftRequest(InputMessageBase*);
// -1 means no deadline.
int64_t deadline_us() const { return _deadline_us; }
+ using AfterRpcRespFnType = std::function<void(Controller* cntl,
+ const google::protobuf::Message* req,
+ const google::protobuf::Message* res)>;
+
+ void SetAfterRpcRespFn(AfterRpcRespFnType&& fn) { _after_rpc_resp_fn = fn; }
Review Comment:
简单getter/setter，使用小写+下划线命名风格
##########
src/brpc/controller.h:
##########
@@ -567,6 +568,14 @@ friend void policy::ProcessThriftRequest(InputMessageBase*);
// -1 means no deadline.
int64_t deadline_us() const { return _deadline_us; }
+ using AfterRpcRespFnType = std::function<void(Controller* cntl,
+ const google::protobuf::Message* req,
+ const google::protobuf::Message* res)>;
+
+ void SetAfterRpcRespFn(AfterRpcRespFnType&& fn) { _after_rpc_resp_fn = fn; }
Review Comment:
简单getter/setter，使用小写+下划线命名风格
--
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-unsubscribe@brpc.apache.org
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
Re: [PR] send response之后，request/response对象析构之前执行一些自定义逻辑 (brpc)
Posted by "yockie (via GitHub)" <gi...@apache.org>.
yockie commented on PR #2328:
URL: https://github.com/apache/brpc/pull/2328#issuecomment-1654907725
> 另外，补充一下单测吧
[ RUN ] SocketTest.health_check
I0728 02:41:49.501999 13225 src/brpc/socket.cpp:2465] Checking Socket{id=60129542144 addr=0.0.0.0:7878} (0x2278000)
I0728 02:41:49.502532 13227 src/brpc/socket.cpp:2525] Revived Socket{id=60129542144 addr=0.0.0.0:7878} (0x2278000) (Connectable)
brpc_socket_unittest.cpp:756: Failure
Expected: (butil::gettimeofday_us()) < (start_time + 1200000L), actual: 1690512110703509 vs 1690512110702475
[ FAILED ] SocketTest.health_check (1302 ms)
大佬 这个ut没通过 跟我的改动没啥关系吧
--
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-unsubscribe@brpc.apache.org
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
Re: [PR] send response之后，request/response对象析构之前执行一些自定义逻辑 (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on PR #2328:
URL: https://github.com/apache/brpc/pull/2328#issuecomment-1680102776
LGTM
--
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-unsubscribe@brpc.apache.org
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
Re: [PR] send response之后，request/response对象析构之前执行一些自定义逻辑 (brpc)
Posted by "yockie (via GitHub)" <gi...@apache.org>.
yockie commented on PR #2328:
URL: https://github.com/apache/brpc/pull/2328#issuecomment-1653754496
> 另外，补充一下单测吧
之前单测写的不对，已经修改了，本地跑通了
--
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-unsubscribe@brpc.apache.org
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
Re: [PR] send response之后，request/response对象析构之前执行一些自定义逻辑 (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on code in PR #2328:
URL: https://github.com/apache/brpc/pull/2328#discussion_r1274307736
##########
src/brpc/controller.h:
##########
@@ -567,6 +568,14 @@ friend void policy::ProcessThriftRequest(InputMessageBase*);
// -1 means no deadline.
int64_t deadline_us() const { return _deadline_us; }
+ using after_rpc_resp_fn_t = std::function<void(Controller* cntl,
+ const google::protobuf::Message* req,
+ const google::protobuf::Message* res)>;
+
+ void set_after_rpc_resp_fn(after_rpc_resp_fn_t&& fn) { _after_rpc_resp_fn = fn; }
+
+ void call_after_rpc_resp(const google::protobuf::Message* req, const google::protobuf::Message* res);
Review Comment:
函数命名，非简单getter/setter函数，使用大写开关驼峰命名
##########
src/brpc/controller.h:
##########
@@ -567,6 +568,14 @@ friend void policy::ProcessThriftRequest(InputMessageBase*);
// -1 means no deadline.
int64_t deadline_us() const { return _deadline_us; }
+ using after_rpc_resp_fn_t = std::function<void(Controller* cntl,
+ const google::protobuf::Message* req,
+ const google::protobuf::Message* res)>;
+
+ void set_after_rpc_resp_fn(after_rpc_resp_fn_t&& fn) { _after_rpc_resp_fn = fn; }
+
+ void call_after_rpc_resp(const google::protobuf::Message* req, const google::protobuf::Message* res);
Review Comment:
函数命名，非简单getter/setter函数，使用大写开关驼峰命名
--
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-unsubscribe@brpc.apache.org
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
Re: [PR] send response之后，request/response对象析构之前执行一些自定义逻辑 (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm merged PR #2328:
URL: https://github.com/apache/brpc/pull/2328
--
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-unsubscribe@brpc.apache.org
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
Re: [PR] send response之后，request/response对象析构之前执行一些自定义逻辑 (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on PR #2328:
URL: https://github.com/apache/brpc/pull/2328#issuecomment-1650891350
另外，补充一下单测吧
--
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-unsubscribe@brpc.apache.org
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
Re: [PR] send response之后，request/response对象析构之前执行一些自定义逻辑 (brpc)
Posted by "yockie (via GitHub)" <gi...@apache.org>.
yockie commented on PR #2328:
URL: https://github.com/apache/brpc/pull/2328#issuecomment-1651348298
> 另外，补充一下单测吧
done
--
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-unsubscribe@brpc.apache.org
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