A/B experiment for re-establishing TCP connections.

We would like to test the impact of automatic retries when a TCP connection exceeds a certain threshold before it gets back an ACK. We are observing a fair number of sockets where the connection had been established, but the sockets
were not used thereafter.


r=mbelche


Review URL: http://codereview.chromium.org/3191019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57276 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 50 insertions, 25 deletions

diff --git a/net/socket/client_socket.cc b/net/socket/client_socket.cc
index c0c62cd..7722c64 100644
--- a/net/socket/client_socket.cc
+++ b/net/socket/client_socket.cc
@@ -4,6 +4,7 @@
 
 #include "net/socket/client_socket.h"
 
+#include "base/field_trial.h"
 #include "base/histogram.h"
 
 namespace net {
@@ -41,7 +42,15 @@ void ClientSocket::EmitPreconnectionHistograms() const {
     result += 3;
   else if (subresource_speculation_)
     result += 6;
-  UMA_HISTOGRAM_ENUMERATION("Net.PreconnectUtilization", result, 9);
+
+  static const bool connect_backup_jobs_fieldtrial =
+      FieldTrialList::Find("ConnnectBackupJobs") &&
+      !FieldTrialList::Find("ConnnectBackupJobs")->group_name().empty();
+  if (connect_backup_jobs_fieldtrial) {
+    UMA_HISTOGRAM_ENUMERATION(
+        FieldTrial::MakeName("Net.PreconnectUtilization", "ConnnectBackupJobs"),
+        result, 9);
+  }
 }
 
 void ClientSocket::SetSubresourceSpeculation() {
diff --git a/net/socket/client_socket_pool_base.cc b/net/socket/client_socket_pool_base.cc
index 334a9dc..2e8534e 100644
--- a/net/socket/client_socket_pool_base.cc
+++ b/net/socket/client_socket_pool_base.cc
@@ -27,6 +27,10 @@ namespace {
 // some conditions.  See http://crbug.com/4606.
 const int kCleanupInterval = 10;  // DO NOT INCREASE THIS TIMEOUT.
 
+// Indicate whether or not we should establish a new TCP connection after a
+// certain timeout has passed without receiving an ACK.
+bool g_connect_backup_jobs_enabled = true;
+
 }  // namespace
 
 namespace net {
@@ -137,7 +141,7 @@ ClientSocketPoolBaseHelper::ClientSocketPoolBaseHelper(
       unused_idle_socket_timeout_(unused_idle_socket_timeout),
       used_idle_socket_timeout_(used_idle_socket_timeout),
       connect_job_factory_(connect_job_factory),
-      backup_jobs_enabled_(false),
+      connect_backup_jobs_enabled_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)),
       pool_generation_number_(0),
       in_destructor_(false) {
@@ -247,10 +251,11 @@ int ClientSocketPoolBaseHelper::RequestSocketInternal(
     // If we don't have any sockets in this group, set a timer for potentially
     // creating a new one.  If the SYN is lost, this backup socket may complete
     // before the slow socket, improving end user latency.
-    if (group.IsEmpty() && !group.backup_job && backup_jobs_enabled_) {
-      group.backup_job = connect_job_factory_->NewConnectJob(group_name,
-                                                             *request,
-                                                             this);
+    if (group.IsEmpty() && !group.connect_backup_job &&
+        connect_backup_jobs_enabled_) {
+      group.connect_backup_job = connect_job_factory_->NewConnectJob(group_name,
+                                                                    *request,
+                                                                     this);
       StartBackupSocketTimer(group_name);
     }
 
@@ -327,7 +332,7 @@ void ClientSocketPoolBaseHelper::OnBackupSocketTimerFired(
   CHECK(group.backup_task);
   group.backup_task = NULL;
 
-  CHECK(group.backup_job);
+  CHECK(group.connect_backup_job);
 
   // If there are no more jobs pending, there is no work to do.
   // If we've done our cleanups correctly, this should not happen.
@@ -345,14 +350,15 @@ void ClientSocketPoolBaseHelper::OnBackupSocketTimerFired(
     return;
   }
 
-  group.backup_job->net_log().AddEvent(NetLog::TYPE_SOCKET_BACKUP_CREATED,
-                                       NULL);
+  group.connect_backup_job->net_log().AddEvent(
+      NetLog::TYPE_SOCKET_BACKUP_CREATED,
+      NULL);
   SIMPLE_STATS_COUNTER("socket.backup_created");
-  int rv = group.backup_job->Connect();
+  int rv = group.connect_backup_job->Connect();
   connecting_socket_count_++;
-  group.jobs.insert(group.backup_job);
-  ConnectJob* job = group.backup_job;
-  group.backup_job = NULL;
+  group.jobs.insert(group.connect_backup_job);
+  ConnectJob* job = group.connect_backup_job;
+  group.connect_backup_job = NULL;
   if (rv != ERR_IO_PENDING)
     OnConnectJobComplete(rv, job);
 }
@@ -495,6 +501,15 @@ void ClientSocketPoolBaseHelper::CleanupIdleSockets(bool force) {
   }
 }
 
+// static
+void ClientSocketPoolBaseHelper::set_connect_backup_jobs_enabled(bool enabled) {
+  g_connect_backup_jobs_enabled = enabled;
+}
+
+void ClientSocketPoolBaseHelper::EnableConnectBackupJobs() {
+  connect_backup_jobs_enabled_ = g_connect_backup_jobs_enabled;
+}
+
 void ClientSocketPoolBaseHelper::IncrementIdleCount() {
   if (++idle_socket_count_ == 1)
     timer_.Start(TimeDelta::FromSeconds(kCleanupInterval), this,
diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h
index 96c78d5..ee727f0 100644
--- a/net/socket/client_socket_pool_base.h
+++ b/net/socket/client_socket_pool_base.h
@@ -239,7 +239,8 @@ class ClientSocketPoolBaseHelper
     return connect_job_factory_->ConnectionTimeout();
   }
 
-  void EnableBackupJobs() { backup_jobs_enabled_ = true; }
+  static void set_connect_backup_jobs_enabled(bool enabled);
+  void EnableConnectBackupJobs();
 
  private:
   friend class base::RefCounted<ClientSocketPoolBaseHelper>;
@@ -271,7 +272,7 @@ class ClientSocketPoolBaseHelper
   struct Group {
     Group()
         : active_socket_count(0),
-          backup_job(NULL),
+          connect_backup_job(NULL),
           backup_task(NULL) {
     }
 
@@ -299,9 +300,9 @@ class ClientSocketPoolBaseHelper
     }
 
     void CleanupBackupJob() {
-      if (backup_job) {
-        delete backup_job;
-        backup_job = NULL;
+      if (connect_backup_job) {
+        delete connect_backup_job;
+        connect_backup_job = NULL;
       }
       if (backup_task) {
         backup_task->Cancel();
@@ -314,7 +315,7 @@ class ClientSocketPoolBaseHelper
     RequestQueue pending_requests;
     int active_socket_count;  // number of active sockets used by clients
     // A backup job in case the connect for this group takes too long.
-    ConnectJob* backup_job;
+    ConnectJob* connect_backup_job;
     CancelableTask* backup_task;
   };
 
@@ -461,7 +462,7 @@ class ClientSocketPoolBaseHelper
   const scoped_ptr<ConnectJobFactory> connect_job_factory_;
 
   // TODO(vandebo) Remove when backup jobs move to TCPClientSocketPool
-  bool backup_jobs_enabled_;
+  bool connect_backup_jobs_enabled_;
 
   // A factory to pin the backup_job tasks.
   ScopedRunnableMethodFactory<ClientSocketPoolBaseHelper> method_factory_;
@@ -595,7 +596,7 @@ class ClientSocketPoolBase {
     return histograms_;
   }
 
-  void EnableBackupJobs() { helper_->EnableBackupJobs(); }
+  void EnableConnectBackupJobs() { helper_->EnableConnectBackupJobs(); }
 
   void Flush() { helper_->Flush(); }
 
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index 6351562..e9a675c 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -419,7 +419,7 @@ class TestClientSocketPool : public ClientSocketPool {
 
   void CleanupTimedOutIdleSockets() { base_.CleanupIdleSockets(false); }
 
-  void EnableBackupJobs() { base_.EnableBackupJobs(); }
+  void EnableConnectBackupJobs() { base_.EnableConnectBackupJobs(); }
 
  private:
   ~TestClientSocketPool() {}
@@ -995,7 +995,7 @@ TEST_F(ClientSocketPoolBaseTest, WaitForStalledSocketAtSocketLimit) {
 // Regression test for http://crbug.com/40952.
 TEST_F(ClientSocketPoolBaseTest, CloseIdleSocketAtSocketLimitDeleteGroup) {
   CreatePool(kDefaultMaxSockets, kDefaultMaxSocketsPerGroup);
-  pool_->EnableBackupJobs();
+  pool_->EnableConnectBackupJobs();
   connect_job_factory_->set_job_type(TestConnectJob::kMockJob);
 
   for (int i = 0; i < kDefaultMaxSockets; ++i) {
@@ -1923,7 +1923,7 @@ TEST_F(ClientSocketPoolBaseTest, DoNotReuseSocketAfterFlush) {
 TEST_F(ClientSocketPoolBaseTest, BackupSocketCancelAtMaxSockets) {
   // Max 4 sockets globally, max 4 sockets per group.
   CreatePool(kDefaultMaxSockets, kDefaultMaxSockets);
-  pool_->EnableBackupJobs();
+  pool_->EnableConnectBackupJobs();
 
   // Create the first socket and set to ERR_IO_PENDING.  This creates a
   // backup job.
diff --git a/net/socket/tcp_client_socket_pool.cc b/net/socket/tcp_client_socket_pool.cc
index c2fedb2..f51196f 100644
--- a/net/socket/tcp_client_socket_pool.cc
+++ b/net/socket/tcp_client_socket_pool.cc
@@ -202,7 +202,7 @@ TCPClientSocketPool::TCPClientSocketPool(
             base::TimeDelta::FromSeconds(kUsedIdleSocketTimeout),
             new TCPConnectJobFactory(client_socket_factory,
                                      host_resolver, net_log)) {
-  base_.EnableBackupJobs();
+  base_.EnableConnectBackupJobs();
 }
 
 TCPClientSocketPool::~TCPClientSocketPool() {}