Fix connection reuse by delaying a poll cycle. (#73)

* Fix connection reuse by delaying a poll cycles. Signed-off-by: John Plevyak <jplevyak@gmail.com>
yxue · May 31, 2019 · bc63ae7 · bc63ae7
1 parent 299c1d1
commit bc63ae7
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 3 deletions.
diff --git a/source/common/http/http1/conn_pool.cc b/source/common/http/http1/conn_pool.cc
@@ -30,6 +30,10 @@ ConnPoolImpl::ConnPoolImpl(Event::Dispatcher& dispatcher, Upstream::HostConstSha
       upstream_ready_timer_(dispatcher_.createTimer([this]() { onUpstreamReady(); })) {}
 
 ConnPoolImpl::~ConnPoolImpl() {
+  while (!delayed_clients_.empty()) {
+    delayed_clients_.front()->codec_client_->close();
+  }
+
   while (!ready_clients_.empty()) {
     ready_clients_.front()->codec_client_->close();
   }
@@ -147,7 +151,12 @@ void ConnPoolImpl::onConnectionEvent(ActiveClient& client, Network::ConnectionEv
     } else if (!client.connect_timer_) {
       // The connect timer is destroyed on connect. The lack of a connect timer means that this
       // client is idle and in the ready pool.
-      removed = client.removeFromList(ready_clients_);
+      if (client.delayed_) {
+        client.delayed_ = 0;
+        removed = client.removeFromList(delayed_clients_);
+      } else {
+        removed = client.removeFromList(ready_clients_);
+      }
       check_for_drained = false;
     } else {
       // The only time this happens is if we actually saw a connect failure.
@@ -222,6 +231,20 @@ void ConnPoolImpl::onResponseComplete(ActiveClient& client) {
 
 void ConnPoolImpl::onUpstreamReady() {
   upstream_ready_enabled_ = false;
+  auto it = delayed_clients_.begin();
+  while (it != delayed_clients_.end()) {
+    ActiveClient& client = **it;
+    it++; // Move forward before moveBetweenLists which would invalidate 'it'.
+    client.delayed_--;
+    if (client.delayed_ == 0) {
+      ENVOY_CONN_LOG(debug, "moving from delay to ready", *client.codec_client_);
+      client.moveBetweenLists(delayed_clients_, ready_clients_);
+    }
+  }
+  if (!delayed_clients_.empty()) {
+    upstream_ready_enabled_ = true;
+    upstream_ready_timer_->enableTimer(std::chrono::milliseconds(0));
+  }
   while (!pending_requests_.empty() && !ready_clients_.empty()) {
     ActiveClient& client = *ready_clients_.front();
     ENVOY_CONN_LOG(debug, "attaching to next request", *client.codec_client_);
@@ -236,7 +259,13 @@ void ConnPoolImpl::onUpstreamReady() {
 
 void ConnPoolImpl::processIdleClient(ActiveClient& client, bool delay) {
   client.stream_wrapper_.reset();
-  if (pending_requests_.empty() || delay) {
+  if (delay) {
+    ENVOY_CONN_LOG(debug, "moving to delay", *client.codec_client_);
+    // N.B. libevent does not guarantee ordering of events, so to ensure that the delayed client
+    // experiences a poll cycle before being made ready, delay for 2 event loops.
+    client.delayed_ = 2;
+    client.moveBetweenLists(busy_clients_, delayed_clients_);
+  } else if (pending_requests_.empty()) {
     // There is nothing to service or delayed processing is requested, so just move the connection
     // into the ready list.
     ENVOY_CONN_LOG(debug, "moving to ready", *client.codec_client_);
@@ -250,7 +279,7 @@ void ConnPoolImpl::processIdleClient(ActiveClient& client, bool delay) {
     pending_requests_.pop_back();
   }
 
-  if (delay && !pending_requests_.empty() && !upstream_ready_enabled_) {
+  if (!delayed_clients_.empty() && !upstream_ready_enabled_) {
     upstream_ready_enabled_ = true;
     upstream_ready_timer_->enableTimer(std::chrono::milliseconds(0));
   }

diff --git a/source/common/http/http1/conn_pool.h b/source/common/http/http1/conn_pool.h
@@ -102,6 +102,7 @@ class ConnPoolImpl : public ConnectionPool::Instance, public ConnPoolImplBase {
     Event::TimerPtr connect_timer_;
     Stats::TimespanPtr conn_length_;
     uint64_t remaining_requests_;
+    int delayed_{0};
   };
 
   typedef std::unique_ptr<ActiveClient> ActiveClientPtr;
@@ -118,6 +119,7 @@ class ConnPoolImpl : public ConnectionPool::Instance, public ConnPoolImplBase {
 
   Stats::TimespanPtr conn_connect_ms_;
   Event::Dispatcher& dispatcher_;
+  std::list<ActiveClientPtr> delayed_clients_;
   std::list<ActiveClientPtr> ready_clients_;
   std::list<ActiveClientPtr> busy_clients_;
   std::list<DrainedCb> drained_callbacks_;