From 07c150868f7833b8cf659cf888aa3febd8c2ca8a Mon Sep 17 00:00:00 2001
From: Yang Chi <yangchi@fb.com>
Date: Thu, 12 Nov 2015 15:34:45 -0800
Subject: [PATCH] Allow a AsyncSocket to be corked the whole time

Summary: Add a new method to cork a socket in a persistent manner, instead of the current on-off manner. This is default to false. The liger part of turning this on will be in a separate diff.

I thought about whether I need to turn cork off based on some criteria to alleviate the perf degradation. The obvious things I can think off is just amount of data written as a threshold, or a timeout. But TCP is doing this already for us, unless we want the data threshold to be less than MSS, or we want the timeout to be less than 200ms. THoughts?

Reviewed By: shikong

Differential Revision: D2639260

fb-gh-sync-id: 2821f669c9f72d5ac4c33195bb192fc4110ffe9d
---
 folly/io/async/AsyncSSLSocket.cpp |  2 +-
 folly/io/async/AsyncSSLSocket.h   |  2 --
 folly/io/async/AsyncSocket.cpp    | 31 +++++++++++++++++++++++++++++++
 folly/io/async/AsyncSocket.h      | 20 ++++++++++++++++++++
 4 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/folly/io/async/AsyncSSLSocket.cpp b/folly/io/async/AsyncSSLSocket.cpp
index 101c1fc6..d2520bef 100644
--- a/folly/io/async/AsyncSSLSocket.cpp
+++ b/folly/io/async/AsyncSSLSocket.cpp
@@ -1305,7 +1305,7 @@ ssize_t AsyncSSLSocket::performWrite(const iovec* vec,
       return -1;
   }
 
-  bool cork = isSet(flags, WriteFlags::CORK);
+  bool cork = isSet(flags, WriteFlags::CORK) || persistentCork_;
   CorkGuard guard(fd_, count > 1, cork, &corked_);
 
 #if 0
diff --git a/folly/io/async/AsyncSSLSocket.h b/folly/io/async/AsyncSSLSocket.h
index b70fd283..ee750031 100644
--- a/folly/io/async/AsyncSSLSocket.h
+++ b/folly/io/async/AsyncSSLSocket.h
@@ -832,8 +832,6 @@ class AsyncSSLSocket : public virtual AsyncSocket {
 
   static void sslInfoCallback(const SSL *ssl, int type, int val);
 
-  // Whether we've applied the TCP_CORK option to the socket
-  bool corked_{false};
   // SSL related members.
   bool server_{false};
   // Used to prevent client-initiated renegotiation.  Note that AsyncSSLSocket
diff --git a/folly/io/async/AsyncSocket.cpp b/folly/io/async/AsyncSocket.cpp
index 28668245..373066e0 100644
--- a/folly/io/async/AsyncSocket.cpp
+++ b/folly/io/async/AsyncSocket.cpp
@@ -1206,6 +1206,37 @@ int AsyncSocket::setTCPProfile(int profd) {
   return 0;
 }
 
+void AsyncSocket::setPersistentCork(bool cork) {
+  if (setCork(cork) == 0) {
+    persistentCork_ = cork;
+  }
+}
+
+int AsyncSocket::setCork(bool cork) {
+#ifdef TCP_CORK
+  if (fd_ < 0) {
+    VLOG(4) << "AsyncSocket::setCork() called on non-open socket "
+            << this << "(stats=" << state_ << ")";
+    return EINVAL;
+  }
+
+  if (corked_ == cork) {
+    return 0;
+  }
+
+  int flag = cork ? 1 : 0;
+  if (setsockopt(fd_, IPPROTO_TCP, TCP_CORK, &flag, sizeof(flag)) != 0) {
+    int errnoCopy = errno;
+    VLOG(2) << "faield to turn on TCP_CORK option on AsyncSocket"
+            << this << "(fd=" << fd_ << ", state=" << state_ << "):"
+            << folly::errnoStr(errnoCopy);
+    return errnoCopy;
+  }
+  corked_ = cork;
+#endif
+  return 0;
+}
+
 void AsyncSocket::ioReady(uint16_t events) noexcept {
   VLOG(7) << "AsyncSocket::ioRead() this=" << this << ", fd" << fd_
           << ", events=" << std::hex << events << ", state=" << state_;
diff --git a/folly/io/async/AsyncSocket.h b/folly/io/async/AsyncSocket.h
index de1f5c23..d158878d 100644
--- a/folly/io/async/AsyncSocket.h
+++ b/folly/io/async/AsyncSocket.h
@@ -462,6 +462,14 @@ class AsyncSocket : virtual public AsyncTransportWrapper {
   #define SO_SET_NAMESPACE        41
   int setTCPProfile(int profd);
 
+  /**
+   * Set TCP_CORK on the socket, and turn on/off the persistentCork_ flag
+   *
+   * When persistentCork_ is true, CorkGuard in AsyncSSLSocket will not be
+   * able to toggle TCP_CORK
+   *
+   */
+  void setPersistentCork(bool cork);
 
   /**
    * Generic API for reading a socket option.
@@ -779,6 +787,13 @@ class AsyncSocket : virtual public AsyncTransportWrapper {
 
   std::string withAddr(const std::string& s);
 
+  /**
+   * Set TCP_CORK on this socket
+   *
+   * @return 0 if Cork is turned on, or non-zero errno on error
+   */
+  int setCork(bool cork);
+
   StateEnum state_;                     ///< StateEnum describing current state
   uint8_t shutdownFlags_;               ///< Shutdown state (ShutdownFlags)
   uint16_t eventFlags_;                 ///< EventBase::HandlerFlags settings
@@ -807,6 +822,11 @@ class AsyncSocket : virtual public AsyncTransportWrapper {
 
   std::chrono::steady_clock::time_point connectStartTime_;
   std::chrono::steady_clock::time_point connectEndTime_;
+
+  // Whether this connection is persistently corked
+  bool persistentCork_{false};
+  // Whether we've applied the TCP_CORK option to the socket
+  bool corked_{false};
 };
 
 
-- 
2.34.1