bug.userCanView does too much work when not eager loaded

Bug #619039 reported by Robert Collins
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Launchpad itself
Fix Released
Low
William Grant

Bug Description

bug.userCanView does this:
for subscription in self.subscriptions:
     if user.inTeam(subscription.person):
        return True

This is a problem:
some bugs have hundreds of subscriptions, and inTeam requires separate work as well.

Better to do an explicit direct query on bugsubscription joined with teamparticipation.

With a little glue this can update the users teamparticipation cache too, but this will drop 2 queries to 1, and reduce a many ms query to (probably) a 1-2ms query.

Of course, checking userCanView on bugs at all is bad for performance, but if we're going to do it, this will make it less painful.

Tags: lp-bugs
Revision history for this message
Robert Collins (lifeless) wrote :

Also this code is duplicated verbatim in security.py!

Revision history for this message
Robert Collins (lifeless) wrote :

Now confirmed, this code is a primary cause for milestone timeouts with private bugs.

Revision history for this message
Robert Collins (lifeless) wrote :

I've removed the code duplication and added a code path that should cause few, if any executions of the slow code, but the slow code is still in place. see https://code.edge.launchpad.net/~lifeless/launchpad/milestones.

Deryck Hodge (deryck)
Changed in malone:
status: New → Triaged
importance: Undecided → High
tags: added: timeout
Revision history for this message
Robert Collins (lifeless) wrote :

Removing the timeout tag: the eager loading security check can be applied to other bug lookups and will prevent this causing timeouts (and its the right way to do it: doing a per-bug check would still be very slow - we're not in a procedural world). That said, we should still fix this, it will keep biting in random places until we do.

tags: removed: timeout
summary: - bug.userCanView does too much work
+ bug.userCanView does too much work when not eager loaded
Changed in launchpad:
importance: High → Low
Curtis Hovey (sinzui)
Changed in launchpad:
assignee: nobody → William Grant (wgrant)
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.