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

diff --git a/folly/io/async/AsyncSSLSocket.cpp b/folly/io/async/AsyncSSLSocket.cpp
index 101c1fc69edbbcf39019d4dca6c926c56cfac173..d2520bef12ee5e07692e03db4096be49944bb889 100644 (file)
--- 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 b70fd2839256771a74075e7aba19b958656927f8..ee7500311a0d31535c6899422afa270c72caa336 100644 (file)
--- 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 2866824535b1b2f07dfd8d151e007bb74a2f18b6..373066e0a40fd07ebb7048ae752e9a32ef7d9deb 100644 (file)
--- 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 de1f5c23bfc22b3a006fdca55d770a225887f9bc..d158878dac2922adbc78e6eed9df8bea87a9cca3 100644 (file)
--- 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};
 };