Launchpad itself

Merge lp:~thumper/launchpad/fix-mailman-excessive-queries into lp:launchpad

  1. fix-mailman-excessive-queries
  2. Merge into devel
Proposed by Tim Penhey
Status: Merged
Approved by: Brad Crittenden
Approved revision: no longer in the source branch.
Merged at revision: 11803
Proposed branch: lp:~thumper/launchpad/fix-mailman-excessive-queries
Merge into: lp:launchpad
Diff against target: 127 lines (+49/-16)
5 files modified
lib/lp/registry/doc/message-holds.txt (+4/-4)
lib/lp/registry/interfaces/mailinglist.py (+11/-0)
lib/lp/registry/model/mailinglist.py (+29/-1)
lib/lp/registry/tests/mailinglists_helper.py (+1/-6)
lib/lp/registry/xmlrpc/mailinglist.py (+4/-5)
To merge this branch: bzr merge lp:~thumper/launchpad/fix-mailman-excessive-queries
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+39335@code.launchpad.net

Commit message

Make getMessageDispositions much more efficient in the number of DB queries.

Description of the change

The current implementation of getMessageDispositions on the MailingList xmlrpc api is excessively simplistic in its implementation. It gets an unlimited number of MessageApproval objects and then iterates over them both updating a status and querying for the team name.

This causes OOPSes like OOPS-1758XMLP341.

I changed the implementation of the IMessageApprovalSet to do the joins and return just the message_id and team names as needed. I also implemented a new method that does a set based update.

tests:
  message-holds
  mailinglists

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Really nice branch Tim.

I think you have a typo in your new method name that got propagated.
acknowledgeMessagesWithStats ->
acknowledgeMessagesWithStatus

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/doc/message-holds.txt'
2--- lib/lp/registry/doc/message-holds.txt 2010-10-18 22:24:59 +0000
3+++ lib/lp/registry/doc/message-holds.txt 2010-10-26 03:53:45 +0000
4@@ -292,10 +292,10 @@
5 ... print message_id, list_name, subject
6
7 >>> def print_messages(status):
8- ... held_messages = sorted(hold_set.getHeldMessagesWithStatus(status),
9- ... key=attrgetter('message_id'))
10- ... for message_hold in held_messages:
11- ... print_hold(message_hold)
12+ ... held_messages = sorted(hold_set.getHeldMessagesWithStatus(status))
13+ ... for message_id, team_name in held_messages:
14+ ... held_message = hold_set.getMessageByMessageID(message_id)
15+ ... print_hold(held_message)
16
17 Here are all the messages pending approval...
18
19
20=== modified file 'lib/lp/registry/interfaces/mailinglist.py'
21--- lib/lp/registry/interfaces/mailinglist.py 2010-09-17 03:58:05 +0000
22+++ lib/lp/registry/interfaces/mailinglist.py 2010-10-26 03:53:45 +0000
23@@ -808,6 +808,17 @@
24 :rtype: sequence of MessageApproval
25 """
26
27+ def acknowledgeMessagesWithStatus(status):
28+ """Acknowledge all the MessageApprovals with the matching status.
29+
30+ This changes the statuses APPROVAL_PENDING to APPROVED,
31+ REJECTION_PENDING to REJECTED and DISCARD_PENDING to DISCARD. It is
32+ illegal to call this function when the status is not one of these
33+ states.
34+
35+ :param status: A PostedMessageStatus enum value.
36+ """
37+
38
39 class IHeldMessageDetails(Interface):
40 """Details on a held message.
41
42=== modified file 'lib/lp/registry/model/mailinglist.py'
43--- lib/lp/registry/model/mailinglist.py 2010-09-17 03:58:05 +0000
44+++ lib/lp/registry/model/mailinglist.py 2010-10-26 03:53:45 +0000
45@@ -73,6 +73,7 @@
46 EmailAddressStatus,
47 IEmailAddressSet,
48 )
49+from canonical.launchpad.interfaces.lpstorm import IMasterStore
50 from canonical.launchpad.webapp.interfaces import (
51 IStoreSelector,
52 MAIN_STORE,
53@@ -887,7 +888,34 @@
54
55 def getHeldMessagesWithStatus(self, status):
56 """See `IMessageApprovalSet`."""
57- return MessageApproval.selectBy(status=status)
58+ # Use the master store as the messages will also be acknowledged and
59+ # we want to make sure we are acknowledging the same messages that we
60+ # iterate over.
61+ return IMasterStore(MessageApproval).find(
62+ (Message.rfc822msgid, Person.name),
63+ MessageApproval.status == status,
64+ MessageApproval.message == Message.id,
65+ MessageApproval.mailing_list == MailingList.id,
66+ MailingList.team == Person.id)
67+
68+ def acknowledgeMessagesWithStatus(self, status):
69+ """See `IMessageApprovalSet`."""
70+ transitions = {
71+ PostedMessageStatus.APPROVAL_PENDING:
72+ PostedMessageStatus.APPROVED,
73+ PostedMessageStatus.REJECTION_PENDING:
74+ PostedMessageStatus.REJECTED,
75+ PostedMessageStatus.DISCARD_PENDING:
76+ PostedMessageStatus.DISCARDED,
77+ }
78+ try:
79+ next_state = transitions[status]
80+ except KeyError:
81+ raise AssertionError(
82+ 'Not an acknowledgeable state: %s' % status)
83+ approvals = IMasterStore(MessageApproval).find(
84+ MessageApproval, MessageApproval.status == status)
85+ approvals.set(status=next_state)
86
87
88 class HeldMessageDetails:
89
90=== modified file 'lib/lp/registry/tests/mailinglists_helper.py'
91--- lib/lp/registry/tests/mailinglists_helper.py 2010-09-20 19:21:57 +0000
92+++ lib/lp/registry/tests/mailinglists_helper.py 2010-10-26 03:53:45 +0000
93@@ -263,15 +263,10 @@
94 mailing_list.transitionToStatus(MailingListStatus.ACTIVE)
95 # Simulate acknowledging held messages.
96 message_set = getUtility(IMessageApprovalSet)
97- message_ids = set()
98 for status in (PostedMessageStatus.APPROVAL_PENDING,
99 PostedMessageStatus.REJECTION_PENDING,
100 PostedMessageStatus.DISCARD_PENDING):
101- for message in message_set.getHeldMessagesWithStatus(status):
102- message_ids.add(message.message_id)
103- for message_id in message_ids:
104- message = message_set.getMessageByMessageID(message_id)
105- message.acknowledge()
106+ message_set.acknowledgeMessagesWithStatus(status)
107
108
109 mailman = MailmanStub()
110
111=== modified file 'lib/lp/registry/xmlrpc/mailinglist.py'
112--- lib/lp/registry/xmlrpc/mailinglist.py 2010-10-03 15:30:06 +0000
113+++ lib/lp/registry/xmlrpc/mailinglist.py 2010-10-26 03:53:45 +0000
114@@ -275,9 +275,8 @@
115 (PostedMessageStatus.DISCARD_PENDING, 'discard'),
116 )
117 for status, disposition in status_dispositions:
118- for held_message in message_set.getHeldMessagesWithStatus(status):
119- held_message.acknowledge()
120- response[held_message.message_id] = (
121- removeSecurityProxy(held_message.mailing_list.team).name,
122- disposition)
123+ held_messages = message_set.getHeldMessagesWithStatus(status)
124+ for message_id, team_name in held_messages:
125+ response[message_id] = (team_name, disposition)
126+ message_set.acknowledgeMessagesWithStatus(status)
127 return response