launchpad-dev team mailing list archive

[Edwin Grubbs <edwin.grubbs@xxxxxxxxxxxxx>]

On Tue, Aug 10, 2010 at 5:20 PM, Robert Collins
<robert.collins@xxxxxxxxxxxxx> wrote:
> On Wed, Aug 11, 2010 at 10:08 AM, Edwin Grubbs
> <edwin.grubbs@xxxxxxxxxxxxx> wrote:
>> After looking at bugs 353950 and 615654 and their oopses, it appears
>> that the two main causes of the timeouts are
>> TeamMembership._fillTeamParticipation() and
>> TeamMembership._sendStatusChangeNotification(), which sends emails to
>> all the members of a team individually if it does not have a preferred
>> email address.
>
> So, in general - neither of these things is directly what the user
> asked us to do - they are respectively housekeeping a cache and
> performing notifications about the change.
>
> Ideally we would record the /intent/ - the patch if you like - in a
> work queue after validating that it will be ok, and then both of these
> things can be processed out of band.
>
> This shouldn't be terribly hard, but its not trivial either. I'd like
> to make it easier.
>
>> The _sendStatusChangeNotification() method would be easy to batch, and
>> that would change the experience in the UI at all. However, the
>> _fillTeamParticipation() method is currently loading all the members
>> and submembers of a team and iterates over them, running a separate
>> query to check if the new superteam has an entry in the
>> TeamParticipation table and then inserting an entry by creating a
>> storm object that isn't going to be used after the insert. It seems
>> like even huge teams could be handled rather quickly with a query such
>> as:
>>
>>
>> INSERT INTO TeamParticipation (person, team)
>> SELECT Person, NEW_TEAM_ID
>> FROM TeamParticipation tp1
>> WHERE team = OLD_TEAM_ID
>>    AND NOT EXISTS (
>>        SELECT 1
>>        FROM TeamParticipation tp2
>>        WHERE tp2.person = tp1.person
>>            AND tp2.team = NEW_TEAM_ID
>>            );
>
> Makes sense to me at a casual glance; this is orthogonal to /when/ we
> do the work though, IMO.


The reason I think we shouldn't batch filling in the TeamParticipation
table unless absolutely necessary is that the members of the newly
added team don't get any of the benefits until that is done. We also
have to provide some kind of status indicator and possibly an
estimated time till completion. If a specific person needs the
superteam's privileges right away, the superteam's admin may feel
forced into adding that person directly instead of waiting for the
queue to process the team.

-Edwin

>> Is there is any reason that this hasn't been done already? Also, is
>> there anything else that could be taking up a lot of time that is not
>> readily apparent from the oopses?
>
> Things that are not well enumerated in oopses:
>  - web service calls (librarian, google)
>  - GIL contention (e.g. if other threads are stomping on you)
>  - hitting swap or other external effects
>
> Possibly more, but thats all I'm aware of now.
>
> -Rob
>