launchpad-reviewers team mailing list archive

[Stuart Bishop <stuart.bishop@xxxxxxxxxxxxx>]

Stuart Bishop has proposed merging lp:~stub/launchpad/librarian-disco into lp:launchpad with lp:~stub/launchpad/branch-rewrite as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #840098 in Launchpad itself: "Librarian should return 503 pages during database outages"
  https://bugs.launchpad.net/launchpad/+bug/840098

For more details, see:
https://code.launchpad.net/~stub/launchpad/librarian-disco/+merge/73934

= Summary =

Tests that confirm the Librarian survives database outages, and make it emit the correct response code while we are there.

== Proposed fix ==

== Pre-implementation notes ==

== Implementation details ==

== Tests ==

== Demo and Q/A ==


= Launchpad lint =

Just lint from dependant branches. Cleaned up web.py.
-- 
https://code.launchpad.net/~stub/launchpad/librarian-disco/+merge/73934
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stub/launchpad/librarian-disco into lp:launchpad.

=== added file 'lib/canonical/librarian/tests/test_db_outage.py'
--- lib/canonical/librarian/tests/test_db_outage.py	1970-01-01 00:00:00 +0000
+++ lib/canonical/librarian/tests/test_db_outage.py	2011-09-03 05:17:27 +0000
@@ -0,0 +1,109 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test behavior of the Librarian during a database outage.
+
+Database outages happen by accident and during fastdowntime deployments."""
+
+__metaclass__ = type
+
+from cStringIO import StringIO
+import urllib2
+
+from fixtures import Fixture
+
+from canonical.librarian.client import LibrarianClient
+from canonical.librarian.testing.server import LibrarianServerFixture
+from canonical.testing.layers import (
+    BaseLayer,
+    DatabaseFunctionalLayer,
+    )
+from lp.testing import TestCase
+from lp.testing.fixture import PGBouncerFixture
+
+
+class PGBouncerLibrarianLayer(DatabaseFunctionalLayer):
+    """Custom layer for TestLibrarianDBOutage.
+
+    We are using a custom layer instead of standard setUp/tearDown to
+    avoid the lengthy Librarian startup time, and to cope with undoing
+    changes made to BaseLayer.config_fixture to allow access to the
+    Librarian we just started up.
+    """
+    pgbouncer_fixture = None
+    librarian_fixture = None
+
+    @classmethod
+    def setUp(cls):
+        # Fixture to hold other fixtures.
+        cls._fixture = Fixture()
+        cls._fixture.setUp()
+
+        cls.pgbouncer_fixture = PGBouncerFixture()
+        # Install the PGBouncer fixture so we shut it down to
+        # create database outages.
+        cls._fixture.useFixture(cls.pgbouncer_fixture)
+
+        # Bring up the Librarian, which will be connecting via
+        # pgbouncer.
+        cls.librarian_fixture = LibrarianServerFixture(
+            BaseLayer.config_fixture)
+        cls._fixture.useFixture(cls.librarian_fixture)
+
+    @classmethod
+    def tearDown(cls):
+        cls.pgbouncer_fixture = None
+        cls.librarian_fixture = None
+        cls._fixture.cleanUp()
+
+    @classmethod
+    def testSetUp(cls):
+        cls.pgbouncer_fixture.start()
+
+    @classmethod
+    def testTearDown(cls):
+        cls.pgbouncer_fixture.start()
+
+
+class TestLibrarianDBOutage(TestCase):
+    layer = PGBouncerLibrarianLayer
+
+    def setUp(self):
+        super(TestLibrarianDBOutage, self).setUp()
+        self.pgbouncer = PGBouncerLibrarianLayer.pgbouncer_fixture
+        self.client = LibrarianClient()
+
+        # Add a file to the Librarian so we can download it.
+        data = 'whatever'
+        self.url = self.client.remoteAddFile(
+            'foo.txt', len(data), StringIO(data), 'text/plain')
+
+    def get_error_code(self):
+        # We need to talk to every Librarian thread to ensure all the
+        # Librarian database connections are in a known state.
+        # XXX StuartBishop 2011-09-01 bug=840046: 20 might be overkill
+        # for the test run, but we have no real way of knowing how many
+        # connections are in use.
+        num_librarian_threads = 20
+        codes = set()
+        for count in range(num_librarian_threads):
+            try:
+                urllib2.urlopen(self.url).read()
+                codes.add(200)
+            except urllib2.HTTPError, error:
+                codes.add(error.code)
+        self.assertTrue(len(codes) == 1, 'Mixed responses: %s' % str(codes))
+        return codes.pop()
+
+    def test_outage(self):
+        # Everything should be working fine to start with.
+        self.assertEqual(self.get_error_code(), 200)
+
+        # When the outage kicks in, we start getting 503 responses
+        # instead of 200 and 404s.
+        self.pgbouncer.stop()
+        self.assertEqual(self.get_error_code(), 503)
+
+        # When the outage is over, things are back to normal.
+        self.pgbouncer.start()
+        self.assertEqual(self.get_error_code(), 200)

=== modified file 'lib/canonical/librarian/web.py'
--- lib/canonical/librarian/web.py	2011-03-23 16:28:51 +0000
+++ lib/canonical/librarian/web.py	2011-09-03 05:17:27 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -7,6 +7,7 @@
 import time
 from urlparse import urlparse
 
+from storm.exceptions import DisconnectionError
 from twisted.python import log
 from twisted.web import resource, static, util, server, proxy
 from twisted.internet.threads import deferToThread
@@ -26,7 +27,8 @@
         <h1>Launchpad Librarian</h1>
         <p>
         http://librarian.launchpad.net/ is a
-        file repository used by <a href="https://launchpad.net/";>Launchpad</a>.
+        file repository used by
+        <a href="https://launchpad.net/";>Launchpad</a>.
         </p>
         <p><small>Copyright 2004-2009 Canonical Ltd.</small></p>
         <!-- kthxbye. -->
@@ -107,7 +109,8 @@
 
         token = request.args.get('token', [None])[0]
         path = request.path
-        deferred = deferToThread(self._getFileAlias, self.aliasID, token, path)
+        deferred = deferToThread(
+            self._getFileAlias, self.aliasID, token, path)
         deferred.addCallback(
                 self._cb_getFileAlias, filename, request
                 )
@@ -125,11 +128,19 @@
             raise NotFound
 
     def _eb_getFileAlias(self, failure):
-        failure.trap(NotFound)
-        return fourOhFour
+        err = failure.trap(NotFound, DisconnectionError)
+        if err == DisconnectionError:
+            return resource.ErrorPage(
+                503, 'Database unavailable',
+                'A required database is unavailable.\n'
+                'See http://identi.ca/launchpadstatus '
+                'for maintenance and outage notifications.')
+        else:
+            return fourOhFour
 
     def _cb_getFileAlias(
-            self, (dbcontentID, dbfilename, mimetype, date_created, restricted),
+            self,
+            (dbcontentID, dbfilename, mimetype, date_created, restricted),
             filename, request
             ):
         # Return a 404 if the filename in the URL is incorrect. This offers
@@ -167,6 +178,7 @@
 
 class File(static.File):
     isLeaf = True
+
     def __init__(
         self, contentType, encoding, modification_time, *args, **kwargs):
         # Have to convert the UTC datetime to POSIX timestamp (localtime)

=== modified file 'lib/canonical/testing/layers.py'
--- lib/canonical/testing/layers.py	2011-08-19 13:58:57 +0000
+++ lib/canonical/testing/layers.py	2011-09-03 05:17:27 +0000
@@ -1502,6 +1502,7 @@
 class LaunchpadTestSetup(PgTestSetup):
     template = 'launchpad_ftest_template'
     dbuser = 'launchpad'
+    host = 'localhost'
 
 
 class LaunchpadZopelessLayer(LaunchpadScriptLayer):

=== modified file 'lib/lp/testing/fixture.py'
--- lib/lp/testing/fixture.py	2011-09-03 05:17:26 +0000
+++ lib/lp/testing/fixture.py	2011-09-03 05:17:27 +0000
@@ -85,6 +85,10 @@
             self.users[section_name] = 'trusted'
             self.users[section_name + '_ro'] = 'trusted'
         self.users[os.environ['USER']] = 'trusted'
+        self.users['pgbouncer'] = 'trusted'
+
+        # Administrative access is useful for debugging.
+        self.admin_users = ['launchpad', 'pgbouncer', os.environ['USER']]
 
     def setUp(self):
         super(PGBouncerFixture, self).setUp()