You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by "chenBright (via GitHub)" <gi...@apache.org> on 2023/02/26 09:48:17 UTC
[PR] Support user interceptor of server (brpc)
chenBright opened a new pull request, #2137:
URL: https://github.com/apache/brpc/pull/2137
### What problem does this PR solve?
Issue Number: #2134
Problem Summary:
支持自定义服务端拒绝（拦截器）
### What is changed and the side effects?
Changed:
Side effects:
- Performance effects(性能影响):
- Breaking backward compatibility(向后兼容性):
---
### Check List:
- Please make sure your changes are compilable(请确保你的更改可以通过编译).
- When providing us with a new feature, it is best to add related tests(如果你向我们增加一个新的功能, 请添加相关测试).
- Please follow [Contributor Covenant Code of Conduct](../../master/CODE_OF_CONDUCT.md).(请遵循贡献者准则).
--
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] Support user interceptor of server (brpc)
Posted by "yanglimingcn (via GitHub)" <gi...@apache.org>.
yanglimingcn commented on PR #2137:
URL: https://github.com/apache/brpc/pull/2137#issuecomment-1564968843
这个能支持按照不同请求类型，或者根据请求里面的一些信息做拦截策略吗？如果支持，可能让用户基于此可以用来定义一个qos算法。
--
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] Support user interceptor of server (brpc)
Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright commented on code in PR #2137:
URL: https://github.com/apache/brpc/pull/2137#discussion_r1131189024
##########
src/brpc/server.cpp:
##########
@@ -2197,6 +2197,21 @@ int Server::MaxConcurrencyOf(google::protobuf::Service* service,
return MaxConcurrencyOf(service->GetDescriptor()->full_name(), method_name);
}
+bool Server::AcceptRequest(Controller* cntl) const {
+ const Interceptor* interceptor = _options.interceptor;
Review Comment:
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
Re: [PR] Support user interceptor of server (brpc)
Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright commented on code in PR #2137:
URL: https://github.com/apache/brpc/pull/2137#discussion_r1118955320
##########
src/brpc/server.cpp:
##########
@@ -451,6 +453,10 @@ Server::~Server() {
delete _options.auth;
_options.auth = NULL;
}
+ if (_options.interceptor) {
Review Comment:
写错了，已修改为server_owns_interceptor
--
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] Support user interceptor of server (brpc)
Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright commented on code in PR #2137:
URL: https://github.com/apache/brpc/pull/2137#discussion_r1118955761
##########
src/brpc/policy/baidu_rpc_protocol.cpp:
##########
@@ -309,6 +309,22 @@ void EndRunningCallMethodInPool(
return EndRunningUserCodeInPool(CallMethodInBackupThread, args);
};
+// Returns true if accept request, reject request otherwise.
+bool AcceptRequest(const Server* server, Controller* cntl) {
Review Comment:
已实现为server的成员函数
--
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] Support user interceptor of server (brpc)
Posted by "serverglen (via GitHub)" <gi...@apache.org>.
serverglen commented on PR #2137:
URL: https://github.com/apache/brpc/pull/2137#issuecomment-1495325800
LGTM
I think this PR can be merged
--
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] Support user interceptor of server (brpc)
Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright commented on PR #2137:
URL: https://github.com/apache/brpc/pull/2137#issuecomment-1521238450
> LGTM I think this PR can be merged
+1 @wwbmmm
--
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] Support user interceptor of server (brpc)
Posted by "thorneliu (via GitHub)" <gi...@apache.org>.
thorneliu commented on code in PR #2137:
URL: https://github.com/apache/brpc/pull/2137#discussion_r1126468512
##########
src/brpc/server.cpp:
##########
@@ -2191,6 +2197,21 @@ int Server::MaxConcurrencyOf(google::protobuf::Service* service,
return MaxConcurrencyOf(service->GetDescriptor()->full_name(), method_name);
}
+bool Server::AcceptRequest(Controller* cntl) const {
+ const Interceptor* interceptor = _options.interceptor;
+ int error_code = 0;
+ std::string error_text;
+ if (cntl && interceptor &&
+ !interceptor->Accept(cntl, error_code, error_text)) {
+ cntl->SetFailed(error_code,
Review Comment:
And the Erron, should it always be predefined Enum EREJECT?
--
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] Support user interceptor of server (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm merged PR #2137:
URL: https://github.com/apache/brpc/pull/2137
--
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] Support user interceptor of server (brpc)
Posted by "yanglimingcn (via GitHub)" <gi...@apache.org>.
yanglimingcn commented on PR #2137:
URL: https://github.com/apache/brpc/pull/2137#issuecomment-1564971642
也许controller里面加上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.
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] Support user interceptor of server (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on PR #2137:
URL: https://github.com/apache/brpc/pull/2137#issuecomment-1447440317
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] Support user interceptor of server (brpc)
Posted by "wasphin (via GitHub)" <gi...@apache.org>.
wasphin commented on code in PR #2137:
URL: https://github.com/apache/brpc/pull/2137#discussion_r1118117894
##########
src/brpc/server.cpp:
##########
@@ -451,6 +453,10 @@ Server::~Server() {
delete _options.auth;
_options.auth = NULL;
}
+ if (_options.interceptor) {
Review Comment:
`server_owns_interceptor`?
##########
src/brpc/policy/baidu_rpc_protocol.cpp:
##########
@@ -309,6 +309,22 @@ void EndRunningCallMethodInPool(
return EndRunningUserCodeInPool(CallMethodInBackupThread, args);
};
+// Returns true if accept request, reject request otherwise.
+bool AcceptRequest(const Server* server, Controller* cntl) {
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.
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] Support user interceptor of server (brpc)
Posted by "serverglen (via GitHub)" <gi...@apache.org>.
serverglen commented on PR #2137:
URL: https://github.com/apache/brpc/pull/2137#issuecomment-1451162208
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] Support user interceptor of server (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on code in PR #2137:
URL: https://github.com/apache/brpc/pull/2137#discussion_r1118209742
##########
src/brpc/policy/baidu_rpc_protocol.cpp:
##########
@@ -309,6 +309,22 @@ void EndRunningCallMethodInPool(
return EndRunningUserCodeInPool(CallMethodInBackupThread, args);
};
+// Returns true if accept request, reject request otherwise.
+bool AcceptRequest(const Server* server, Controller* cntl) {
Review Comment:
这个是不是可以实现为server的成员函数
--
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] Support user interceptor of server (brpc)
Posted by "thorneliu (via GitHub)" <gi...@apache.org>.
thorneliu commented on code in PR #2137:
URL: https://github.com/apache/brpc/pull/2137#discussion_r1131848337
##########
src/brpc/server.cpp:
##########
@@ -2191,6 +2197,21 @@ int Server::MaxConcurrencyOf(google::protobuf::Service* service,
return MaxConcurrencyOf(service->GetDescriptor()->full_name(), method_name);
}
+bool Server::AcceptRequest(Controller* cntl) const {
+ const Interceptor* interceptor = _options.interceptor;
+ int error_code = 0;
+ std::string error_text;
+ if (cntl && interceptor &&
+ !interceptor->Accept(cntl, error_code, error_text)) {
+ cntl->SetFailed(error_code,
Review Comment:
OK
--
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] Support user interceptor of server (brpc)
Posted by "thorneliu (via GitHub)" <gi...@apache.org>.
thorneliu commented on code in PR #2137:
URL: https://github.com/apache/brpc/pull/2137#discussion_r1126454968
##########
src/brpc/server.cpp:
##########
@@ -2197,6 +2197,21 @@ int Server::MaxConcurrencyOf(google::protobuf::Service* service,
return MaxConcurrencyOf(service->GetDescriptor()->full_name(), method_name);
}
+bool Server::AcceptRequest(Controller* cntl) const {
+ const Interceptor* interceptor = _options.interceptor;
Review Comment:
Most LIKELY, the interceptor is NULL for a non-interception policy.
So I think we could first check nullity of interceptor and also error_code and error_text will be in smaller scope, for better performance.
```
if (!interceptor) {
return true;
}
int error_code = 0;
std::string error_text;
if (!interceptor->Accept(cntl, error_code, error_text)) {
cntl->SetFailed(error_code, "Reject by Interceptor: %s", error_text.c_str());
return false;
}
return true;
```
--
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] Support user interceptor of server (brpc)
Posted by "thorneliu (via GitHub)" <gi...@apache.org>.
thorneliu commented on code in PR #2137:
URL: https://github.com/apache/brpc/pull/2137#discussion_r1126454968
##########
src/brpc/server.cpp:
##########
@@ -2197,6 +2197,21 @@ int Server::MaxConcurrencyOf(google::protobuf::Service* service,
return MaxConcurrencyOf(service->GetDescriptor()->full_name(), method_name);
}
+bool Server::AcceptRequest(Controller* cntl) const {
+ const Interceptor* interceptor = _options.interceptor;
Review Comment:
Most LIKELY, the interceptor is NULL for a non-interception policy.
So I think we could firstly check the nullity of interceptor and hence error_code and error_text will be in smaller scope iff interceptor is valid, for better performance.
```
if (!interceptor) {
return true;
}
int error_code = 0;
std::string error_text;
if (!interceptor->Accept(cntl, error_code, error_text)) {
cntl->SetFailed(error_code, "Reject by Interceptor: %s", error_text.c_str());
return false;
}
return true;
```
--
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] Support user interceptor of server (brpc)
Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright commented on code in PR #2137:
URL: https://github.com/apache/brpc/pull/2137#discussion_r1131183178
##########
src/brpc/server.cpp:
##########
@@ -2191,6 +2197,21 @@ int Server::MaxConcurrencyOf(google::protobuf::Service* service,
return MaxConcurrencyOf(service->GetDescriptor()->full_name(), method_name);
}
+bool Server::AcceptRequest(Controller* cntl) const {
+ const Interceptor* interceptor = _options.interceptor;
+ int error_code = 0;
+ std::string error_text;
+ if (cntl && interceptor &&
+ !interceptor->Accept(cntl, error_code, error_text)) {
+ cntl->SetFailed(error_code,
Review Comment:
Maybe it would be more appropriate to let the user set the error code. Give users more control, such as custom client retry logic.
--
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] Support user interceptor of server (brpc)
Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright commented on PR #2137:
URL: https://github.com/apache/brpc/pull/2137#issuecomment-1565255835
> 这个能支持按照不同请求类型，或者根据请求里面的一些信息做拦截策略吗？如果支持，可能让用户基于此可以用来定义一个qos算法。
Controller::method()应该可以用来区分请求类型吧
--
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