wwbmmm commented on issue #3167:
URL: https://github.com/apache/brpc/issues/3167#issuecomment-3660079857

   > 是的,当前方案只是将存在rdma宏定义的方法简单抽取出来,然后拆分到不同transport子类上
   
   
这种方法看似简单,但实际实现可能会遇到问题,这些方法里面会访问Socket的私有成员,这就意味着具体Transport的实现里面,还要访问Socket的私有成员。这样并没有从根本上解决Socket和Transport解耦的问题,而且会引入新的问题——Socket里和Transport无关的一些代码被复制到多个Transport的实现代码里,增加了维护成本。
   
   因此,我不建议这种改法,我觉得可以这样:
   
   1. 将Socket里面,和特定transport相关的私有成员抽取出来到各自的transport中,如
   
   ```cpp
   class Socket {
   private:
     Transport* _transport;
   };
   
   class TcpTransport {
   private:
     int _fd;
     // ...其他tcp独有的成员
   };
   
   class RdmaTransport {
   private:
     rdma::RdmaEndpoint* _rdma_ep;
     RdmaState _rdma_state;
     TcpTransport* _tcp_transport; // RDMA状态关闭时, fallback到tcp
     // ...其他rdma独有的成员
   };
   ```
   
   2. 把Socket里面,#if BRPC_WITH_RDMA 相关的代码抽取到transport中,如
   
   StartWrite里面
   
   ```cpp
       if (_conn) {
           butil::IOBuf* data_arr[1] = { &req->data };
           nw = _conn->CutMessageIntoFileDescriptor(fd(), data_arr, 1);
       } else {
   #if BRPC_WITH_RDMA
           if (_rdma_ep && _rdma_state != RDMA_OFF) {
               butil::IOBuf* data_arr[1] = { &req->data };
               nw = _rdma_ep->CutFromIOBufList(data_arr, 1);
           } else {
   #else
           {
   #endif
               nw = req->data.cut_into_file_descriptor(fd());
           }
       }
   ```
   
   这段代码可以改成
   
   ```cpp
       if (_conn) {
           butil::IOBuf* data_arr[1] = { &req->data };
           nw = _conn->CutMessageIntoFileDescriptor(fd(), data_arr, 1);
       } else {
           nw = _transport->CutFromIOBuf(&req->data);
       }
   ```
   
   然后各自的transport里面
   
   ```cpp
   int TcpTransport::CutFromIOBuf(butil::IOBuf* buf) {
       return buf->cut_into_file_descriptor(_fd);
   }
   
   int RdmaTransport::CutFromIOBuf(butil::IOBuf* buf) {
       if (_rdma_ep && _rdma_state != RDMA_OFF) {
           butil::IOBuf* data_arr[1] = { buf };
           return _rdma_ep->CutFromIOBufList(data_arr, 1);
       } else {
           return _tcp_transport->CutFromIOBuf(buf);
       }
   }
   ```
   
   
   


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

Reply via email to