You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by "MrGuin (via GitHub)" <gi...@apache.org> on 2023/06/15 10:27:19 UTC

[PR] StreamWrite and Async RPC write to socket in background bthread. (brpc)

MrGuin opened a new pull request, #2280:
URL: https://github.com/apache/brpc/pull/2280

   Add write_in_background parameter to Socket::StartWrite.
   StreamWrite and async RPC create background bthread to write to socket so that current thread won't block for 10-20us writing to 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: 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] StreamWrite and Async RPC write to socket in background bthread. (brpc)

Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright commented on code in PR #2280:
URL: https://github.com/apache/brpc/pull/2280#discussion_r1232979292


##########
src/brpc/stream.cpp:
##########
@@ -290,7 +290,9 @@ int Stream::AppendIfNotFull(const butil::IOBuf &data) {
 
     size_t data_length = data.length();
     butil::IOBuf copied_data(data);
-    const int rc = _fake_socket_weak_ref->Write(&copied_data);
+    // Always write stream message in background bthread to avoid blocking current thread.
+    bool write_in_background = true;

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] StreamWrite and Async RPC write to socket in background bthread. (brpc)

Posted by "MrGuin (via GitHub)" <gi...@apache.org>.
MrGuin commented on code in PR #2280:
URL: https://github.com/apache/brpc/pull/2280#discussion_r1233652155


##########
src/brpc/controller.cpp:
##########
@@ -1173,14 +1173,16 @@ void Controller::IssueRPC(int64_t start_realtime_us) {
     wopt.ignore_eovercrowded = has_flag(FLAGS_IGNORE_EOVERCROWDED);
     int rc;
     size_t packet_size = 0;
+    // For async RPC, write to socket in background bthread to avoid blocking current thread.
+    bool write_in_background = _done != nullptr;

Review Comment:
   确实，我稍后改一下。
   问下 `SocketVarsCollector::nkeepwrite` 和 `nkeepwrite_second` 这两个 bvar 是什么涵义呢？看起来是每次 `KeepWrite` 都把 `nkeepwrite` 自增一下，那 `nkeepwrite_second` 就是一秒内创建了多少次 KeepWrite bthread 吗？我这里在异步 RPC 的负载下观察到监控页面里的 rpc_keepwrite_second，这个值在改动之前是每秒一万二左右，改动之后每次都直接起 bthread 这个值就变成了每秒二万四左右。看起来像是多创建了一倍的 KeepWrite bthread，而且有些混合场景下性能会有下降，感觉可能是 bthread 创建调度开销导致的。
   那异步 RPC 默认行为还是走正常的流程，在某些特殊场景下（比如要一次性发出大量 RPC）可以启用这个选项。



-- 
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] StreamWrite and Async RPC write to socket in background bthread. (brpc)

Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on code in PR #2280:
URL: https://github.com/apache/brpc/pull/2280#discussion_r1233465848


##########
src/brpc/controller.cpp:
##########
@@ -1173,14 +1173,16 @@ void Controller::IssueRPC(int64_t start_realtime_us) {
     wopt.ignore_eovercrowded = has_flag(FLAGS_IGNORE_EOVERCROWDED);
     int rc;
     size_t packet_size = 0;
+    // For async RPC, write to socket in background bthread to avoid blocking current thread.
+    bool write_in_background = _done != nullptr;

Review Comment:
   +1
   或者加个gflag控制也行



-- 
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] StreamWrite and Async RPC write to socket in background bthread. (brpc)

Posted by "MrGuin (via GitHub)" <gi...@apache.org>.
MrGuin commented on code in PR #2280:
URL: https://github.com/apache/brpc/pull/2280#discussion_r1233915451


##########
src/brpc/controller.cpp:
##########
@@ -1173,14 +1173,16 @@ void Controller::IssueRPC(int64_t start_realtime_us) {
     wopt.ignore_eovercrowded = has_flag(FLAGS_IGNORE_EOVERCROWDED);
     int rc;
     size_t packet_size = 0;
+    // For async RPC, write to socket in background bthread to avoid blocking current thread.
+    bool write_in_background = _done != nullptr;

Review Comment:
   修改了，write_in_background 作为一个选项传递给 Socket::Write，默认为 false，需要的场景可手动启用。大家再帮看一下有没有问题。



-- 
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] StreamWrite and Async RPC write to socket in background bthread. (brpc)

Posted by "lorinlee (via GitHub)" <gi...@apache.org>.
lorinlee commented on code in PR #2280:
URL: https://github.com/apache/brpc/pull/2280#discussion_r1233007764


##########
src/brpc/controller.cpp:
##########
@@ -1173,14 +1173,16 @@ void Controller::IssueRPC(int64_t start_realtime_us) {
     wopt.ignore_eovercrowded = has_flag(FLAGS_IGNORE_EOVERCROWDED);
     int rc;
     size_t packet_size = 0;
+    // For async RPC, write to socket in background bthread to avoid blocking current thread.
+    bool write_in_background = _done != nullptr;

Review Comment:
   +1，这里每次都走KeepWrite的话，不确定是不是所有场景都是正向收益，最糟糕的场景如果batch不起来的话，每次Write都需要启动后台bthread来做，那每次Write都需要额外负担一个bthread创建+等待调度的时延开销，所以我觉得Options里让用户指定好一点，默认为false，有需要的场景单独设就好了



-- 
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] StreamWrite and Async RPC write to socket in background bthread. (brpc)

Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm commented on PR #2280:
URL: https://github.com/apache/brpc/pull/2280#issuecomment-1605965691

   CI失败的问题master分支已修复，请更新一下


-- 
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] StreamWrite and Async RPC write to socket in background bthread. (brpc)

Posted by "chenBright (via GitHub)" <gi...@apache.org>.
chenBright commented on code in PR #2280:
URL: https://github.com/apache/brpc/pull/2280#discussion_r1232978741


##########
src/brpc/socket.h:
##########
@@ -292,11 +292,11 @@ friend class policy::H2GlobalStreamCreator;
             , pipelined_count(0), auth_flags(0)
             , ignore_eovercrowded(false) {}
     };
-    int Write(butil::IOBuf *msg, const WriteOptions* options = NULL);
+    int Write(butil::IOBuf *msg, const WriteOptions* options = NULL, bool in_background = false);

Review Comment:
   in_background应该是WriteOptions的一项吧，这样是不是合理一点？同时也不用加多一个默认参数了。



##########
src/brpc/controller.cpp:
##########
@@ -1173,14 +1173,16 @@ void Controller::IssueRPC(int64_t start_realtime_us) {
     wopt.ignore_eovercrowded = has_flag(FLAGS_IGNORE_EOVERCROWDED);
     int rc;
     size_t packet_size = 0;
+    // For async RPC, write to socket in background bthread to avoid blocking current thread.
+    bool write_in_background = _done != nullptr;

Review Comment:
   是不是可以在Controller中加一个接口，让用户选择是否要write_in_background？



##########
src/brpc/stream.cpp:
##########
@@ -290,7 +290,9 @@ int Stream::AppendIfNotFull(const butil::IOBuf &data) {
 
     size_t data_length = data.length();
     butil::IOBuf copied_data(data);
-    const int rc = _fake_socket_weak_ref->Write(&copied_data);
+    // Always write stream message in background bthread to avoid blocking current thread.
+    bool write_in_background = true;

Review Comment:
   同Controller，让用户选择是否要write_in_background？



-- 
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] Add option for StreamWrite and Async RPC to write to socket in background bthread. (brpc)

Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm merged PR #2280:
URL: https://github.com/apache/brpc/pull/2280


-- 
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