launchpad-reviewers team mailing list archive

[Stuart Bishop <stuart.bishop@xxxxxxxxxxxxx>]

Stuart Bishop has proposed merging lp:~stub/launchpad/bug-840068-librarian-dbcheck into lp:launchpad with lp:~stub/launchpad/trivial as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #840068 in Launchpad itself: "code import workers failing due to Librarian client remoteAddFile check"
  https://bugs.launchpad.net/launchpad/+bug/840068

For more details, see:
https://code.launchpad.net/~stub/launchpad/bug-840068-librarian-dbcheck/+merge/73930

= Summary =

On remoteAddFile(), the Librarian confirms that the database the client was hoping to get the file added to iss the database the Librarian is actually connected to.

Unfortunately, there are two methods of determining the database name. The Librarian used introspection by issuing a 'SELECT current_database();' statement. The librarian client code however sniffed the connection string, as some clients do not have access to the database at all. With the addition of pgbouncer, these methods started giving different results, as we are using database aliases to allow us to easily change the role of databases without having to reconfigure clients.

== Proposed fix ==

Check both the connection string (which will return launchpad_prod_master) and the real name (launchpad_prod_3 currently), and let a match on either pass.

== Pre-implementation notes ==

== Implementation details ==

== Tests ==

== Demo and Q/A ==


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/librarian/storage.py
-- 
https://code.launchpad.net/~stub/launchpad/bug-840068-librarian-dbcheck/+merge/73930
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stub/launchpad/bug-840068-librarian-dbcheck into lp:launchpad.

=== modified file 'lib/canonical/librarian/storage.py'
--- lib/canonical/librarian/storage.py	2010-09-24 15:40:49 +0000
+++ lib/canonical/librarian/storage.py	2011-09-03 03:36:29 +0000
@@ -11,6 +11,8 @@
 
 from zope.component import getUtility
 
+from canonical.config import dbconfig
+from canonical.database.postgresql import ConnectionString
 from canonical.launchpad.webapp.interfaces import (
         IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)
 from lp.services.database import write_transaction
@@ -123,12 +125,21 @@
             # If the client told us the name of the database it's using,
             # check that it matches.
             if self.databaseName is not None:
+                # Per Bug #840068, there are two methods of getting the
+                # database name (connection string and db
+                # introspection), and they can give different results
+                # due to pgbouncer database aliases. Lets check both,
+                # and if either patch pass.
+                config_dbname = ConnectionString(
+                    dbconfig.rw_main_master).dbname
+
                 store = getUtility(IStoreSelector).get(
                         MAIN_STORE, DEFAULT_FLAVOR)
                 result = store.execute("SELECT current_database()")
-                databaseName = result.get_one()[0]
-                if self.databaseName != databaseName:
-                    raise WrongDatabaseError(self.databaseName, databaseName)
+                real_dbname = result.get_one()[0]
+                if self.databaseName not in (config_dbname, real_dbname):
+                    raise WrongDatabaseError(
+                        self.databaseName, (config_dbname, real_dbname))
 
             self.debugLog.append(
                 'database name %r ok' % (self.databaseName, ))