Set connection timeout in container sync

Container sync might get stuck without a connection timeout if the remote proxy
is not responding.

This patch sets a default timeout of 5.0 seconds for the connection attempt. The
value is much higher than other connection timeouts inside Swift (0.5); however
there might be a much higher latency to the remote peer, thus playing it safe.
There is also a retry if the attempt timed out.

Note that this setting only applies to the connection request itself. Setting
this timeout does not apply when the remote proxy goes away during a request.

Also added a short test to ensure urlopen is called with the timeout value.

Co-Authored-By: Alistair Coles <alistair.coles@hp.com>

Change-Id: Ic08a55157fa91fe1316653781adf4d66eead61bc
Partial-Bug: 1419916

4 files changed, 31 insertions, 4 deletions

diff --git a/etc/container-server.conf-sample b/etc/container-server.conf-sample
index de511368a..6e881d9e0 100644
--- a/etc/container-server.conf-sample
+++ b/etc/container-server.conf-sample
@@ -167,6 +167,9 @@ use = egg:swift#recon
 #
 # Maximum amount of time to spend syncing each container per pass
 # container_time = 60
+#
+# Maximum amount of time in seconds for the connection attempt
+# conn_timeout = 5
 
 # Note: Put it at the beginning of the pipeline to profile all middleware. But
 # it is safer to put this after healthcheck.
diff --git a/swift/container/sync.py b/swift/container/sync.py
index 4bf5fc5c3..0f42de6e9 100644
--- a/swift/container/sync.py
+++ b/swift/container/sync.py
@@ -158,6 +158,7 @@ class ContainerSync(Daemon):
         self._myport = int(conf.get('bind_port', 6001))
         swift.common.db.DB_PREALLOCATION = \
             config_true_value(conf.get('db_preallocation', 'f'))
+        self.conn_timeout = float(conf.get('conn_timeout', 5))
 
     def get_object_ring(self, policy_idx):
         """
@@ -361,7 +362,8 @@ class ContainerSync(Daemon):
                         headers['x-container-sync-key'] = user_key
                     delete_object(sync_to, name=row['name'], headers=headers,
                                   proxy=self.select_http_proxy(),
-                                  logger=self.logger)
+                                  logger=self.logger,
+                                  timeout=self.conn_timeout)
                 except ClientException as err:
                     if err.http_status != HTTP_NOT_FOUND:
                         raise
@@ -434,7 +436,8 @@ class ContainerSync(Daemon):
                     headers['x-container-sync-key'] = user_key
                 put_object(sync_to, name=row['name'], headers=headers,
                            contents=FileLikeIter(body),
-                           proxy=self.select_http_proxy(), logger=self.logger)
+                           proxy=self.select_http_proxy(), logger=self.logger,
+                           timeout=self.conn_timeout)
                 self.container_puts += 1
                 self.logger.increment('puts')
                 self.logger.timing_since('puts.timing', start_time)
diff --git a/test/unit/common/test_internal_client.py b/test/unit/common/test_internal_client.py
index df140ebdd..d4027261d 100644
--- a/test/unit/common/test_internal_client.py
+++ b/test/unit/common/test_internal_client.py
@@ -333,6 +333,24 @@ class TestInternalClient(unittest.TestCase):
         self.assertEquals(3, client.sleep_called)
         self.assertEquals(4, client.tries)
 
+    def test_base_request_timeout(self):
+        # verify that base_request passes timeout arg on to urlopen
+        body = {"some": "content"}
+
+        class FakeConn(object):
+            def read(self):
+                return json.dumps(body)
+
+        for timeout in (0.0, 42.0, None):
+            mocked_func = 'swift.common.internal_client.urllib2.urlopen'
+            with mock.patch(mocked_func) as mock_urlopen:
+                mock_urlopen.side_effect = [FakeConn()]
+                sc = internal_client.SimpleClient('http://0.0.0.0/')
+                _, resp_body = sc.base_request('GET', timeout=timeout)
+                mock_urlopen.assert_called_once_with(mock.ANY, timeout=timeout)
+                # sanity check
+                self.assertEquals(body, resp_body)
+
     def test_make_request_method_path_headers(self):
         class InternalClient(internal_client.InternalClient):
             def __init__(self):
diff --git a/test/unit/container/test_sync.py b/test/unit/container/test_sync.py
index 645c7935c..aa5cebc28 100644
--- a/test/unit/container/test_sync.py
+++ b/test/unit/container/test_sync.py
@@ -652,7 +652,7 @@ class TestContainerSync(unittest.TestCase):
             fake_logger = FakeLogger()
 
             def fake_delete_object(path, name=None, headers=None, proxy=None,
-                                   logger=None):
+                                   logger=None, timeout=None):
                 self.assertEquals(path, 'http://sync/to/path')
                 self.assertEquals(name, 'object')
                 if realm:
@@ -666,6 +666,7 @@ class TestContainerSync(unittest.TestCase):
                         {'x-container-sync-key': 'key', 'x-timestamp': '1.2'})
                 self.assertEquals(proxy, 'http://proxy')
                 self.assertEqual(logger, fake_logger)
+                self.assertEqual(timeout, 5.0)
 
             sync.delete_object = fake_delete_object
             cs = sync.ContainerSync({}, container_ring=FakeRing())
@@ -759,7 +760,8 @@ class TestContainerSync(unittest.TestCase):
             fake_logger = FakeLogger()
 
             def fake_put_object(sync_to, name=None, headers=None,
-                                contents=None, proxy=None, logger=None):
+                                contents=None, proxy=None, logger=None,
+                                timeout=None):
                 self.assertEquals(sync_to, 'http://sync/to/path')
                 self.assertEquals(name, 'object')
                 if realm:
@@ -780,6 +782,7 @@ class TestContainerSync(unittest.TestCase):
                 self.assertEquals(contents.read(), 'contents')
                 self.assertEquals(proxy, 'http://proxy')
                 self.assertEqual(logger, fake_logger)
+                self.assertEqual(timeout, 5.0)
 
             sync.put_object = fake_put_object