launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/db-external-suggestions-template-cache into lp:launchpad.

Requested reviews:
  Stuart Bishop (stub): code
  Launchpad code reviewers (launchpad-reviewers): code


= Suggestive translations cache =

The +translate pages are pretty slow.  A lot of that time is spent finding "external suggestions"—translations for identical English messages occurring elsewhere in Launchpad.

A good portion of the time searching for external suggestions for a message (as well as impressive swathes of SQL and probably a lot of the locking hazards) is spent figuring out which templates it can take translations from.

We figure out the exact same list of templates for every translatable message on the page.

This branch creates (but does not yet start using) a simple database cache of these "suggestive" templates.  It's just a list of template ids to replace this complex repetitive work.  Writing the cache is simple and fast, and takes the common work out of the +translate page.  We could probably afford to rewrite it every minute if we wanted, but once a day should be fine for what it does.

The SQL for looking for external suggestions becomes lots simpler, but I left that to a separate branch since it may interfere with ongoing work.

Test:
{{{
./bin/test -vvc -m lp.translations -t suggestive.templates
}}}

To Q/A, run the patch; run cronscripts/cache-suggestive-potemplates.py; and verify that SuggestivePOTemplate contains the number of non-obsolete templates for Ubuntu plus Translations-using projects of rows.  Each row is unique.  Join it to POTemplate to see that you get the same number of rows.

I'll still be wanting a proper patch number.  Before landing I can update that, as well as the sample data.


Jeroen
-- 
https://code.launchpad.net/~jtv/launchpad/db-external-suggestions-template-cache/+merge/30694
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/db-external-suggestions-template-cache into lp:launchpad.

=== added file 'cronscripts/cache-suggestive-potemplates.py'
--- cronscripts/cache-suggestive-potemplates.py	1970-01-01 00:00:00 +0000
+++ cronscripts/cache-suggestive-potemplates.py	2010-07-22 17:36:25 +0000
@@ -0,0 +1,21 @@
+#! /usr/bin/python2.5
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Refresh the SuggestivePOTemplate cache.
+
+The SuggestivePOTemplate cache is a narrow, lightweight database table
+containing only the ids of `POTemplate`s that can provide external
+translation suggestions.
+"""
+
+import _pythonpath
+
+__metaclass__ = type
+
+from lp.translations.scripts.cachesuggestivepotemplates import (
+    CacheSuggestivePOTemplates)
+
+if __name__ == '__main__':
+    script = CacheSuggestivePOTemplates(dbuser='suggestivepotemplates')
+    script.lock_and_run()

=== added file 'database/schema/patch-2207-99-0.sql'
--- database/schema/patch-2207-99-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2207-99-0.sql	2010-07-22 17:36:25 +0000
@@ -0,0 +1,19 @@
+SET client_min_messages=ERROR;
+
+CREATE TABLE SuggestivePOTemplate(potemplate integer UNIQUE);
+
+INSERT INTO SuggestivePOTemplate (
+    SELECT POTemplate.id
+    FROM POTemplate
+    LEFT JOIN DistroSeries ON DistroSeries.id = POTemplate.distroseries
+    LEFT JOIN Distribution ON Distribution.id = DistroSeries.distribution
+    LEFT JOIN ProductSeries ON ProductSeries.id = POTemplate.productseries
+    LEFT JOIN Product ON Product.id = ProductSeries.product
+    WHERE
+	POTemplate.iscurrent AND
+        (Distribution.official_rosetta OR Product.official_rosetta)
+    ORDER BY POTemplate.id
+);
+
+-- XXX: Correct patch number.
+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 99, 0);

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2010-07-21 21:51:03 +0000
+++ database/schema/security.cfg	2010-07-22 17:36:25 +0000
@@ -259,6 +259,7 @@
 public.standardshipitrequest            = SELECT, INSERT, UPDATE, DELETE
 public.staticdiff                       = SELECT, INSERT, UPDATE
 public.structuralsubscription           = SELECT, INSERT, UPDATE, DELETE
+public.suggestivepotemplate             = SELECT, INSERT, DELETE
 public.temporaryblobstorage             = SELECT, INSERT, DELETE
 public.translationgroup                 = SELECT, INSERT, UPDATE
 public.translationimportqueueentry      = SELECT, INSERT, UPDATE, DELETE
@@ -1431,6 +1432,16 @@
 public.translationmessage               = SELECT
 public.translationtemplateitem          = SELECT
 
+[suggestivepotemplates]
+type=user
+groups=script
+public.distribution                     = SELECT
+public.distroseries                     = SELECT
+public.potemplate                       = SELECT
+public.product                          = SELECT
+public.productseries                    = SELECT
+public.suggestivepotemplate             = INSERT, DELETE
+
 [oopsprune]
 type=user
 groups=script

=== modified file 'lib/lp/translations/interfaces/potemplate.py'
--- lib/lp/translations/interfaces/potemplate.py	2010-07-20 12:33:43 +0000
+++ lib/lp/translations/interfaces/potemplate.py	2010-07-22 17:36:25 +0000
@@ -640,6 +640,18 @@
         migration spec.
         """
 
+    def wipeSuggestivePOTemplatesCache():
+        """Erase suggestive-templates cache.
+
+        :return: Number of rows deleted.
+        """
+
+    def populateSuggestivePOTemplatesCache():
+        """Populate suggestive-templates cache.
+
+        :return: Numver of rows inserted.
+        """
+
 
 class IPOTemplateSharingSubset(Interface):
     """A subset of sharing PO templates."""

=== modified file 'lib/lp/translations/model/potemplate.py'
--- lib/lp/translations/model/potemplate.py	2010-07-20 18:19:08 +0000
+++ lib/lp/translations/model/potemplate.py	2010-07-22 17:36:25 +0000
@@ -39,7 +39,7 @@
 from canonical.database.sqlbase import (
     SQLBase, quote, quote_like, flush_database_updates, sqlvalues)
 from canonical.launchpad import helpers
-from canonical.launchpad.interfaces.lpstorm import IStore
+from canonical.launchpad.interfaces.lpstorm import IMasterStore, IStore
 from lp.translations.utilities.rosettastats import RosettaStats
 from lp.services.database.prejoin import prejoin
 from lp.services.database.collection import Collection
@@ -1323,7 +1323,7 @@
 
     @staticmethod
     def compareSharingPrecedence(left, right):
-        """See IPOTemplateSet."""
+        """See `IPOTemplateSet`."""
         if left == right:
             return 0
 
@@ -1357,6 +1357,33 @@
             assert left.id < right.id, "Got unordered ids."
             return 1
 
+    def wipeSuggestivePOTemplatesCache(self):
+        """See `IPOTemplateSet`."""
+        return IMasterStore(POTemplate).execute(
+            "DELETE FROM SuggestivePOTemplate").rowcount
+
+    def populateSuggestivePOTemplatesCache(self):
+        """See `IPOTemplateSet`."""
+        return IMasterStore(POTemplate).execute("""
+            INSERT INTO SuggestivePOTemplate (
+                SELECT POTemplate.id
+                FROM POTemplate
+                LEFT JOIN DistroSeries ON
+                    DistroSeries.id = POTemplate.distroseries
+                LEFT JOIN Distribution ON
+                    Distribution.id = DistroSeries.distribution
+                LEFT JOIN ProductSeries ON
+                    ProductSeries.id = POTemplate.productseries
+                LEFT JOIN Product ON
+                    Product.id = ProductSeries.product
+                WHERE
+                    POTemplate.iscurrent AND (
+                        Distribution.official_rosetta OR
+                        Product.official_rosetta)
+                ORDER BY POTemplate.id
+            )
+            """).rowcount
+
 
 class POTemplateSharingSubset(object):
     implements(IPOTemplateSharingSubset)

=== added file 'lib/lp/translations/scripts/cachesuggestivepotemplates.py'
--- lib/lp/translations/scripts/cachesuggestivepotemplates.py	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/scripts/cachesuggestivepotemplates.py	2010-07-22 17:36:25 +0000
@@ -0,0 +1,45 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Update the SuggestivePOTemplate cache.
+
+The SuggestivePOTemplate cache is a narrow, lightweight database table
+containing only the ids of `POTemplate`s that can provide external
+translation suggestions.
+"""
+
+__metaclass__ = type
+__all__ = [
+    'CacheSuggestivePOTemplates',
+    ]
+
+import transaction
+
+from zope.component import getUtility
+
+from lp.services.scripts.base import LaunchpadCronScript
+from lp.translations.interfaces.potemplate import IPOTemplateSet
+
+
+class CacheSuggestivePOTemplates(LaunchpadCronScript):
+    """Refresh the SuggestivePOTemplate cache."""
+
+    def add_my_options(self):
+        """See `LaunchpadScript`."""
+        self.parser.add_option(
+            '-n', '--dry-run', action='store_true', dest='dry_run',
+            help="Only pretend; do not commit changes to the database.")
+
+    def main(self):
+        utility = getUtility(IPOTemplateSet)
+        self.logger.debug("Wiping cache.")
+        old_rows = utility.wipeSuggestivePOTemplatesCache()
+        self.logger.debug("Repopulating cache.")
+        new_rows = utility.populateSuggestivePOTemplatesCache()
+        if self.options.dry_run:
+            self.logger.info("Dry run; not committing.")
+            transaction.abort()
+        else:
+            transaction.commit()
+        self.logger.info("Cache size was %d; is now %d." % (
+            old_rows, new_rows))

=== added file 'lib/lp/translations/scripts/tests/test_cache_suggestive_templates.py'
--- lib/lp/translations/scripts/tests/test_cache_suggestive_templates.py	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/scripts/tests/test_cache_suggestive_templates.py	2010-07-22 17:36:25 +0000
@@ -0,0 +1,157 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test script that refreshes the suggestive-POTemplates cache."""
+
+__metaclass__ = type
+
+import transaction
+import unittest
+
+from zope.component import getUtility
+
+from canonical.launchpad.ftests.script import run_script
+from canonical.launchpad.webapp.interfaces import (
+    IStoreSelector, DEFAULT_FLAVOR, MAIN_STORE)
+from canonical.testing.layers import ZopelessDatabaseLayer
+from lp.testing import TestCase, TestCaseWithFactory
+
+from lp.translations.interfaces.potemplate import IPOTemplateSet
+from lp.translations.scripts.cachesuggestivepotemplates import (
+    CacheSuggestivePOTemplates)
+
+
+class TestSuggestivePOTemplatesCache(TestCaseWithFactory):
+    layer = ZopelessDatabaseLayer
+
+    def setUp(self):
+        super(TestSuggestivePOTemplatesCache, self).setUp()
+        self.utility = getUtility(IPOTemplateSet)
+
+    def _refreshCache(self):
+        """Refresh the cache, but do not commit."""
+        self.utility.wipeSuggestivePOTemplatesCache()
+        self.utility.populateSuggestivePOTemplatesCache()
+
+    def _readCache(self):
+        """Read cache contents, in deterministic order."""
+        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
+        result = store.execute(
+            "SELECT * FROM SuggestivePOTemplate ORDER BY potemplate")
+        return [id for id, in result.get_all()]
+
+    def test_consistent_contents(self):
+        # Refreshing the cache will reproduce the same cache if there
+        # have been no intervening template changes.
+        self._refreshCache()
+        contents = self._readCache()
+        self._refreshCache()
+        self.assertEqual(contents, self._readCache())
+
+    def test_wipe(self):
+        # The wipe method clears the cache.
+        self._refreshCache()
+        self.assertNotEqual([], self._readCache())
+
+        self.utility.wipeSuggestivePOTemplatesCache()
+
+        self.assertEqual([], self._readCache())
+
+    def test_populate(self):
+        # The populate method fills an empty cache.
+        self.utility.wipeSuggestivePOTemplatesCache()
+        self.utility.populateSuggestivePOTemplatesCache()
+        self.assertNotEqual([], self._readCache())
+
+    def test_main(self):
+        # The main method repopulates the cache, and commits.
+        self._refreshCache()
+        cache_before = self._readCache()
+
+        pot = self.factory.makePOTemplate()
+        CacheSuggestivePOTemplates(test_args=['-q']).main()
+
+        # main() committed, so aborting here has no effect.
+        transaction.abort()
+
+        cache_after = self._readCache()
+        self.assertNotEqual(cache_before, cache_after)
+        self.assertContentEqual(cache_before + [pot.id], cache_after)
+
+    def test_new_template(self):
+        # A new template appears in the cache on the next refresh.
+        self._refreshCache()
+        cache_before = self._readCache()
+
+        pot = self.factory.makePOTemplate()
+        self._refreshCache()
+
+        self.assertContentEqual(cache_before + [pot.id], self._readCache())
+
+    def test_product_official_rosetta(self):
+        # Templates from projects are included in the cache only where
+        # the project uses Launchpad Translations.
+        productseries = self.factory.makeProductSeries()
+        productseries.product.official_rosetta = True
+        pot = self.factory.makePOTemplate(productseries=productseries)
+        self._refreshCache()
+
+        cache_with_template = self._readCache()
+
+        productseries.product.official_rosetta = False
+        self._refreshCache()
+
+        cache_without_template = self._readCache()
+        self.assertNotEqual(cache_with_template, cache_without_template)
+        self.assertContentEqual(
+            cache_with_template, cache_without_template + [pot.id])
+
+    def test_distro_official_rosetta(self):
+        # Templates from distributions are included in the cache only
+        # where the distribution uses Launchpad Translations.
+        package = self.factory.makeSourcePackage()
+        package.distroseries.distribution.official_rosetta = True
+        pot = self.factory.makePOTemplate(
+            distroseries=package.distroseries,
+            sourcepackagename=package.sourcepackagename)
+        self._refreshCache()
+
+        cache_with_template = self._readCache()
+
+        package.distroseries.distribution.official_rosetta = False
+        self._refreshCache()
+
+        cache_without_template = self._readCache()
+        self.assertNotEqual(cache_with_template, cache_without_template)
+        self.assertContentEqual(
+            cache_with_template, cache_without_template + [pot.id])
+
+    def test_disabled_template(self):
+        # A template that is not current is excluded from the cache.
+        self._refreshCache()
+        cache_before = self._readCache()
+
+        pot = self.factory.makePOTemplate()
+        pot.iscurrent = False
+        self._refreshCache()
+
+        self.assertEqual(cache_before, self._readCache())
+
+    def test_dry_run(self):
+        # The --dry-run option inhibits any real database changes.
+        self._refreshCache()
+        cache_before = self._readCache()
+
+        potemplate = self.factory.makePOTemplate()
+        CacheSuggestivePOTemplates(test_args=['--dry-run', '-q']).main()
+
+        self.assertEqual(cache_before, self._readCache())
+
+
+class TestCacheSuggestivePOTemplatesScript(TestCase):
+    """Test real script run.  Costly, so do only once."""
+
+    def test_run_script(self):
+        (returncode, stdout, stderr) = run_script(
+            'cronscripts/cache-suggestive-potemplates.py', ['--dry-run'])
+        self.assertEqual(0, returncode)