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 2024/02/25 16:03:53 UTC

[PR] Support shutdown write and notify for success write (brpc)

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

   ### What problem does this PR solve?
   
   Issue Number:
   
   Problem Summary:
   
   ### What is changed and the side effects?
   
   Changed:
   
   1. 支持半关闭：无需SetFailed，关闭写入端，但不影响读数据。
   2. 支持在数据写入socket写入成功后调用id_wait回调函数：对于一些请求和响应不是一一对应的场景，例如WebSocket、TCP Stream客户端、Streaming rpc等，只需要关注数据是否写入成功。数据写入socket写入成功后调用id_wait回调函数，可以统计成功率、耗时等。
   
   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 shutdown write and notify for success write (brpc)

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

   @wwbmmm 有空看看这个PR


-- 
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 shutdown write and notify for success write (brpc)

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


##########
src/brpc/socket.cpp:
##########
@@ -307,33 +307,56 @@ const uint32_t MAX_PIPELINED_COUNT = 16384;
 
 struct BAIDU_CACHELINE_ALIGNMENT Socket::WriteRequest {
     static WriteRequest* const UNCONNECTED;
-    
+
     butil::IOBuf data;
     WriteRequest* next;
     bthread_id_t id_wait;
-    Socket* socket;
+
+    void clear_and_set_control_bits(bool notify_on_success,
+                                    bool shutdown_write) {
+        _socket_and_control_bits.set_extra(
+            (uint16_t)notify_on_success << 1 | (uint16_t)shutdown_write);
+    }
+
+    void set_socket(Socket* s) {
+        _socket_and_control_bits.set(s);
+    }
+
+    // If this field is set to true, notify when write successfully.
+    bool is_notify_on_success() const {
+        return _socket_and_control_bits.extra() & ((uint16_t)1 << 1);
+    }
+
+    // Whether shutdown write of the socket after this write complete.
+    bool need_shutdown_write() const {
+        return _socket_and_control_bits.extra() & (uint16_t)1;
+    }
+
+    Socket* get_socket() const {
+        return _socket_and_control_bits.get();
+    }
     
     uint32_t pipelined_count() const {
-        return (_pc_and_udmsg >> 48) & 0x3FFF;
+        return _pc_and_udmsg.extra() & 0x3FFF;
     }
     uint32_t get_auth_flags() const {
-       return (_pc_and_udmsg >> 62) & 0x03;
+       return (_pc_and_udmsg.extra() >> 14) & 0x03;
     }
     void clear_pipelined_count_and_auth_flags() {
-        _pc_and_udmsg &= 0xFFFFFFFFFFFFULL;
+        _pc_and_udmsg.reset_ptr_and_extra();

Review Comment:
   done



##########
src/brpc/socket.h:
##########
@@ -917,6 +994,8 @@ friend void DereferenceSocket(Socket*);
     // Storing data that are not flushed into `fd' yet.
     butil::atomic<WriteRequest*> _write_head;
 
+    bool _is_wirte_shutdown;

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 shutdown write and notify for success write (brpc)

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


-- 
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 shutdown write and notify for success write (brpc)

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

   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 shutdown write and notify for success write (brpc)

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


##########
src/brpc/socket.cpp:
##########
@@ -307,33 +307,56 @@ const uint32_t MAX_PIPELINED_COUNT = 16384;
 
 struct BAIDU_CACHELINE_ALIGNMENT Socket::WriteRequest {
     static WriteRequest* const UNCONNECTED;
-    
+
     butil::IOBuf data;
     WriteRequest* next;
     bthread_id_t id_wait;
-    Socket* socket;
+
+    void clear_and_set_control_bits(bool notify_on_success,
+                                    bool shutdown_write) {
+        _socket_and_control_bits.set_extra(
+            (uint16_t)notify_on_success << 1 | (uint16_t)shutdown_write);
+    }
+
+    void set_socket(Socket* s) {
+        _socket_and_control_bits.set(s);
+    }
+
+    // If this field is set to true, notify when write successfully.
+    bool is_notify_on_success() const {
+        return _socket_and_control_bits.extra() & ((uint16_t)1 << 1);
+    }
+
+    // Whether shutdown write of the socket after this write complete.
+    bool need_shutdown_write() const {
+        return _socket_and_control_bits.extra() & (uint16_t)1;
+    }
+
+    Socket* get_socket() const {
+        return _socket_and_control_bits.get();
+    }
     
     uint32_t pipelined_count() const {
-        return (_pc_and_udmsg >> 48) & 0x3FFF;
+        return _pc_and_udmsg.extra() & 0x3FFF;
     }
     uint32_t get_auth_flags() const {
-       return (_pc_and_udmsg >> 62) & 0x03;
+       return (_pc_and_udmsg.extra() >> 14) & 0x03;
     }
     void clear_pipelined_count_and_auth_flags() {
-        _pc_and_udmsg &= 0xFFFFFFFFFFFFULL;
+        _pc_and_udmsg.reset_ptr_and_extra();

Review Comment:
   是不是应该是reset_extra() ？



##########
src/brpc/socket.h:
##########
@@ -917,6 +994,8 @@ friend void DereferenceSocket(Socket*);
     // Storing data that are not flushed into `fd' yet.
     butil::atomic<WriteRequest*> _write_head;
 
+    bool _is_wirte_shutdown;

Review Comment:
   wirte -> write



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