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

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
1=== modified file 'lib/lp/code/interfaces/revision.py'
2--- lib/lp/code/interfaces/revision.py 2010-09-10 20:03:43 +0000
3+++ lib/lp/code/interfaces/revision.py 2010-10-12 05:41:07 +0000
4@@ -154,7 +154,7 @@
5 :return: ResultSet containing tuples of (Revision, RevisionAuthor)
6 """
7
8- def getRevisionsNeedingKarmaAllocated():
9+ def getRevisionsNeedingKarmaAllocated(limit=None):
10 """Get the revisions needing karma allocated.
11
12 Under normal circumstances karma is allocated for revisions by the
13
14=== modified file 'lib/lp/code/model/revision.py'
15--- lib/lp/code/model/revision.py 2010-10-03 15:30:06 +0000
16+++ lib/lp/code/model/revision.py 2010-10-12 05:41:07 +0000
17@@ -32,7 +32,6 @@
18 And,
19 Asc,
20 Desc,
21- Exists,
22 Join,
23 Not,
24 Or,
25@@ -65,7 +64,7 @@
26 EmailAddressStatus,
27 IEmailAddressSet,
28 )
29-from canonical.launchpad.interfaces.lpstorm import IMasterStore
30+from canonical.launchpad.interfaces.lpstorm import IMasterStore, IStore
31 from canonical.launchpad.webapp.interfaces import (
32 DEFAULT_FLAVOR,
33 IStoreSelector,
34@@ -426,26 +425,26 @@
35 return result_set.order_by(Desc(Revision.revision_date))
36
37 @staticmethod
38- def getRevisionsNeedingKarmaAllocated():
39+ def getRevisionsNeedingKarmaAllocated(limit=None):
40 """See `IRevisionSet`."""
41 # Here to stop circular imports.
42 from lp.code.model.branch import Branch
43 from lp.code.model.branchrevision import BranchRevision
44 from lp.registry.model.person import ValidPersonCache
45
46- store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
47- return store.find(
48+ store = IStore(Revision)
49+ results_with_dupes = store.find(
50 Revision,
51 Revision.revision_author == RevisionAuthor.id,
52 RevisionAuthor.person == ValidPersonCache.id,
53 Not(Revision.karma_allocated),
54- Exists(
55- Select(True,
56- And(BranchRevision.revision == Revision.id,
57- BranchRevision.branch == Branch.id,
58- Or(Branch.product != None,
59- Branch.distroseries != None)),
60- (Branch, BranchRevision))))
61+ BranchRevision.revision == Revision.id,
62+ BranchRevision.branch == Branch.id,
63+ Or(Branch.product != None, Branch.distroseries != None))[:limit]
64+ # Eliminate duplicate rows, returning <= limit rows
65+ return store.find(
66+ Revision, Revision.id.is_in(
67+ results_with_dupes.get_select_expr(Revision.id)))
68
69 @staticmethod
70 def getPublicRevisionsForPerson(person, day_limit=30):
71
72=== modified file 'lib/lp/code/scripts/revisionkarma.py'
73--- lib/lp/code/scripts/revisionkarma.py 2010-08-20 20:31:18 +0000
74+++ lib/lp/code/scripts/revisionkarma.py 2010-10-12 05:41:07 +0000
75@@ -40,7 +40,7 @@
76 # Break into bits.
77 while True:
78 revisions = list(
79- revision_set.getRevisionsNeedingKarmaAllocated()[:100])
80+ revision_set.getRevisionsNeedingKarmaAllocated(100))
81 if len(revisions) == 0:
82 break
83 for revision in revisions: