Merge lp:~stub/launchpad/garbo into lp:launchpad/db-devel

[Stuart Bishop]

[Stuart Bishop (stub)]

Addresses:

  Bug #351363: DBLoopTuner often cannot detect long running transactions
  Bug #348874: OpenIDAssociations need to be garbage collected

We now use a SECURITY DEFINER wrapper around pg_stat_activity to allow DBLoopTuner to query this information even when not connected as a database superuser.

The OpenIDAssociation and OpenIDConsumerAssociation tables are now garbage collected.

Scripts using the auth Store now connect as a sane database user.

When the test infrastructure is used to reset the Zopeless connection settings, ZStorm is reset and all Stores closed. After changing the connection settings, Stores need to be refetched but they will be connected using the correct database user.

[Brad Crittenden (bac)]

Hi Stuart,

The branch looks good. I don't have anything to add but do ask for
you to add a comment on one test -- it's probably just the case of me
being paranoid.

Also the logic in bug 348874 seems to be wrong w.r.t. '>' vs '<' but
is switched and correct in theimplementation here. For completeness
would you make a note on that bug?

> === modified file 'lib/canonical/launchpad/scripts/tests/test_garbo.py'
> --- lib/canonical/launchpad/scripts/tests/test_garbo.py 2009-03-27 11:15:14 +0000
> +++ lib/canonical/launchpad/scripts/tests/test_garbo.py 2009-04-03 07:16:43 +0000
> @@ -12,6 +12,7 @@
> from pytz import UTC
> from storm.locals import Min
> import transaction
> +from zope.component import getUtility
>
> from canonical.launchpad.database.codeimportresult import CodeImportResult
> from canonical.launchpad.database.oauth import OAuthNonce
> @@ -21,9 +22,12 @@
> CodeImportResultStatus)
> from canonical.launchpad.testing import TestCase
> from canonical.launchpad.scripts.garbo import (
> - DailyDatabaseGarbageCollector, HourlyDatabaseGarbageCollector)
> + DailyDatabaseGarbageCollector, HourlyDatabaseGarbageCollector,
> + OpenIDAssociationPruner, OpenIDConsumerAssociationPruner)
> from canonical.launchpad.scripts.tests import run_script
> from canonical.launchpad.scripts.logger import QuietFakeLogger
> +from canonical.launchpad.webapp.interfaces import (
> + IStoreSelector, MASTER_FLAVOR)
> from canonical.testing.layers import (
> DatabaseLayer, LaunchpadScriptLayer, LaunchpadZopelessLayer)
>
> @@ -57,20 +61,21 @@
> self.runDaily()
> self.runHourly()
>
> - def runDaily(self):
> - LaunchpadZopelessLayer.switchDbUser('garbo-daily')
> + def runDaily(self, maximum_chunk_size=2):
> + LaunchpadZopelessLayer.switchDbUser('garbo_daily')
> collector = DailyDatabaseGarbageCollector(test_args=[])
> + collector._maximum_chunk_size = maximum_chunk_size
> collector.logger = QuietFakeLogger()
> collector.main()
>
> - def runHourly(self):
> - LaunchpadZopelessLayer.switchDbUser('garbo-hourly')
> + def runHourly(self, maximum_chunk_size=2):
> + LaunchpadZopelessLayer.switchDbUser('garbo_hourly')
> collector = HourlyDatabaseGarbageCollector(test_args=[])
> + collector._maximum_chunk_size = maximum_chunk_size
> collector.logger = QuietFakeLogger()
> collector.main()
>
> def test_OAuthNoncePruner(self):
> - store = IMasterStore(OAuthNonce)
> now = datetime.utcnow().replace(tzinfo=UTC)
> timestamps = [
> now - timedelta(days=2), # Garbage
> @@ -79,6 +84,7 @@
> now, # Not garbage
> ]
> LaunchpadZopelessLayer.switchDbUser('testadmin')
> + store = IMasterStore(OAuthNonce)
>
> # Make sure we start with 0 nonces.
> self.failUnlessEqual(store.find(OAuthNonce).count(), 0)
> @@ -93,7 +99,9 @@
> # Make sure we have 4 nonces now.
> self.failUnlessEqual(store.find(OAuthNonce).count(), 4)
>
> - self.runHourly()
> + self.runHourly(maximum_chunk_size=60) # 1 minute maxi...

review: Approve (code)

[Brad Crittenden (bac)]

Hi Stuart,

The branch looks good. I don't have anything to add but do ask for
you to add a comment on one test -- it's probably just the case of me
being paranoid.

Also the logic in bug 348874 seems to be wrong w.r.t. '>' vs '<' but
is switched and correct in theimplementation here. For completeness
would you make a note on that bug?

> === modified file 'lib/canonical/launchpad/scripts/tests/test_garbo.py'
> --- lib/canonical/launchpad/scripts/tests/test_garbo.py 2009-03-27 11:15:14 +0000
> +++ lib/canonical/launchpad/scripts/tests/test_garbo.py 2009-04-03 07:16:43 +0000
> @@ -12,6 +12,7 @@
> from pytz import UTC
> from storm.locals import Min
> import transaction
> +from zope.component import getUtility
>
> from canonical.launchpad.database.codeimportresult import CodeImportResult
> from canonical.launchpad.database.oauth import OAuthNonce
> @@ -21,9 +22,12 @@
> CodeImportResultStatus)
> from canonical.launchpad.testing import TestCase
> from canonical.launchpad.scripts.garbo import (
> - DailyDatabaseGarbageCollector, HourlyDatabaseGarbageCollector)
> + DailyDatabaseGarbageCollector, HourlyDatabaseGarbageCollector,
> + OpenIDAssociationPruner, OpenIDConsumerAssociationPruner)
> from canonical.launchpad.scripts.tests import run_script
> from canonical.launchpad.scripts.logger import QuietFakeLogger
> +from canonical.launchpad.webapp.interfaces import (
> + IStoreSelector, MASTER_FLAVOR)
> from canonical.testing.layers import (
> DatabaseLayer, LaunchpadScriptLayer, LaunchpadZopelessLayer)
>
> @@ -57,20 +61,21 @@
> self.runDaily()
> self.runHourly()
>
> - def runDaily(self):
> - LaunchpadZopelessLayer.switchDbUser('garbo-daily')
> + def runDaily(self, maximum_chunk_size=2):
> + LaunchpadZopelessLayer.switchDbUser('garbo_daily')
> collector = DailyDatabaseGarbageCollector(test_args=[])
> + collector._maximum_chunk_size = maximum_chunk_size
> collector.logger = QuietFakeLogger()
> collector.main()
>
> - def runHourly(self):
> - LaunchpadZopelessLayer.switchDbUser('garbo-hourly')
> + def runHourly(self, maximum_chunk_size=2):
> + LaunchpadZopelessLayer.switchDbUser('garbo_hourly')
> collector = HourlyDatabaseGarbageCollector(test_args=[])
> + collector._maximum_chunk_size = maximum_chunk_size
> collector.logger = QuietFakeLogger()
> collector.main()
>
> def test_OAuthNoncePruner(self):
> - store = IMasterStore(OAuthNonce)
> now = datetime.utcnow().replace(tzinfo=UTC)
> timestamps = [
> now - timedelta(days=2), # Garbage
> @@ -79,6 +84,7 @@
> now, # Not garbage
> ]
> LaunchpadZopelessLayer.switchDbUser('testadmin')
> + store = IMasterStore(OAuthNonce)
>
> # Make sure we start with 0 nonces.
> self.failUnlessEqual(store.find(OAuthNonce).count(), 0)
> @@ -93,7 +99,9 @@
> # Make sure we have 4 nonces now.
> self.failUnlessEqual(store.find(OAuthNonce).count(), 4)
>
> - self.runHourly()
> + self.runHourly(maximum_chunk_size=60) # 1 minute maxi...

review: Approve (code)

[Stuart Bishop (stub)]

On Tue, Apr 7, 2009 at 8:11 PM, Brad Crittenden <email address hidden> wrote:

>> +    def test_OpenIDAssociationPruner(self, pruner=OpenIDAssociationPruner):
>> +        store_name = pruner.store_name
>> +        table_name = pruner.table_name
>> +        LaunchpadZopelessLayer.switchDbUser('testadmin')
>> +        store_selector = getUtility(IStoreSelector)
>> +        store = store_selector.get(store_name, MASTER_FLAVOR)
>> +        now = time.time()
>> +        # Create some associations in the past with lifetimes
>> +        for delta in range(0, 20):
>> +            store.execute("""
>> +                INSERT INTO %s (server_url, handle, issued, lifetime)
>> +                VALUES (%s, %s, %d, %d)
>> +                """ % (table_name, str(delta), str(delta), now-10, delta))
>> +        transaction.commit()
>> +
>> +        num_expired = store.execute("""
>> +            SELECT COUNT(*) FROM %s
>> +            WHERE issued + lifetime < %f
>> +            """ % (table_name, now)).get_one()[0]
>> +        self.failUnless(num_expired > 0)
>> +
>> +        self.runHourly()
>> +
>> +        LaunchpadZopelessLayer.switchDbUser('testadmin')
>> +        store = store_selector.get(store_name, MASTER_FLAVOR)
>> +        num_expired = store.execute("""
>> +            SELECT COUNT(*) FROM %s
>> +            WHERE issued + lifetime < %f
>> +            """ % (table_name, now)).get_one()[0]
>> +        self.failUnlessEqual(num_expired, 0)
>
> This test depends on all three parts completing within one second, the
> granularity of your delta.  While I'm having a hard time envisioning
> that being a problem it does seem like a potential spot for random
> failure.  Perhaps if you just state the assumption in a comment that
> will suffice and be a big clue if it should ever fail.

I thought I'd fixed the logic to stop that already. I have a
consistent 'now' at the top of the test. The first test checks that
there is at least one item expired at time 'now'. The second check
tests that there are 0 items expired at time 'now'. The only change I
see if things pause or run slow is self.runHourly(), and this just
means more items might get expired. The final check doesn't care,
because it is just ensuring that all the items at the test start time
where expired and it couldn't care less if more where expired.

So if my logic is correct (it is 4:30am), the problem is that I'm not
checking that items that should not have been expired have not been
expired.

--
Stuart Bishop <email address hidden>
http://www.stuartbishop.net/

[Stuart Bishop (stub)]

On Wed, Apr 8, 2009 at 4:42 AM, Stuart Bishop <email address hidden> wrote:

> I thought I'd fixed the logic to stop that already. I have a
> consistent 'now' at the top of the test. The first test checks that
> there is at least one item expired at time 'now'. The second check
> tests that there are 0 items expired at time 'now'.  The only change I
> see if things pause or run slow is self.runHourly(), and this just
> means more items might get expired. The final check doesn't care,
> because it is just ensuring that all the items at the test start time
> where expired and it couldn't care less if more where expired.
>
> So if my logic is correct (it is 4:30am), the problem is that I'm not
> checking that items that should not have been expired have not been
> expired.

I've commented this test better and added the extra 'make sure we didn't just trash everything' check.

--
Stuart Bishop <email address hidden>
http://www.stuartbishop.net/

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2009-05-08 02:33:14 +0000
+++ database/schema/security.cfg	2009-05-10 03:41:25 +0000
@@ -11,6 +11,7 @@
 [public]
 # The public role is automatically granted to all users by PostgreSQL
 type=group
+public.activity()                          = EXECUTE
 public.person_sort_key(text, text)         = EXECUTE
 public.debversion_sort_key(text)           = EXECUTE
 public.null_count(anyarray)                = EXECUTE
@@ -98,6 +99,9 @@
 public.personlanguage                   = SELECT
 public.teammembership                   = SELECT
 public.teamparticipation                = SELECT
+# XXX 2009-05-07 stub bug=373252: SELECT and DELETE permissions required
+# for garbo.py. INSERT permission needed for the tests.
+public.openidassociation                = SELECT, INSERT, DELETE
 
 [launchpad_main]
 # lpmain replication set access from the main Z3 application.
@@ -1613,7 +1617,7 @@
 public.libraryfiledownloadcount         = SELECT, INSERT, UPDATE, DELETE
 
 [garbo]
-# garbo-hourly and garbo-daily script permissions. We define the
+# garbo_hourly and garbo_daily script permissions. We define the
 # permissions here in this group instead of in the users, so tasks can
 # be shuffled around between the daily and hourly sections without
 # changing DB permissions.
@@ -1621,6 +1625,8 @@
 groups=script
 public.codeimportresult                 = SELECT, DELETE
 public.oauthnonce                       = SELECT, DELETE
+public.openidassociation                = SELECT, DELETE
+public.openidconsumerassociation        = SELECT, DELETE
 public.openidconsumernonce              = SELECT, DELETE
 public.revisioncache                    = SELECT, DELETE
 public.person                           = SELECT
@@ -1628,11 +1634,11 @@
 public.hwsubmission                     = SELECT, UPDATE
 public.mailinglistsubscription          = SELECT, DELETE
 
-[garbo-daily]
+[garbo_daily]
 type=user
 groups=garbo
 
-[garbo-hourly]
+[garbo_hourly]
 type=user
 groups=garbo
 
 
=== modified file 'database/schema/trusted.sql'
--- database/schema/trusted.sql	2009-04-24 14:43:14 +0000
+++ database/schema/trusted.sql	2009-05-10 03:41:25 +0000
@@ -48,10 +48,52 @@
 
 COMMENT ON FUNCTION null_count(anyarray) IS 'Return the number of NULLs in the first row of the given array.';
 
+
+CREATE OR REPLACE FUNCTION replication_lag() RETURNS interval
+LANGUAGE plpgsql STABLE SECURITY DEFINER AS
+$$
+    DECLARE
+        v_lag interval;
+    BEGIN
+        SELECT INTO v_lag max(st_lag_time) FROM _sl.sl_status;
+        RETURN v_lag;
+    -- Slony-I not installed here - non-replicated setup.
+    EXCEPTION
+        WHEN invalid_schema_name THEN
+            RETURN NULL;
+        WHEN undefined_table THEN
+            RETURN NULL;
+    END;
+$$;
+
+COMMENT ON FUNCTION replication_lag() IS
+'Returns the worst lag time known to this node in our cluster, or NULL if not a replicated installation.';
+
+
+CREATE OR REPLACE FUNCTION activity()
+RETURNS SETOF pg_catalog.pg_stat_activity
+LANGUAGE SQL VOLATILE SECURITY DEFINER AS
+$$
+    SELECT
+        datid, datname, procpid, usesysid, usename,
+        CASE
+            WHEN current_query LIKE '<IDLE>%'
+                THEN current_query
+            ELSE
+                NULL
+        END AS current_query,
+        waiting, xact_start, query_start,
+        backend_start, client_addr, client_port
+    FROM pg_catalog.pg_stat_activity;
+$$;
+
+COMMENT ON FUNCTION activity() IS
+'SECURITY DEFINER wrapper around pg_stat_activity allowing unprivileged users to access most of its information.';
+
+
 /* This is created as a function so the same definition can be used with
     many tables
 */
-
 CREATE OR REPLACE FUNCTION valid_name(text) RETURNS boolean
 LANGUAGE plpythonu IMMUTABLE RETURNS NULL ON NULL INPUT AS
 $$
@@ -858,27 +900,6 @@
 'AFTER UPDATE trigger on BugAffectsPerson maintaining the Bug.users_affected_count column';
 
 
-CREATE OR REPLACE FUNCTION replication_lag() RETURNS interval
-LANGUAGE plpgsql STABLE SECURITY DEFINER AS
-$$
-    DECLARE
-        v_lag interval;
-    BEGIN
-        SELECT INTO v_lag max(st_lag_time) FROM _sl.sl_status;
-        RETURN v_lag;
-    -- Slony-I not installed here - non-replicated setup.
-    EXCEPTION
-        WHEN invalid_schema_name THEN
-            RETURN NULL;
-        WHEN undefined_table THEN
-            RETURN NULL;
-    END;
-$$;
-
-COMMENT ON FUNCTION replication_lag() IS
-'Returns the worst lag time known to this node in our cluster, or NULL if not a replicated installation.';
-
-
 CREATE OR REPLACE FUNCTION set_bugtask_date_milestone_set() RETURNS TRIGGER
 LANGUAGE plpgsql AS
 $$
 
=== modified file 'lib/canonical/database/sqlbase.py'
--- lib/canonical/database/sqlbase.py	2009-04-17 10:32:16 +0000
+++ lib/canonical/database/sqlbase.py	2009-05-10 03:41:25 +0000
@@ -5,6 +5,7 @@
 import warnings
 from datetime import datetime
 import re
