launchpad-reviewers team mailing list archive

[Colin Watson <mp+411026@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:librarian-swift-timeout into launchpad:master.

Commit message:
Add a timeout to librarian Swift connections

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1948711 in Launchpad itself: "librarian processes get wedged"
  https://bugs.launchpad.net/launchpad/+bug/1948711

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/411026

Otherwise librarian servers progressively deteriorate and eventually hang if Swift connections are failing.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:librarian-swift-timeout into launchpad:master.

diff --git a/lib/lp/services/config/schema-lazr.conf b/lib/lp/services/config/schema-lazr.conf
index 668b8cf..5201354 100644
--- a/lib/lp/services/config/schema-lazr.conf
+++ b/lib/lp/services/config/schema-lazr.conf
@@ -1338,6 +1338,10 @@ old_os_tenant_name: none
 # datatype: string
 old_os_auth_version: 2.0
 
+# Time in seconds to wait for a response from Swift.
+# datatype: integer
+swift_timeout: 15
+
 
 # Mailman configuration.  Most of this is configured in
 # https://git.launchpad.net/lp-mailman instead; the entries here are only
diff --git a/lib/lp/services/librarianserver/swift.py b/lib/lp/services/librarianserver/swift.py
index 1602cd3..58cb8df 100644
--- a/lib/lp/services/librarianserver/swift.py
+++ b/lib/lp/services/librarianserver/swift.py
@@ -426,6 +426,7 @@ class ConnectionPool:
             key=self.os_password,
             tenant_name=self.os_tenant_name,
             auth_version=self.os_auth_version,
+            timeout=float(config.librarian_server.swift_timeout),
             )
 
 
diff --git a/lib/lp/services/librarianserver/tests/test_swift.py b/lib/lp/services/librarianserver/tests/test_swift.py
index cbc65ce..2fbff88 100644
--- a/lib/lp/services/librarianserver/tests/test_swift.py
+++ b/lib/lp/services/librarianserver/tests/test_swift.py
@@ -363,6 +363,15 @@ class TestFeedSwift(TestCase):
             finally:
                 swift_client.close()
 
+    def test_swift_timeout(self):
+        # The librarian's Swift connections honour the configured timeout.
+        self.pushConfig(
+            'librarian_server', os_username='timeout', swift_timeout=0.1)
+        swift_client = self.swift_fixture.connect()
+        self.assertRaises(
+            swiftclient.ClientException,
+            swift_client.get_object, 'container', 'name')
+
 
 class TestHashStream(TestCase):
     layer = BaseLayer
diff --git a/lib/lp/testing/swift/fakeswift.py b/lib/lp/testing/swift/fakeswift.py
index 38f4f6d..a96fcf9 100644
--- a/lib/lp/testing/swift/fakeswift.py
+++ b/lib/lp/testing/swift/fakeswift.py
@@ -135,6 +135,8 @@ class FakeKeystone(resource.Resource):
         if tenant_name not in self.root.tenants:
             request.setResponseCode(http.FORBIDDEN)
             return b""
+        if username == "timeout":
+            time.sleep(60)
         if username not in self.users:
             request.setResponseCode(http.FORBIDDEN)
             return b""