launchpad-reviewers team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

Gary Poster has proposed merging lp:~gary/launchpad/bug-687951 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #687951 Large number of DBs left around after test suite runs in buildbot
  https://bugs.launchpad.net/bugs/687951


This is an expedient solution to the problem described in bug 687951--or put another way, it is a good solution to the buildbot and operational problem, while not addressing the underlying Launchpad problem.  I will create another bug for the Launchpad problem, which Robert addressed in the IRC conversation I quoted in that bug's comments.

The test_on_merge.py is operational code for which we do not have tests.  To demonstrate the way this patch works, do the following.

1) See if your machine already has some of the problematic old databases.  Run psql -l and you may see database names of the form launchpad_ftest_NUMBER, like lines 10 through 43 of https://pastebin.canonical.com/40714/ .  If you don't have any, you can create some for the test using psql.  "create database launchpad_ftest_30322;" will give you one to work with.

2) run test_on_merge.py, with some argument to limit the tests run.  For instance, "./test_on_merge.py -t foobar" will not run any tests.  There's probably a nicer way to do this, but I'm not worrying about it right now.  When you do this, the first one or more lines will look like this: "Dropped old ftest database launchpad_ftest_30322.  This should not happen.  See bug 687951."  After test_on_merge.py runs, psql -l will show you that the databases have been dropped.
-- 
https://code.launchpad.net/~gary/launchpad/bug-687951/+merge/44300
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gary/launchpad/bug-687951 into lp:launchpad.

=== modified file 'test_on_merge.py'
--- test_on_merge.py	2010-09-28 12:37:55 +0000
+++ test_on_merge.py	2010-12-20 23:18:07 +0000
@@ -8,6 +8,7 @@
 
 import sys, time
 import os, errno
+from psycopg2.extensions import ISOLATION_LEVEL_AUTOCOMMIT
 import tabnanny
 from StringIO import StringIO
 import psycopg2
@@ -34,6 +35,7 @@
 
     Returns 1 on error, otherwise it returns the testrunner's exit code.
     """
+    cleanup_old_ftest_databases() # See bug 687951.
     if setup_test_database() != 0:
         return 1
 
@@ -224,6 +226,32 @@
     return rv
 
 
+def cleanup_old_ftest_databases():
+    """Kill old ftest databases."""
+    conn = psycopg2.connect('dbname=template1')
+    conn.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT)
+    cur = conn.cursor()
+    try:
+        cur.execute(r"""
+            SELECT d.datname
+            FROM pg_catalog.pg_database d
+            WHERE d.datname ~ '^launchpad_ftest_[0-9]+$';""")
+        old_databases = cur.fetchall()
+        for (database_name,) in old_databases:
+            print ("Dropped old ftest database %s.  "
+                   "This should not happen.  See bug 687951.") % (
+                   database_name,)
+            # We are not doing db-level parameterization because it does not work
+            # (is not designed for) schema entities.
+            cur.execute(r"""
+                DROP DATABASE %s;""" % (database_name,))
+    finally:
+        cur.close()
+        del cur
+        conn.close()
+        del conn
+
+
 def cleanup_hung_testrunner(process):
     """Kill and clean up the testrunner process and its children."""
     print