Merge lp:~stub/launchpad/bug-658124-revision-karma into lp:launchpad

[Stuart Bishop]

Fix performance of revision karma allocator with PostgreSQL 8.4

The revision karma allocator is currently disabled due to major performance problems with PostgreSQL 8.4, per Bug #658124

[Robert Collins (lifeless)]

Looks to me like a DISTINCT ON would be more efficient and faster. Perhaps if you're moving it to the garbo post release it could be reorganised too?

review: Approve (release-critical)

[Tim Penhey (thumper)]

Looks reasonable

review: Approve

=== modified file 'lib/lp/code/interfaces/revision.py'
--- lib/lp/code/interfaces/revision.py	2010-09-10 20:03:43 +0000
+++ lib/lp/code/interfaces/revision.py	2010-10-12 05:41:07 +0000
@@ -154,7 +154,7 @@
         :return: ResultSet containing tuples of (Revision, RevisionAuthor)
         """
 
-    def getRevisionsNeedingKarmaAllocated():
+    def getRevisionsNeedingKarmaAllocated(limit=None):
         """Get the revisions needing karma allocated.
 
         Under normal circumstances karma is allocated for revisions by the

=== modified file 'lib/lp/code/model/revision.py'
--- lib/lp/code/model/revision.py	2010-10-03 15:30:06 +0000
+++ lib/lp/code/model/revision.py	2010-10-12 05:41:07 +0000
@@ -32,7 +32,6 @@
     And,
     Asc,
     Desc,
-    Exists,
     Join,
     Not,
     Or,
@@ -65,7 +64,7 @@
     EmailAddressStatus,
     IEmailAddressSet,
     )
-from canonical.launchpad.interfaces.lpstorm import IMasterStore
+from canonical.launchpad.interfaces.lpstorm import IMasterStore, IStore
 from canonical.launchpad.webapp.interfaces import (
     DEFAULT_FLAVOR,
     IStoreSelector,
@@ -426,26 +425,26 @@
         return result_set.order_by(Desc(Revision.revision_date))
 
     @staticmethod
-    def getRevisionsNeedingKarmaAllocated():
+    def getRevisionsNeedingKarmaAllocated(limit=None):
         """See `IRevisionSet`."""
         # Here to stop circular imports.
         from lp.code.model.branch import Branch
         from lp.code.model.branchrevision import BranchRevision
         from lp.registry.model.person import ValidPersonCache
 
-        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
-        return store.find(
+        store = IStore(Revision)
+        results_with_dupes = store.find(
             Revision,
             Revision.revision_author == RevisionAuthor.id,
             RevisionAuthor.person == ValidPersonCache.id,
             Not(Revision.karma_allocated),
-            Exists(
-                Select(True,
-                       And(BranchRevision.revision == Revision.id,
-                           BranchRevision.branch == Branch.id,
-                           Or(Branch.product != None,
-                              Branch.distroseries != None)),
-                       (Branch, BranchRevision))))
+            BranchRevision.revision == Revision.id,
+            BranchRevision.branch == Branch.id,
+            Or(Branch.product != None, Branch.distroseries != None))[:limit]
+        # Eliminate duplicate rows, returning <= limit rows
+        return store.find(
+            Revision, Revision.id.is_in(
+                results_with_dupes.get_select_expr(Revision.id)))
 
     @staticmethod
     def getPublicRevisionsForPerson(person, day_limit=30):

=== modified file 'lib/lp/code/scripts/revisionkarma.py'
--- lib/lp/code/scripts/revisionkarma.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/scripts/revisionkarma.py	2010-10-12 05:41:07 +0000
@@ -40,7 +40,7 @@
         # Break into bits.
         while True:
             revisions = list(
-                revision_set.getRevisionsNeedingKarmaAllocated()[:100])
+                revision_set.getRevisionsNeedingKarmaAllocated(100))
             if len(revisions) == 0:
                 break
             for revision in revisions: