You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by "jenrryyou (via GitHub)" <gi...@apache.org> on 2024/09/02 13:24:30 UTC

[PR] Batch create and accept stream (brpc)

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

   ### What problem does this PR solve?
   
   Issue Number: #2733 
   
   Problem Summary: 支持一次RPC调用创建多个流
   
   
   
   Side effects:
   - Performance effects(性能影响):
   对批量创建stream场景有性能提升
   
   - 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](https://github.com/apache/brpc/blob/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] Batch create and accept stream (brpc)

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


##########
src/brpc/policy/baidu_rpc_protocol.cpp:
##########
@@ -266,7 +273,7 @@ void SendRpcResponse(int64_t correlation_id,
         // Response_stream can be INVALID_STREAM_ID when error occurs.
         if (SendStreamData(sock, &res_buf,
                            accessor.remote_stream_settings()->stream_id(),
-                           accessor.response_stream()) != 0) {
+                           accessor.response_streams()[0]) != 0) {

Review Comment:
   我看上一行注释写着:// Response_stream can be INVALID_STREAM_ID when error occurs.



-- 
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] Batch create and accept stream (brpc)

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


##########
src/brpc/stream_impl.h:
##########
@@ -63,8 +63,11 @@ class BAIDU_CACHELINE_ALIGNMENT Stream : public SocketConnection {
     void FillSettings(StreamSettings *settings);
     static int SetFailed(StreamId id, int error_code, const char* reason_fmt, ...)
         __attribute__ ((__format__ (__printf__, 3, 4)));
+    static int SetFailed(const StreamIds& ids, int error_code, const char* reason_fmt, ...)
+    __attribute__ ((__format__ (__printf__, 3, 4)));
     void Close(int error_code, const char* reason_fmt, ...)
         __attribute__ ((__format__ (__printf__, 3, 4)));
+    int shareHostSocket(Stream& other_stream);

Review Comment:
   share大写开头



##########
src/brpc/policy/baidu_rpc_protocol.cpp:
##########
@@ -266,7 +273,7 @@ void SendRpcResponse(int64_t correlation_id,
         // Response_stream can be INVALID_STREAM_ID when error occurs.
         if (SendStreamData(sock, &res_buf,
                            accessor.remote_stream_settings()->stream_id(),
-                           accessor.response_stream()) != 0) {
+                           accessor.response_streams()[0]) != 0) {

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] Batch create and accept stream (brpc)

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


##########
src/brpc/policy/baidu_rpc_protocol.cpp:
##########
@@ -266,7 +273,7 @@ void SendRpcResponse(int64_t correlation_id,
         // Response_stream can be INVALID_STREAM_ID when error occurs.
         if (SendStreamData(sock, &res_buf,
                            accessor.remote_stream_settings()->stream_id(),
-                           accessor.response_stream()) != 0) {
+                           accessor.response_streams()[0]) != 0) {

Review Comment:
   Response_stream为INVALID_STREAM_ID不会造成越界,因为reponse_streams里还是有元素的。对端收到StreamData会在ProcessRpcResponse里处理
   ![image](https://github.com/user-attachments/assets/20b795d3-8877-46c4-a492-c3eb011b5742)
   
   Response_stream为INVALID_STREAM_ID的情况的然后Response_stream为INVALID_STREAM_ID的时候,cntl一定会传errorcode回来,然后rpc会失败,endRPC里会关闭对端的stream.
   
   P.S. 返回INVALID_STREAM_ID的MR还是我之前提的



-- 
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] Batch create and accept stream (brpc)

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

   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] Batch create and accept stream (brpc)

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


##########
src/brpc/policy/baidu_rpc_protocol.cpp:
##########
@@ -266,7 +273,7 @@ void SendRpcResponse(int64_t correlation_id,
         // Response_stream can be INVALID_STREAM_ID when error occurs.
         if (SendStreamData(sock, &res_buf,
                            accessor.remote_stream_settings()->stream_id(),
-                           accessor.response_stream()) != 0) {
+                           accessor.response_streams()[0]) != 0) {

Review Comment:
   > 是不是用前面232行的`StreamId response_stream_id = response_stream_ids[0]`更好一点?
   同上面的回复,我尝试加个判断



##########
src/brpc/policy/baidu_rpc_protocol.cpp:
##########
@@ -226,11 +228,16 @@ void SendRpcResponse(int64_t correlation_id,
         meta.set_attachment_size(attached_size);
     }
     SocketUniquePtr stream_ptr;
-    if (response_stream_id != INVALID_STREAM_ID) {
+    if (!response_stream_ids.empty()) {
+        StreamId response_stream_id = response_stream_ids[0];

Review Comment:
   除了baidu_rpc_protocol.cpp 672行(这里后面判断了INVALID_STREAM_ID,没有其他地方会设置INVALID_STREAM_ID了(之前controller初始化stream_id会用INVALID_STREAM_ID),不过为了防止以后有人设置,我还是加个判断
   
   > 还需要判断INVALID_STREAM_ID吗?
   
   



##########
src/brpc/stream.cpp:
##########
@@ -759,37 +771,73 @@ int StreamClose(StreamId stream_id) {
 
 int StreamCreate(StreamId *request_stream, Controller &cntl,
                  const StreamOptions* options) {
-    if (cntl._request_stream != INVALID_STREAM_ID) {
+    if (request_stream == NULL) {
+        LOG(ERROR) << "request_stream is NULL";
+        return -1;
+    }
+    StreamIds request_streams;
+    StreamCreate(request_streams, 1, cntl, options);
+    *request_stream = request_streams[0];
+    return 0;
+}
+
+int StreamCreate(StreamIds& request_streams, int request_stream_size, Controller & cntl,
+                 const StreamOptions* options) {
+    if (!cntl._request_streams.empty()) {
         LOG(ERROR) << "Can't create request stream more than once";
         return -1;
     }
-    if (request_stream == NULL) {
-        LOG(ERROR) << "request_stream is NULL";
+    if (!request_streams.empty()) {
+        LOG(ERROR) << "request_streams should be empty";
         return -1;
     }
-    StreamId stream_id;
     StreamOptions opt;
     if (options != NULL) {
         opt = *options;
     }
-    if (Stream::Create(opt, NULL, &stream_id) != 0) {
-        LOG(ERROR) << "Fail to create stream";
-        return -1;
+    for (auto i = 0; i < request_stream_size; ++i) {
+        StreamId stream_id;
+        if (Stream::Create(opt, NULL, &stream_id) != 0) {
+            // Close already created streams
+            Stream::SetFailed(request_streams, 0 , "Fail to create stream at %d index", i);
+            LOG(ERROR) << "Fail to create stream";
+            return -1;
+        }
+        cntl._request_streams.push_back(stream_id);
+        request_streams.push_back(stream_id);
     }
-    cntl._request_stream = stream_id;
-    *request_stream = stream_id;
     return 0;
 }
 
 int StreamAccept(StreamId* response_stream, Controller &cntl,
                  const StreamOptions* options) {
+    if (response_stream == NULL) {
+        LOG(ERROR) << "response_stream is NULL";
+        return -1;
+    }
+    StreamIds response_streams;
+    int res = StreamAccept(response_streams, cntl, options);
+    if(res != 0) {
+        return res;
+    }
+    if(response_streams.size() != 1) {
+        Stream::SetFailed(response_streams, 0, "Logic error");
+        LOG(ERROR) << "accept more than one response_stream";
+        return -1;
+    }

Review Comment:
   > 1. 819行accept成功,并设置到cntl._response_streams了,但是这里返回了失败给用户,即服务端accept失败,客户端成功?
   > 2. 这个接口处于不可用的状态了吧?要调StreamAccept(StreamIds& ...)才能保证一定成功。
   
   理论上819成功,后面不可能失败了,这里只是为了防止用户用错接口(用批量stream create,结果rpc处理的时候用了单个stream accept接口)。不过确实会存在你说的情况,我看下怎么报错更一致一些



##########
src/brpc/controller.cpp:
##########
@@ -1370,34 +1370,54 @@ void* Controller::session_local_data() {
 }
 
 void Controller::HandleStreamConnection(Socket *host_socket) {
-    if (_request_stream == INVALID_STREAM_ID) {
+    if (_request_streams.empty()) {
         CHECK(!has_remote_stream());
         return;
     }
-    SocketUniquePtr ptr;
+    size_t stream_num = _request_streams.size();
+    std::vector<SocketUniquePtr> ptrs(stream_num);
     if (!FailedInline()) {
-        if (Socket::Address(_request_stream, &ptr) != 0) {
-            if (!FailedInline()) {
-                SetFailed(EREQUEST, "Request stream=%" PRIu64 " was closed before responded",
-                                     _request_stream);
-            }
-        } else if (_remote_stream_settings == NULL) {
+        if (_remote_stream_settings == NULL) {
             if (!FailedInline()) {
                 SetFailed(EREQUEST, "The server didn't accept the stream");
             }
+        } else {
+            for (size_t i = 0; i < stream_num; ++i) {
+                if (Socket::Address(_request_streams[i], &ptrs[i]) != 0) {
+                    if (!FailedInline()) {
+                        SetFailed(EREQUEST, "Request stream=%" PRIu64 " was closed before responded",
+                                  _request_streams[i]);
+                        break;
+                    }

Review Comment:
   > 一个closed就导致全部失败吗?
   目前是这么设计的,同一个RPC注册的一批要全部成功,任意一个失败都要全部关闭
   



##########
src/brpc/stream.cpp:
##########
@@ -759,37 +771,73 @@ int StreamClose(StreamId stream_id) {
 
 int StreamCreate(StreamId *request_stream, Controller &cntl,
                  const StreamOptions* options) {
-    if (cntl._request_stream != INVALID_STREAM_ID) {
+    if (request_stream == NULL) {
+        LOG(ERROR) << "request_stream is NULL";
+        return -1;
+    }
+    StreamIds request_streams;
+    StreamCreate(request_streams, 1, cntl, options);
+    *request_stream = request_streams[0];
+    return 0;

Review Comment:
   单个是批量的一种情况(为了兼容老接口),单个复用批量接口比较自然一些。批量调用单个的话有一些特殊的逻辑要处理下,而且要额外传入这次要创建的序号(方便知道是extra_stream里的第几个)



-- 
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] Batch create and accept stream (brpc)

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


##########
src/brpc/policy/baidu_rpc_protocol.cpp:
##########
@@ -266,7 +273,7 @@ void SendRpcResponse(int64_t correlation_id,
         // Response_stream can be INVALID_STREAM_ID when error occurs.
         if (SendStreamData(sock, &res_buf,
                            accessor.remote_stream_settings()->stream_id(),
-                           accessor.response_stream()) != 0) {
+                           accessor.response_streams()[0]) != 0) {

Review Comment:
   Response_stream为INVALID_STREAM_ID也没有问题,对端收到StreamData会在ProcessRpcResponse里处理
   ![image](https://github.com/user-attachments/assets/20b795d3-8877-46c4-a492-c3eb011b5742)
   
   Response_stream为INVALID_STREAM_ID的情况的然后Response_stream为INVALID_STREAM_ID的时候,cntl一定会传errorcode回来,然后rpc会失败,endRPC里会关闭对端的stream.
   
   P.S. 返回INVALID_STREAM_ID的MR还是我之前提的



-- 
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] Batch create and accept stream (brpc)

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


##########
src/brpc/policy/baidu_rpc_protocol.cpp:
##########
@@ -266,7 +273,7 @@ void SendRpcResponse(int64_t correlation_id,
         // Response_stream can be INVALID_STREAM_ID when error occurs.
         if (SendStreamData(sock, &res_buf,
                            accessor.remote_stream_settings()->stream_id(),
-                           accessor.response_stream()) != 0) {
+                           accessor.response_streams()[0]) != 0) {

Review Comment:
   has_remote_stream为true的话,response_streams一定不为空(cntl里response_streams包含了meta里stream_id和extra_stream_ids里的全部stream_id)



-- 
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] Batch create and accept stream (brpc)

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


-- 
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] Batch create and accept stream (brpc)

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

   看了下macos protobuf22的报错
   ![image](https://github.com/user-attachments/assets/bb2708b7-0bde-4803-846a-6a8fd0739360)
   ,是因为链接的库libprotobuf.dylib没有
   google::protobuf::internal::ThreadSafeArena::thread_cache_定义,但是include 的thread_safe_arena.h文件有这个符号。说明brew编译的libprotobuf.dylib macos版本是开启了PROTOBUF_NO_THREADLOCAL宏(所以有google::protobuf::internal::ThreadSafeArena::thread_cache()定义,但是没有google::protobuf::internal::ThreadSafeArena::thread_cache_定义),brpc编译时候是没有PROTOBUF_NO_THREADLOCAL宏,所以链接时候报错。 附图:![image](https://github.com/user-attachments/assets/b8624ddc-6c88-477c-9b88-bf389852dcf6)


-- 
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] Batch create and accept stream (brpc)

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

   
   
   
   > LGTM
   
   有空帮忙点下merge,多谢


-- 
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] Batch create and accept stream (brpc)

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


##########
src/brpc/stream.cpp:
##########
@@ -759,37 +771,73 @@ int StreamClose(StreamId stream_id) {
 
 int StreamCreate(StreamId *request_stream, Controller &cntl,
                  const StreamOptions* options) {
-    if (cntl._request_stream != INVALID_STREAM_ID) {
+    if (request_stream == NULL) {
+        LOG(ERROR) << "request_stream is NULL";
+        return -1;
+    }
+    StreamIds request_streams;
+    StreamCreate(request_streams, 1, cntl, options);
+    *request_stream = request_streams[0];
+    return 0;

Review Comment:
   是不是StreamCreate StreamIds接口循环调StreamCreate StreamId接口,更自然和合理一点?



##########
src/brpc/policy/baidu_rpc_protocol.cpp:
##########
@@ -226,11 +228,16 @@ void SendRpcResponse(int64_t correlation_id,
         meta.set_attachment_size(attached_size);
     }
     SocketUniquePtr stream_ptr;
-    if (response_stream_id != INVALID_STREAM_ID) {
+    if (!response_stream_ids.empty()) {
+        StreamId response_stream_id = response_stream_ids[0];

Review Comment:
   还需要判断INVALID_STREAM_ID吗?



##########
src/brpc/policy/baidu_rpc_protocol.cpp:
##########
@@ -266,7 +273,7 @@ void SendRpcResponse(int64_t correlation_id,
         // Response_stream can be INVALID_STREAM_ID when error occurs.
         if (SendStreamData(sock, &res_buf,
                            accessor.remote_stream_settings()->stream_id(),
-                           accessor.response_stream()) != 0) {
+                           accessor.response_streams()[0]) != 0) {

Review Comment:
   是不是用前面232行的`StreamId response_stream_id = response_stream_ids[0]`更好一点?



##########
src/brpc/controller.cpp:
##########
@@ -1370,34 +1370,54 @@ void* Controller::session_local_data() {
 }
 
 void Controller::HandleStreamConnection(Socket *host_socket) {
-    if (_request_stream == INVALID_STREAM_ID) {
+    if (_request_streams.empty()) {
         CHECK(!has_remote_stream());
         return;
     }
-    SocketUniquePtr ptr;
+    size_t stream_num = _request_streams.size();
+    std::vector<SocketUniquePtr> ptrs(stream_num);
     if (!FailedInline()) {
-        if (Socket::Address(_request_stream, &ptr) != 0) {
-            if (!FailedInline()) {
-                SetFailed(EREQUEST, "Request stream=%" PRIu64 " was closed before responded",
-                                     _request_stream);
-            }
-        } else if (_remote_stream_settings == NULL) {
+        if (_remote_stream_settings == NULL) {
             if (!FailedInline()) {
                 SetFailed(EREQUEST, "The server didn't accept the stream");
             }
+        } else {
+            for (size_t i = 0; i < stream_num; ++i) {
+                if (Socket::Address(_request_streams[i], &ptrs[i]) != 0) {
+                    if (!FailedInline()) {
+                        SetFailed(EREQUEST, "Request stream=%" PRIu64 " was closed before responded",
+                                  _request_streams[i]);
+                        break;
+                    }

Review Comment:
   一个closed就导致全部失败吗?



##########
src/brpc/stream.cpp:
##########
@@ -759,37 +771,73 @@ int StreamClose(StreamId stream_id) {
 
 int StreamCreate(StreamId *request_stream, Controller &cntl,
                  const StreamOptions* options) {
-    if (cntl._request_stream != INVALID_STREAM_ID) {
+    if (request_stream == NULL) {
+        LOG(ERROR) << "request_stream is NULL";
+        return -1;
+    }
+    StreamIds request_streams;
+    StreamCreate(request_streams, 1, cntl, options);
+    *request_stream = request_streams[0];
+    return 0;
+}
+
+int StreamCreate(StreamIds& request_streams, int request_stream_size, Controller & cntl,
+                 const StreamOptions* options) {
+    if (!cntl._request_streams.empty()) {
         LOG(ERROR) << "Can't create request stream more than once";
         return -1;
     }
-    if (request_stream == NULL) {
-        LOG(ERROR) << "request_stream is NULL";
+    if (!request_streams.empty()) {
+        LOG(ERROR) << "request_streams should be empty";
         return -1;
     }
-    StreamId stream_id;
     StreamOptions opt;
     if (options != NULL) {
         opt = *options;
     }
-    if (Stream::Create(opt, NULL, &stream_id) != 0) {
-        LOG(ERROR) << "Fail to create stream";
-        return -1;
+    for (auto i = 0; i < request_stream_size; ++i) {
+        StreamId stream_id;
+        if (Stream::Create(opt, NULL, &stream_id) != 0) {
+            // Close already created streams
+            Stream::SetFailed(request_streams, 0 , "Fail to create stream at %d index", i);
+            LOG(ERROR) << "Fail to create stream";
+            return -1;
+        }
+        cntl._request_streams.push_back(stream_id);
+        request_streams.push_back(stream_id);
     }
-    cntl._request_stream = stream_id;
-    *request_stream = stream_id;
     return 0;
 }
 
 int StreamAccept(StreamId* response_stream, Controller &cntl,
                  const StreamOptions* options) {
+    if (response_stream == NULL) {
+        LOG(ERROR) << "response_stream is NULL";
+        return -1;
+    }
+    StreamIds response_streams;
+    int res = StreamAccept(response_streams, cntl, options);
+    if(res != 0) {
+        return res;
+    }
+    if(response_streams.size() != 1) {
+        Stream::SetFailed(response_streams, 0, "Logic error");
+        LOG(ERROR) << "accept more than one response_stream";
+        return -1;
+    }

Review Comment:
   1. 819行accept成功,并设置到cntl._response_streams了,但是这里返回了失败给用户,即服务端accept失败,客户端成功?
   2. 这个接口处于不可用的状态了吧?要调StreamAccept(StreamIds& ...)才能保证一定成功。



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