+from textwrap import dedent
 
 import psycopg2
 from psycopg2.extensions import (
@@ -293,15 +294,27 @@
         if dbuser is None:
             dbuser = config.launchpad.dbuser
 
-        # Construct a config fragment:
-        overlay = '[database]\n'
-        overlay += 'main_master: %s\n' % connection_string
-        overlay += 'isolation_level: %s\n' % {
+        isolation_level = {
             ISOLATION_LEVEL_AUTOCOMMIT: 'autocommit',
             ISOLATION_LEVEL_READ_COMMITTED: 'read_committed',
             ISOLATION_LEVEL_SERIALIZABLE: 'serializable'}[isolation]
+
+        # Construct a config fragment:
+        overlay = dedent("""\
+            [database]
+            main_master: %(connection_string)s
+            auth_master: %(connection_string)s
+            isolation_level: %(isolation_level)s
+            """ % vars())
+
         if dbuser:
-            overlay += '\n[launchpad]\ndbuser: %s\n' % dbuser
+            # XXX 2009-05-07 stub bug=373252: Scripts should not be connecting
+            # as the launchpad_auth database user.
+            overlay += dedent("""\
+                [launchpad]
+                dbuser: %(dbuser)s
+                auth_dbuser: launchpad_auth
+                """ % vars())
 
         if cls._installed is not None:
             if cls._config_overlay != overlay:
 
=== modified file 'lib/canonical/launchpad/doc/hwdb.txt'
--- lib/canonical/launchpad/doc/hwdb.txt	2009-05-07 02:02:51 +0000
+++ lib/canonical/launchpad/doc/hwdb.txt	2009-05-10 03:41:25 +0000
@@ -371,7 +371,8 @@
     >>> user.validateAndEnsurePreferredEmail(email)
     >>> transaction.commit()
     >>> from canonical.launchpad.scripts.garbo import HWSubmissionEmailLinker
-    >>> HWSubmissionEmailLinker().run()
+    >>> from canonical.launchpad.ftests.logger import MockLogger
+    >>> HWSubmissionEmailLinker(log=MockLogger()).run()
     >>> submission = hw_submission_set.getBySubmissionKey(u'unique-id-2')
     >>> print submission.owner.displayname
     Beeblebrox
@@ -405,7 +406,7 @@
     >>> login_person(user)
     >>> user.validateAndEnsurePreferredEmail(email)
     >>> transaction.commit()
-    >>> HWSubmissionEmailLinker().run()
+    >>> HWSubmissionEmailLinker(log=MockLogger()).run()
     >>> submission = hw_submission_set.getBySubmissionKey(u'unique-id-2')
     >>> print submission.owner.displayname
     Beeblebrox
 
=== modified file 'lib/canonical/launchpad/doc/script-monitoring.txt'
--- lib/canonical/launchpad/doc/script-monitoring.txt	2009-04-17 10:32:16 +0000
+++ lib/canonical/launchpad/doc/script-monitoring.txt	2009-05-10 03:41:25 +0000
@@ -28,7 +28,7 @@
   >>> from canonical.testing.layers import LaunchpadZopelessLayer
 
   >>> UTC = pytz.timezone('UTC')
-  >>> LaunchpadZopelessLayer.switchDbUser('garbo-daily') # A script db user
+  >>> LaunchpadZopelessLayer.switchDbUser('garbo_daily') # A script db user
 
   >>> activity = getUtility(IScriptActivitySet).recordSuccess(
   ...     name='script-name',
@@ -89,7 +89,7 @@
   ...             raise RuntimeError('Some failure')
   ...
   ... if __name__ == '__main__':
-  ...     script = TestScript('test-script', 'garbo-daily')
+  ...     script = TestScript('test-script', 'garbo_daily')
   ...     script.run()
   ... """)
   >>> script_file.flush()
 
=== modified file 'lib/canonical/launchpad/scripts/garbo.py'
--- lib/canonical/launchpad/scripts/garbo.py	2009-05-08 02:33:14 +0000
+++ lib/canonical/launchpad/scripts/garbo.py	2009-05-10 03:41:25 +0000
@@ -23,16 +23,13 @@
 from canonical.launchpad.interfaces.emailaddress import EmailAddressStatus
 from canonical.launchpad.interfaces.looptuner import ITunableLoop
 from canonical.launchpad.scripts.base import LaunchpadCronScript
-from canonical.launchpad.utilities.looptuner import LoopTuner
+from canonical.launchpad.utilities.looptuner import DBLoopTuner
 from canonical.launchpad.webapp.interfaces import (
-    IStoreSelector, MAIN_STORE, MASTER_FLAVOR)
+    IStoreSelector, AUTH_STORE, MAIN_STORE, MASTER_FLAVOR)
 from lp.code.model.codeimportresult import CodeImportResult
-from lp.code.model.revision import RevisionAuthor
+from lp.code.model.revision import RevisionAuthor, RevisionCache
 from lp.registry.model.mailinglist import MailingListSubscription
 
-from lp.code.model.codeimportresult import CodeImportResult
-from lp.code.model.revision import RevisionCache
-
 
 ONE_DAY_IN_SECONDS = 24*60*60
 
@@ -45,9 +42,12 @@
     maximum_chunk_size = None # Override
     cooldown_time = 0
 
+    def __init__(self, log):
+        self.log = log
+
     def run(self):
         assert self.maximum_chunk_size is not None, "Did not override."
-        LoopTuner(
+        DBLoopTuner(
             self, self.goal_seconds,
             minimum_chunk_size = self.minimum_chunk_size,
             maximum_chunk_size = self.maximum_chunk_size,
@@ -61,7 +61,8 @@
     """
     maximum_chunk_size = 6*60*60 # 6 hours in seconds.
 
-    def __init__(self):
+    def __init__(self, log):
+        super(OAuthNoncePruner, self).__init__(log)
         self.store = IMasterStore(OAuthNonce)
         self.oldest_age = self.store.execute("""
             SELECT COALESCE(EXTRACT(EPOCH FROM
@@ -77,6 +78,10 @@
         self.oldest_age = max(
             ONE_DAY_IN_SECONDS, self.oldest_age - chunk_size)
 
+        self.log.debug(
+            "Removed OAuthNonce rows older than %d seconds"
+            % self.oldest_age)
+
         self.store.find(
             OAuthNonce,
             OAuthNonce.request_timestamp < SQL(
@@ -92,7 +97,8 @@
     """
     maximum_chunk_size = 6*60*60 # 6 hours in seconds.
 
-    def __init__(self):
+    def __init__(self, log):
+        super(OpenIDConsumerNoncePruner, self).__init__(log)
         self.store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
         self.earliest_timestamp = self.store.find(
             Min(OpenIDConsumerNonce.timestamp)).one()
@@ -109,12 +115,52 @@
             self.earliest_wanted_timestamp,
             self.earliest_timestamp + chunk_size)
 
+        self.log.debug(
+            "Removing OpenIDConsumerNonce rows older than %s"
+            % self.earliest_timestamp)
+
         self.store.find(
             OpenIDConsumerNonce,
             OpenIDConsumerNonce.timestamp < self.earliest_timestamp).remove()
         transaction.commit()
 
 
+class OpenIDAssociationPruner(TunableLoop):
+    minimum_chunk_size = 3500
+    maximum_chunk_size = 50000
+
+    table_name = 'OpenIDAssociation'
+    store_name = AUTH_STORE
+
+    _num_removed = None
+
+    def __init__(self, log):
+        super(OpenIDAssociationPruner, self).__init__(log)
+        self.store = getUtility(IStoreSelector).get(
+            self.store_name, MASTER_FLAVOR)
+
+    def __call__(self, chunksize):
+        result = self.store.execute("""
+            DELETE FROM %s
+            WHERE (server_url, handle) IN (
+                SELECT server_url, handle FROM %s
+                WHERE issued + lifetime <
+                    EXTRACT(EPOCH FROM CURRENT_TIMESTAMP)
+                LIMIT %d
+                )
+            """ % (self.table_name, self.table_name, int(chunksize)))
+        self._num_removed = result._raw_cursor.rowcount
+        transaction.commit()
+
+    def isDone(self):
+        return self._num_removed == 0
+
+
+class OpenIDConsumerAssociationPruner(OpenIDAssociationPruner):
+    table_name = 'OpenIDConsumerAssociation'
+    store_name = MAIN_STORE
+
+
 class RevisionCachePruner(TunableLoop):
     """A tunable loop to remove old revisions from the cache."""
 
@@ -141,8 +187,9 @@
     and they are not one of the 4 most recent results for that
     CodeImport.
     """
-    maximum_chunk_size = 100
-    def __init__(self):
+    maximum_chunk_size = 1000
+    def __init__(self, log):
+        super(CodeImportResultPruner, self).__init__(log)
         self.store = IMasterStore(CodeImportResult)
 
         self.min_code_import = self.store.find(
@@ -158,6 +205,11 @@
             or self.next_code_import_id > self.max_code_import)
 
     def __call__(self, chunk_size):
+        self.log.debug(
+            "Removing expired CodeImportResults for CodeImports %d -> %d" % (
+                self.next_code_import_id,
+                self.next_code_import_id + chunk_size - 1))
+
         self.store.execute("""
             DELETE FROM CodeImportResult
             WHERE
@@ -193,7 +245,8 @@
 
     maximum_chunk_size = 1000
 
-    def __init__(self):
+    def __init__(self, log):
+        super(RevisionAuthorEmailLinker, self).__init__(log)
         self.author_store = IMasterStore(RevisionAuthor)
         self.email_store = IMasterStore(EmailAddress)
 
@@ -252,7 +305,8 @@
 
     maximum_chunk_size = 1000
 
-    def __init__(self):
+    def __init__(self, log):
+        super(HWSubmissionEmailLinker, self).__init__(log)
         self.submission_store = IMasterStore(HWSubmission)
         self.email_store = IMasterStore(EmailAddress)
 
@@ -311,7 +365,8 @@
 
     maximum_chunk_size = 1000
 
-    def __init__(self):
+    def __init__(self, log):
+        super(MailingListSubscriptionPruner, self).__init__(log)
         self.subscription_store = IMasterStore(MailingListSubscription)
         self.email_store = IMasterStore(EmailAddress)
 
@@ -354,14 +409,24 @@
     script_name = None # Script name for locking and database user. Override.
     tunable_loops = None # Collection of TunableLoops. Override.
 
+    # _maximum_chunk_size is used to override the defined
+    # maximum_chunk_size to allow our tests to ensure multiple calls to
+    # __call__ are required without creating huge amounts of test data.
+    _maximum_chunk_size = None
+
     def __init__(self, test_args=None):
         super(BaseDatabaseGarbageCollector, self).__init__(
-            self.script_name, dbuser=self.script_name, test_args=test_args)
+            self.script_name,
+            dbuser=self.script_name.replace('-','_'),
+            test_args=test_args)
 
     def main(self):
         for tunable_loop in self.tunable_loops:
             self.logger.info("Running %s" % tunable_loop.__name__)
-            tunable_loop().run()
+            tunable_loop = tunable_loop(log=self.logger)
+            if self._maximum_chunk_size is not None:
+                tunable_loop.maximum_chunk_size = self._maximum_chunk_size
+            tunable_loop.run()
 
 
 class HourlyDatabaseGarbageCollector(BaseDatabaseGarbageCollector):
@@ -369,9 +434,12 @@
     tunable_loops = [
         OAuthNoncePruner,
         OpenIDConsumerNoncePruner,
+        OpenIDAssociationPruner,
+        OpenIDConsumerAssociationPruner,
         RevisionCachePruner,
         ]
 
+
 class DailyDatabaseGarbageCollector(BaseDatabaseGarbageCollector):
     script_name = 'garbo-daily'
     tunable_loops = [
 
=== modified file 'lib/canonical/launchpad/scripts/tests/test_garbo.py'
--- lib/canonical/launchpad/scripts/tests/test_garbo.py	2009-05-04 09:16:25 +0000
+++ lib/canonical/launchpad/scripts/tests/test_garbo.py	2009-05-10 03:41:25 +0000
@@ -13,6 +13,7 @@
 from storm.expr import Min
 from storm.store import Store
 import transaction
+from zope.component import getUtility
 
 from lp.code.model.codeimportresult import CodeImportResult
 from canonical.launchpad.database.oauth import OAuthNonce
@@ -22,9 +23,12 @@
 from lp.code.interfaces.codeimportresult import CodeImportResultStatus
 from canonical.launchpad.testing import TestCase, TestCaseWithFactory
 from canonical.launchpad.scripts.garbo import (
-    DailyDatabaseGarbageCollector, HourlyDatabaseGarbageCollector)
+    DailyDatabaseGarbageCollector, HourlyDatabaseGarbageCollector,
+    OpenIDAssociationPruner, OpenIDConsumerAssociationPruner)
 from canonical.launchpad.scripts.tests import run_script
 from canonical.launchpad.scripts.logger import QuietFakeLogger
+from canonical.launchpad.webapp.interfaces import (
+    IStoreSelector, MASTER_FLAVOR)
 from canonical.testing.layers import (
     DatabaseLayer, LaunchpadScriptLayer, LaunchpadZopelessLayer)
 from lp.registry.interfaces.person import PersonCreationRationale
@@ -59,20 +63,21 @@
         self.runDaily()
         self.runHourly()
 
-    def runDaily(self):
-        LaunchpadZopelessLayer.switchDbUser('garbo-daily')
+    def runDaily(self, maximum_chunk_size=2):
+        LaunchpadZopelessLayer.switchDbUser('garbo_daily')
         collector = DailyDatabaseGarbageCollector(test_args=[])
+        collector._maximum_chunk_size = maximum_chunk_size
         collector.logger = QuietFakeLogger()
         collector.main()
 
-    def runHourly(self):
-        LaunchpadZopelessLayer.switchDbUser('garbo-hourly')
+    def runHourly(self, maximum_chunk_size=2):
+        LaunchpadZopelessLayer.switchDbUser('garbo_hourly')
         collector = HourlyDatabaseGarbageCollector(test_args=[])
+        collector._maximum_chunk_size = maximum_chunk_size
         collector.logger = QuietFakeLogger()
         collector.main()
 
     def test_OAuthNoncePruner(self):
-        store = IMasterStore(OAuthNonce)
         now = datetime.utcnow().replace(tzinfo=UTC)
         timestamps = [
             now - timedelta(days=2), # Garbage
@@ -81,6 +86,7 @@
             now, # Not garbage
             ]
         LaunchpadZopelessLayer.switchDbUser('testadmin')
+        store = IMasterStore(OAuthNonce)
 
         # Make sure we start with 0 nonces.
         self.failUnlessEqual(store.find(OAuthNonce).count(), 0)
@@ -95,7 +101,9 @@
         # Make sure we have 4 nonces now.
         self.failUnlessEqual(store.find(OAuthNonce).count(), 4)
 
-        self.runHourly()
+        self.runHourly(maximum_chunk_size=60) # 1 minute maximum chunk size
+
+        store = IMasterStore(OAuthNonce)
 
         # Now back to two, having removed the two garbage entries.
         self.failUnlessEqual(store.find(OAuthNonce).count(), 2)
@@ -139,7 +147,9 @@
         self.failUnlessEqual(store.find(OpenIDConsumerNonce).count(), 4)
 
         # Run the garbage collector.
-        self.runHourly()
+        self.runHourly(maximum_chunk_size=60) # 1 minute maximum chunks.
+
+        store = IMasterStore(OpenIDConsumerNonce)
 
         # We should now have 2 nonces.
         self.failUnlessEqual(store.find(OpenIDConsumerNonce).count(), 2)
@@ -170,16 +180,19 @@
         self.runDaily()
 
         # Nothing is removed, because we always keep the 4 latest.
+        store = IMasterStore(CodeImportResult)
         self.failUnlessEqual(
             store.find(CodeImportResult).count(), 4)
 
         new_code_import_result(now - timedelta(days=31))
         self.runDaily()
+        store = IMasterStore(CodeImportResult)
         self.failUnlessEqual(
             store.find(CodeImportResult).count(), 4)
 
         new_code_import_result(now - timedelta(days=29))
         self.runDaily()
+        store = IMasterStore(CodeImportResult)
         self.failUnlessEqual(
             store.find(CodeImportResult).count(), 4)
 
@@ -189,6 +202,53 @@
                 Min(CodeImportResult.date_created)).one().replace(tzinfo=UTC)
             >= now - timedelta(days=30))
 
+    def test_OpenIDAssociationPruner(self, pruner=OpenIDAssociationPruner):
+        store_name = pruner.store_name
+        table_name = pruner.table_name
+        LaunchpadZopelessLayer.switchDbUser('testadmin')
+        store_selector = getUtility(IStoreSelector)
+        store = store_selector.get(store_name, MASTER_FLAVOR)
+        now = time.time()
+        # Create some associations in the past with lifetimes
+        for delta in range(0, 20):
+            store.execute("""
+                INSERT INTO %s (server_url, handle, issued, lifetime)
+                VALUES (%s, %s, %d, %d)
+                """ % (table_name, str(delta), str(delta), now-10, delta))
+        transaction.commit()
+
+        # Ensure that we created at least one expirable row (using the
+        # test start time as 'now').
+        num_expired = store.execute("""
+            SELECT COUNT(*) FROM %s
+            WHERE issued + lifetime < %f
+            """ % (table_name, now)).get_one()[0]
+        self.failUnless(num_expired > 0)
+
+        # Expire all those expirable rows, and possibly a few more if this
+        # test is running slow.
+        self.runHourly()
+
+        LaunchpadZopelessLayer.switchDbUser('testadmin')
+        store = store_selector.get(store_name, MASTER_FLAVOR)
+        # Confirm all the rows we know should have been expired have
+        # been expired. These are the ones that would be expired using
+        # the test start time as 'now'.
+        num_expired = store.execute("""
+            SELECT COUNT(*) FROM %s
+            WHERE issued + lifetime < %f
+            """ % (table_name, now)).get_one()[0]
+        self.failUnlessEqual(num_expired, 0)
+
+        # Confirm that we haven't expired everything. This test will fail
+        # if it has taken 10 seconds to get this far.
+        num_unexpired = store.execute(
+            "SELECT COUNT(*) FROM %s" % table_name).get_one()[0]
+        self.failUnless(num_unexpired > 0)
+
+    def test_OpenIDConsumerAssociationPruner(self):
+        self.test_OpenIDAssociationPruner(OpenIDConsumerAssociationPruner)
+
     def test_RevisionAuthorEmailLinker(self):
         LaunchpadZopelessLayer.switchDbUser('testadmin')
         rev1 = self.factory.makeRevision('Author 1 <[e-mail]>')
