+editproposedmembers times out when working with many candidates

I agree that it may need batching. However it can probably actually needs to record intent rather than doing the teamparticipation changes directly: even a single team membership change could in principle require hundreds of sql updates to update the cache, and that may be better done in a queue outside the web request.

This bug relates to bug 353950. Bug 353950 bug is about editing the TeamMembership of a single large team that causes a timeout. This bug is about editing several TeamMemberships that can cause a timeout.

Fixing the issues in the older bug will alleviate some of the issues in this bug.

Fixed in stable r11374 <http://bazaar.launchpad.net/~launchpad-pqm/launchpad/stable/revision/11374>.

I do no think this is fixed yet. I got OOPS-1692S124 using the same team on staging trying to approve 52 members (double the number in the original oops). The branch that landed did imrpove the performance (the companion bug is fixed). The form may be a factor in this problem.

I still haven't made the changes to queue sending email. I probably should have just made this bug and bug 365030 and named the branches part1 and part2.

We are calling Person.setMembershipData() for each Person.proposedmembers to Approve or Decline. That in turn calls TeamMembership.setStatus() for each update. This method calls flush_database_updates() to ensure the data is pushed back the db, then self._sendStatusChangeNotification() is called.

The email operations, represented by the repeated email address lookups is difficult problem. I noted earlier today that since there is not event/subscribe mechanism for membership status changes, mailing-list-auto-subscriptions are broken in 3 use cases, this use case is broken after sending a some number of emails. We really want to decouple sending the notification from the approve/decline event, or queue a job.

I think we could get all the proposedmembers *and* their memberships is a single call, but in this case we would be dealing with 35 of the 398 queries (17 memberships were not reached at the time of timeout).

We could revise the TeamMembershipSet to do batch changes. In this use case, we want to make two calls to the TeamMembershipSet to change the status of a list of users to approves or declined and pass the reviewer and his message in. If the email is still sent in proc, we need al bulk email sender for each batch call.

The _all_members code will help I suspect, if refactored to be able to
select on status;

Likewise, moving to set based operations on the candidates will
massively reduce DB calls.

Here is a more recent oops:
OOPS-1719N1105

On Thu, Sep 16, 2010 at 2:23 PM, Edwin Grubbs <email address hidden> wrote:
> Here is a more recent oops:
> OOPS-1719N1105

Thanks, I've had a look at it - it looks like a pretty typical untuned
page; simply eager loading and mass updating should make it perform a
whole lot better.

On Wed, Sep 15, 2010 at 10:28 PM, Robert Collins
<email address hidden> wrote:
> On Thu, Sep 16, 2010 at 2:23 PM, Edwin Grubbs <email address hidden> wrote:
>> Here is a more recent oops:
>> OOPS-1719N1105
>
> Thanks, I've had a look at it - it looks like a pretty typical untuned
> page; simply eager loading and mass updating should make it perform a
> whole lot better.

While eager loading should be done, there is really no benefit to
processing the emails in the page load. A few minutes delay receiving
emails is not going to negatively affect the user experience as much
as a few seconds delay displaying the page. The number of emails being
sent will increase as teams get bigger, so queuing reduces the
likelihood that we will have to fix this again in the near future.
Eventually, just adding all the participants of large participants of
a new super team will have to be queued also, but hopefully by then,
Launchpad will have established infrastructure to notify users of the
ongoing background processes and their estimated time.

Emails are sent after the page is rendered already, though there is a
bug with that.

But yes, I do agree we need to do more; I'm trying to say that we
should optimise the process *anyway* because inefficient code is still
slow on the backend and that will use up DB resources that are the
only unscalable component we have.

On Thu, Sep 16, 2010 at 1:40 PM, Robert Collins
<email address hidden> wrote:
> Emails are sent after the page is rendered already, though there is a
> bug with that.

Emails may be sent after the page is rendered, but gathering all the
information to send the emails is obviously not occurring before the
page times out. How is the page supposed to be rendered before
simple_sendmail() is called? That is what I mean by sending email in
the context of a view class.

> But yes, I do agree we need to do more; I'm trying to say that we
> should optimise the process *anyway* because inefficient code is still
> slow on the backend and that will use up DB resources that are the
> only unscalable component we have.

I think that requires another bug to track that task, since the oops
should be solved before that optimization takes place.

On Tue, Sep 21, 2010 at 5:11 AM, Edwin Grubbs <email address hidden> wrote:
> On Thu, Sep 16, 2010 at 1:40 PM, Robert Collins
> <email address hidden> wrote:
>> Emails are sent after the page is rendered already, though there is a
>> bug with that.
>
> Emails may be sent after the page is rendered, but gathering all the
> information to send the emails is obviously not occurring before the
> page times out. How is the page supposed to be rendered before
> simple_sendmail() is called? That is what I mean by sending email in
> the context of a view class.

Sure; but gathering that information is cheap; its only slow here
because the page is inefficient.

>> But yes, I do agree we need to do more; I'm trying to say that we
>> should optimise the process *anyway* because inefficient code is still
>> slow on the backend and that will use up DB resources that are the
>> only unscalable component we have.
>
> I think that requires another bug to track that task, since the oops
> should be solved before that optimization takes place.

Possibly; adding a queue requires waiting 3 more weeks before its
usable. Optimising the existing code can be done in a day. However,
you're the dude doing the work, so its your call.

-Rob

Fixed in db-stable r9873 <http://bazaar.launchpad.net/~launchpad-pqm/launchpad/db-stable/revision/9873>.

Fixed in db-stable r9882 <http://bazaar.launchpad.net/~launchpad-pqm/launchpad/db-stable/revision/9882>.