[GitHub] [brpc] wwbmmm commented on a diff in pull request #2098: support TCP heartbeat of client

["wwbmmm (via GitHub)" <gi...@apache.org>]

wwbmmm commented on code in PR #2098:
URL: https://github.com/apache/brpc/pull/2098#discussion_r1091347215


##########
src/brpc/socket.h:
##########
@@ -198,6 +212,31 @@ struct SocketOptions {
     std::shared_ptr<AppConnect> app_connect;
     // The created socket will set parsing_context with this value.
     Destroyable* initial_parsing_context;
+
+    // Socket keepalive related options.
+    // Refer to `SocketKeepaliveOptions' for details
+    void enable_keepalive() {
+        if (!_keepalive_options) {
+            _keepalive_options.reset(new SocketKeepaliveOptions);
+        }
+    }
+    bool has_keepalive_options() { return _keepalive_options != NULL; }
+    const SocketKeepaliveOptions& keepalive_options() const {
+        return *_keepalive_options;
+    }
+    SocketKeepaliveOptions* mutable_keepalive_options() {
+        enable_keepalive();
+        return _keepalive_options.get();
+    }
+    std::shared_ptr<SocketKeepaliveOptions>
+    shared_keepalibe_options() const {
+        return _keepalive_options;
+    }
+
+private:
+    // SocketKeepaliveOptions is not often used, allocate it on heap to
+    // prevent SocketKeepaliveOptions from being bloated in most cases.
+    std::shared_ptr<SocketKeepaliveOptions> _keepalive_options;

Review Comment:
   看socket option的其它成员都是public，比如
   `std::shared_ptr<SocketSSLContext> initial_ssl_ctx;`
   `std::shared_ptr<AppConnect> app_connect;`
   这个是不是也直接public就好了？



##########
src/brpc/socket.h:
##########
@@ -198,6 +212,31 @@ struct SocketOptions {
     std::shared_ptr<AppConnect> app_connect;
     // The created socket will set parsing_context with this value.
     Destroyable* initial_parsing_context;
+
+    // Socket keepalive related options.
+    // Refer to `SocketKeepaliveOptions' for details
+    void enable_keepalive() {
+        if (!_keepalive_options) {
+            _keepalive_options.reset(new SocketKeepaliveOptions);
+        }
+    }
+    bool has_keepalive_options() { return _keepalive_options != NULL; }
+    const SocketKeepaliveOptions& keepalive_options() const {
+        return *_keepalive_options;
+    }
+    SocketKeepaliveOptions* mutable_keepalive_options() {
+        enable_keepalive();
+        return _keepalive_options.get();
+    }
+    std::shared_ptr<SocketKeepaliveOptions>
+    shared_keepalibe_options() const {

Review Comment:
   keepalibe -> keepalive



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