launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/launchpad/registry into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #615237 /participants API timing out
  https://bugs.launchpad.net/bugs/615237


So the ValidPersonCache component turns out to turn a 35ms thing into a 6000ms thing. Not so good: this unpacks the view, and performs snappily, fixing timeouts for this on staging.
-- 
https://code.launchpad.net/~lifeless/launchpad/registry/+merge/32953
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/registry into lp:launchpad/devel.

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2010-08-17 13:53:04 +0000
+++ lib/lp/registry/model/person.py	2010-08-18 04:59:45 +0000
@@ -190,6 +190,12 @@
     """Flags if a Person is active and usable in Launchpad.
 
     This is readonly, as this is a view in the database.
+
+    Note that it performs poorly at least some of the time, and if
+    EmailAddress and Person are already being queried, its probably better to
+    query Account directly. See bug
+    https://bugs.edge.launchpad.net/launchpad-registry/+bug/615237 for some
+    corroborating information.
     """
 
 
@@ -1509,8 +1515,9 @@
                 Archive.id == Select(Min(Archive.id),
                     Archive.owner == Person.id, Archive),
                 Archive.purpose == ArchivePurpose.PPA)))
-        if need_preferred_email:
-            # Teams don't have email.
+        # checking validity requires having a preferred email.
+        if need_preferred_email or need_validity:
+            # Teams don't have email, so a left join
             origin.append(
                 LeftJoin(EmailAddress, EmailAddress.person == Person.id))
             columns.append(EmailAddress)
@@ -1520,8 +1527,11 @@
         if need_validity:
             # May find invalid persons
             origin.append(
-                LeftJoin(ValidPersonCache, ValidPersonCache.id == Person.id))
-            columns.append(ValidPersonCache)
+                LeftJoin(Account, Person.account == Account.id))
+            columns.append(Account)
+            conditions = AND(conditions,
+                Or(Account.status == None,
+                    Account.status == AccountStatus.ACTIVE))
         if len(columns) == 1:
             columns = columns[0]
             # Return a simple ResultSet
@@ -1562,8 +1572,13 @@
                 email = row[index]
                 index += 1
                 cache_property(result, '_preferredemail_cached', email)
+            #-- validity caching
             if need_validity:
-                valid = row[index] is not None
+                # valid if:
+                # -- valid account found
+                valid = (row[index] is not None
+                    # -- preferred email found
+                    and result.preferredemail is not None)
                 index += 1
                 cache_property(result, '_is_valid_person_cached', valid)
             return result