@@ -301,5 +361,6 @@
         self.runDaily()
         self.assertEqual(mailing_list.getSubscription(person), None)
 
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)
 
=== modified file 'lib/canonical/launchpad/utilities/looptuner.py'
--- lib/canonical/launchpad/utilities/looptuner.py	2009-04-17 10:32:16 +0000
+++ lib/canonical/launchpad/utilities/looptuner.py	2009-05-10 03:41:25 +0000
@@ -180,10 +180,10 @@
     """
 
     # We block until replication lag is under this threshold.
-    acceptable_replication_lag = timedelta(seconds=90) # In seconds.
+    acceptable_replication_lag = timedelta(seconds=30) # In seconds.
 
     # We block if there are transactions running longer than this threshold.
-    long_running_transaction = 60*60 # In seconds
+    long_running_transaction = 30*60 # In seconds
 
     def _blockWhenLagged(self):
         """When database replication lag is high, block until it drops."""
@@ -222,7 +222,7 @@
                     usename,
                     datname,
                     current_query
-                FROM pg_stat_activity
+                FROM activity()
                 WHERE xact_start < CURRENT_TIMESTAMP - interval '%f seconds'
                 """ % self.long_running_transaction).get_all())
             if not results:
 
=== modified file 'lib/lp/code/model/tests/test_revision.py'
--- lib/lp/code/model/tests/test_revision.py	2009-05-08 02:33:14 +0000
+++ lib/lp/code/model/tests/test_revision.py	2009-05-10 03:41:25 +0000
@@ -17,6 +17,7 @@
 
 from canonical.database.sqlbase import cursor
 from canonical.launchpad.ftests import login, logout
+from canonical.launchpad.ftests.logger import MockLogger
 from canonical.launchpad.interfaces.lpstorm import IMasterObject
 from canonical.launchpad.interfaces.account import AccountStatus
 from canonical.launchpad.scripts.garbo import RevisionAuthorEmailLinker
@@ -165,7 +166,7 @@
         # The person registers with Launchpad.
         author = self.factory.makePerson(email=email)
         # Garbo runs the RevisionAuthorEmailLinker job.
-        RevisionAuthorEmailLinker().run()
+        RevisionAuthorEmailLinker(log=MockLogger()).run()
         # Now the kama needs allocating.
         self.assertEqual(
             [rev], list(RevisionSet.getRevisionsNeedingKarmaAllocated()))
 
=== modified file 'lib/lp/code/model/tests/test_revisionauthor.py'
--- lib/lp/code/model/tests/test_revisionauthor.py	2009-05-07 02:02:51 +0000
+++ lib/lp/code/model/tests/test_revisionauthor.py	2009-05-10 03:41:26 +0000
@@ -11,6 +11,7 @@
 
 from canonical.config import config
 from canonical.launchpad.interfaces.emailaddress import EmailAddressStatus
+from canonical.launchpad.ftests.logger import MockLogger
 from canonical.launchpad.scripts.garbo import RevisionAuthorEmailLinker
 from canonical.launchpad.testing import LaunchpadObjectFactory, TestCase
 from canonical.testing import LaunchpadZopelessLayer
@@ -157,7 +158,7 @@
 
         # After the garbo RevisionAuthorEmailLinker job runs, the link
         # is made.
-        RevisionAuthorEmailLinker().run()
+        RevisionAuthorEmailLinker(log=MockLogger()).run()
         self.assertEqual(harry, self.author.person,
                          'Harry should now be the author.')
 
 
=== modified file 'lib/lp/code/scripts/tests/test_revisionkarma.py'
--- lib/lp/code/scripts/tests/test_revisionkarma.py	2009-05-07 02:02:51 +0000
+++ lib/lp/code/scripts/tests/test_revisionkarma.py	2009-05-10 03:41:26 +0000
@@ -10,6 +10,7 @@
 
 from canonical.config import config
 from canonical.launchpad.database.emailaddress import EmailAddressSet
+from canonical.launchpad.ftests.logger import MockLogger
 from canonical.launchpad.scripts.garbo import RevisionAuthorEmailLinker
 from canonical.launchpad.testing import TestCaseWithFactory
 from canonical.testing import LaunchpadZopelessLayer
@@ -74,7 +75,7 @@
         author = self.factory.makePerson(email=email)
         transaction.commit()
         # Run the RevisionAuthorEmailLinker garbo job.
-        RevisionAuthorEmailLinker().run()
+        RevisionAuthorEmailLinker(log=MockLogger()).run()
         LaunchpadZopelessLayer.switchDbUser(config.revisionkarma.dbuser)
         script = RevisionKarmaAllocator(
             'test', config.revisionkarma.dbuser, ['-q'])
@@ -106,7 +107,7 @@
             EmailAddressSet().new(email, author, account=author.account))
         transaction.commit()
         # Run the RevisionAuthorEmailLinker garbo job.
-        RevisionAuthorEmailLinker().run()
+        RevisionAuthorEmailLinker(log=MockLogger()).run()
 
         # Now that the revision author is linked to the person, the revision
         # needs karma allocated.
 
=== modified file 'scripts/rosetta/message-sharing-merge.py' (properties changed: +x to -x)