zyearn commented on code in PR #2177:
URL: https://github.com/apache/brpc/pull/2177#discussion_r1167982737
##########
src/brpc/controller.cpp:
##########
@@ -719,6 +719,37 @@ inline bool does_error_affect_main_socket(int error_code) {
error_code == EINVAL/*returned by connect "0.0.0.1"*/;
}
+inline void maybe_block_server(int error_code, Controller* cntl,
SharedLoadBalancer* lb, SocketId sock) {
+ if (!does_error_affect_main_socket(error_code)) {
+ // Error code does not indicate that server is down, we can't block it
+ return;
+ }
+ if (!lb) {
+ // Single server mode, we can't block the only server
+ return;
+ }
+ // We try to SelectServer once to check if sock is the last available
server
+ ExcludedServers* excluded = ExcludedServers::Create(1);
+ excluded->Add(sock);
+ SocketUniquePtr tmp_sock;
+ LoadBalancer::SelectIn sel_in = { 0, false, true, 0, excluded };
+ LoadBalancer::SelectOut sel_out(&tmp_sock);
+ const int rc = lb->SelectServer(sel_in, &sel_out);
+ ExcludedServers::Destroy(excluded);
+ if (rc != 0 || tmp_sock->id() == sock) {
+ // sock is the last available server in this LB, we can't block it
+ return;
+ }
+ // main socket should die as well
+ // NOTE: main socket may be wrongly set failed (provided that
+ // short/pooled socket does not hold a ref of the main socket).
+ // E.g. a in-parallel RPC sets the peer_id to be failed
Review Comment:
'an'
##########
src/brpc/controller.cpp:
##########
@@ -749,9 +780,8 @@ void Controller::Call::OnComplete(
// "single" streams are often maintained in a separate SocketMap and
// different from the main socket as well.
if (c->_stream_creator != NULL &&
- does_error_affect_main_socket(error_code) &&
(sending_sock == NULL || sending_sock->id() != peer_id)) {
- Socket::SetFailed(peer_id);
+ maybe_block_server(error_code, c, c->_lb.get(), peer_id);
Review Comment:
实现是"block socket"不是block server". 或许叫checkAndSetFailed更明确一点？
##########
src/brpc/controller.cpp:
##########
@@ -719,6 +719,37 @@ inline bool does_error_affect_main_socket(int error_code) {
error_code == EINVAL/*returned by connect "0.0.0.1"*/;
}
+inline void maybe_block_server(int error_code, Controller* cntl,
SharedLoadBalancer* lb, SocketId sock) {
+ if (!does_error_affect_main_socket(error_code)) {
Review Comment:
为什么要把does_error_affect_main_socket放到这个函数里？
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
