Launchpad itself

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

  1. bug-658124-revision-karma
  2. Merge into devel
Proposed by Stuart Bishop
Status: Merged
Approved by: Robert Collins
Approved revision: no longer in the source branch.
Merged at revision: 11694
Proposed branch: lp:~stub/launchpad/bug-658124-revision-karma
Merge into: lp:launchpad
Diff against target: 83 lines (+13/-14)
3 files modified
lib/lp/code/interfaces/revision.py (+1/-1)
lib/lp/code/model/revision.py (+11/-12)
lib/lp/code/scripts/revisionkarma.py (+1/-1)
To merge this branch: bzr merge lp:~stub/launchpad/bug-658124-revision-karma
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Robert Collins (community) release-critical Approve
Review via email: mp+38181@code.launchpad.net

Commit message

Fix performance of revision karma allocator with PostgreSQL 8.4

Description of the change

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

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

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)
Revision history for this message
Tim Penhey (thumper) wrote :

Looks reasonable

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
154 :return: ResultSet containing tuples of (Revision, RevisionAuthor)154 :return: ResultSet containing tuples of (Revision, RevisionAuthor)
155 """155 """
156156
157 def getRevisionsNeedingKarmaAllocated():157 def getRevisionsNeedingKarmaAllocated(limit=None):
158 """Get the revisions needing karma allocated.158 """Get the revisions needing karma allocated.
159159
160 Under normal circumstances karma is allocated for revisions by the160 Under normal circumstances karma is allocated for revisions by the
161161
=== 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 @@
32 And,32 And,
33 Asc,33 Asc,
34 Desc,34 Desc,
35 Exists,
36 Join,35 Join,
37 Not,36 Not,
38 Or,37 Or,
@@ -65,7 +64,7 @@
65 EmailAddressStatus,64 EmailAddressStatus,
66 IEmailAddressSet,65 IEmailAddressSet,
67 )66 )
68from canonical.launchpad.interfaces.lpstorm import IMasterStore67from canonical.launchpad.interfaces.lpstorm import IMasterStore, IStore
69from canonical.launchpad.webapp.interfaces import (68from canonical.launchpad.webapp.interfaces import (
70 DEFAULT_FLAVOR,69 DEFAULT_FLAVOR,
71 IStoreSelector,70 IStoreSelector,
@@ -426,26 +425,26 @@
426 return result_set.order_by(Desc(Revision.revision_date))425 return result_set.order_by(Desc(Revision.revision_date))
427426
428 @staticmethod427 @staticmethod
429 def getRevisionsNeedingKarmaAllocated():428 def getRevisionsNeedingKarmaAllocated(limit=None):
430 """See `IRevisionSet`."""429 """See `IRevisionSet`."""
431 # Here to stop circular imports.430 # Here to stop circular imports.
432 from lp.code.model.branch import Branch431 from lp.code.model.branch import Branch
433 from lp.code.model.branchrevision import BranchRevision432 from lp.code.model.branchrevision import BranchRevision
434 from lp.registry.model.person import ValidPersonCache433 from lp.registry.model.person import ValidPersonCache
435434
436 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)435 store = IStore(Revision)
437 return store.find(436 results_with_dupes = store.find(
438 Revision,437 Revision,
439 Revision.revision_author == RevisionAuthor.id,438 Revision.revision_author == RevisionAuthor.id,
440 RevisionAuthor.person == ValidPersonCache.id,439 RevisionAuthor.person == ValidPersonCache.id,
441 Not(Revision.karma_allocated),440 Not(Revision.karma_allocated),
442 Exists(441 BranchRevision.revision == Revision.id,
443 Select(True,442 BranchRevision.branch == Branch.id,
444 And(BranchRevision.revision == Revision.id,443 Or(Branch.product != None, Branch.distroseries != None))[:limit]
445 BranchRevision.branch == Branch.id,444 # Eliminate duplicate rows, returning <= limit rows
446 Or(Branch.product != None,445 return store.find(
447 Branch.distroseries != None)),446 Revision, Revision.id.is_in(
448 (Branch, BranchRevision))))447 results_with_dupes.get_select_expr(Revision.id)))
449448
450 @staticmethod449 @staticmethod
451 def getPublicRevisionsForPerson(person, day_limit=30):450 def getPublicRevisionsForPerson(person, day_limit=30):
452451
=== 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 @@
40 # Break into bits.40 # Break into bits.
41 while True:41 while True:
42 revisions = list(42 revisions = list(
43 revision_set.getRevisionsNeedingKarmaAllocated()[:100])43 revision_set.getRevisionsNeedingKarmaAllocated(100))
44 if len(revisions) == 0:44 if len(revisions) == 0:
45 break45 break
46 for revision in revisions:46 for revision in revisions: