Merge lp:~wallyworld/launchpad/improve-latestpersonspr-garbo-job-1071581 into lp:launchpad

[Ian Booth]

Improve PopulateLatestPersonSourcepackageReleaseCache so it only processes new spr records.

== Implementation ==

When I first wrote the garbo job, I didn't now that a new source package release record was created each time a package is uploaded. So the garbo job reset the next_id to 0 after each run, causing all spr's to eventually be reprocessed. This branch alters the job so that a separate, monotonically increasing next id is used for processing creator and maintainer records. Thus, the garbo job should eventually "catch" up to the releases and from then on do minimal work to maintain the report table.

== Tests ==

Update the test_PopulateLatestPersonSourcepackageReleaseCache to chec the job state.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/scripts/garbo.py
  lib/lp/scripts/tests/test_garbo.py

[j.c.sackett (jcsackett)]

Thanks, Ian.

review: Approve

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2012-11-05 11:32:27 +0000
+++ lib/lp/scripts/garbo.py	2012-11-08 07:54:22 +0000
@@ -478,13 +478,15 @@
         self.store = getUtility(IStoreSelector).get(MAIN_STORE, MASTER_FLAVOR)
         # Keep a record of the processed source package release id and data
         # type (creator or maintainer) so we know where to job got up to.
-        self.next_id = 0
+        self.next_id_for_creator = 0
+        self.next_id_for_maintainer = 0
         self.current_person_filter_type = 'creator'
         self.starting_person_filter_type = self.current_person_filter_type
         self.job_name = self.__class__.__name__
         job_data = load_garbo_job_state(self.job_name)
         if job_data:
-            self.next_id = job_data['next_id']
+            self.next_id_for_creator = job_data['next_id_for_creator']
+            self.next_id_for_maintainer = job_data['next_id_for_maintainer']
             self.current_person_filter_type = job_data['person_filter_type']
             self.starting_person_filter_type = self.current_person_filter_type
 
@@ -493,8 +495,10 @@
         # creator or maintainer as required.
         if self.current_person_filter_type == 'creator':
             person_filter = SourcePackageRelease.creatorID
+            next_id = self.next_id_for_creator
         else:
             person_filter = SourcePackageRelease.maintainerID
+            next_id = self.next_id_for_maintainer
         spph = ClassAlias(SourcePackagePublishingHistory, "spph")
         origin = [
             SourcePackageRelease,
@@ -504,7 +508,7 @@
                     spph.archiveID == SourcePackageRelease.upload_archiveID))]
         spr_select = self.store.using(*origin).find(
             (SourcePackageRelease.id, Alias(spph.id, 'spph_id')),
-            SourcePackageRelease.id > self.next_id
+            SourcePackageRelease.id > next_id
         ).order_by(
             person_filter,
             SourcePackageRelease.upload_distroseriesID,
@@ -546,7 +550,6 @@
                 self.current_person_filter_type = 'maintainer'
             else:
                 self.current_person_filter_type = 'creator'
-            self.next_id = 0
             current_count = self.getPendingUpdates().count()
         return current_count == 0
 
@@ -601,17 +604,22 @@
             self.store.execute(Insert(columns, values=inserts))
 
     def __call__(self, chunk_size):
-        max_id = self.next_id
+        max_id = None
         updates = []
         for update in (self.getPendingUpdates()[:chunk_size]):
             updates.append(update)
             max_id = update[0]
         self.update_cache(updates)
 
-        self.next_id = max_id
+        if max_id:
+            if self.current_person_filter_type == 'creator':
+                self.next_id_for_creator = max_id
+            else:
+                self.next_id_for_maintainer = max_id
         self.store.flush()
         save_garbo_job_state(self.job_name, {
-            'next_id': max_id,
+            'next_id_for_creator': self.next_id_for_creator,
+            'next_id_for_maintainer': self.next_id_for_maintainer,
             'person_filter_type': self.current_person_filter_type})
         transaction.commit()
 

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2012-11-07 12:38:19 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2012-11-08 07:54:22 +0000
@@ -1205,6 +1205,11 @@
         _assert_release_by_maintainer(maintainers[0], spr3)
         _assert_release_by_maintainer(maintainers[1], spr4)
 
+        job_data = load_garbo_job_state(
+            'PopulateLatestPersonSourcepackageReleaseCache')
+        self.assertEqual(spr4.id, job_data['next_id_for_creator'])
+        self.assertEqual(spr4.id, job_data['next_id_for_maintainer'])
+
         # Create a newer published source package release and ensure the
         # release cache table is correctly updated.
         switch_dbuser('testadmin')
@@ -1224,6 +1229,11 @@
         _assert_release_by_maintainer(maintainers[0], spr3)
         _assert_release_by_maintainer(maintainers[1], spr5)
 
+        job_data = load_garbo_job_state(
+            'PopulateLatestPersonSourcepackageReleaseCache')
+        self.assertEqual(spr5.id, job_data['next_id_for_creator'])
+        self.assertEqual(spr5.id, job_data['next_id_for_maintainer'])
+
 
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer