Launchpad itself

Merge lp:~lifeless/launchpad/registry into lp:launchpad

  1. registry
  2. Merge into devel
Proposed by Robert Collins
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: 11389
Proposed branch: lp:~lifeless/launchpad/registry
Merge into: lp:launchpad
Diff against target: 89 lines (+26/-9)
1 file modified
lib/lp/registry/model/person.py (+26/-9)
To merge this branch: bzr merge lp:~lifeless/launchpad/registry
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) Approve
Review via email: mp+32953@code.launchpad.net

Commit message

Drop the use of ValidPersonCache from Person._all_members as we usually link EmailAddress in anyway so we can get a more efficient query this way. (6000ms->35ms) for ubuntu-dev

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Two changes discussed on IRC:
 * Indentation—don't mix "right after (" with "tab-indented on following lines."
 * AND should be And.

With that, you have my blessing.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 05:26:48 +0000
@@ -190,6 +190,12 @@
190 """Flags if a Person is active and usable in Launchpad.190 """Flags if a Person is active and usable in Launchpad.
191191
192 This is readonly, as this is a view in the database.192 This is readonly, as this is a view in the database.
193
194 Note that it performs poorly at least some of the time, and if
195 EmailAddress and Person are already being queried, its probably better to
196 query Account directly. See bug
197 https://bugs.edge.launchpad.net/launchpad-registry/+bug/615237 for some
198 corroborating information.
193 """199 """
194200
195201
@@ -1489,7 +1495,7 @@
1489 columns.append(KarmaTotalCache)1495 columns.append(KarmaTotalCache)
1490 if need_ubuntu_coc:1496 if need_ubuntu_coc:
1491 columns.append(Alias(Exists(Select(SignedCodeOfConduct,1497 columns.append(Alias(Exists(Select(SignedCodeOfConduct,
1492 AND(Person._is_ubuntu_coc_signer_condition(),1498 And(Person._is_ubuntu_coc_signer_condition(),
1493 SignedCodeOfConduct.ownerID == Person.id))),1499 SignedCodeOfConduct.ownerID == Person.id))),
1494 name='is_ubuntu_coc_signer'))1500 name='is_ubuntu_coc_signer'))
1495 if need_location:1501 if need_location:
@@ -1509,8 +1515,9 @@
1509 Archive.id == Select(Min(Archive.id),1515 Archive.id == Select(Min(Archive.id),
1510 Archive.owner == Person.id, Archive),1516 Archive.owner == Person.id, Archive),
1511 Archive.purpose == ArchivePurpose.PPA)))1517 Archive.purpose == ArchivePurpose.PPA)))
1512 if need_preferred_email:1518 # checking validity requires having a preferred email.
1513 # Teams don't have email.1519 if need_preferred_email or need_validity:
1520 # Teams don't have email, so a left join
1514 origin.append(1521 origin.append(
1515 LeftJoin(EmailAddress, EmailAddress.person == Person.id))1522 LeftJoin(EmailAddress, EmailAddress.person == Person.id))
1516 columns.append(EmailAddress)1523 columns.append(EmailAddress)
@@ -1518,10 +1525,14 @@
1518 Or(EmailAddress.status == None,1525 Or(EmailAddress.status == None,
1519 EmailAddress.status == EmailAddressStatus.PREFERRED))1526 EmailAddress.status == EmailAddressStatus.PREFERRED))
1520 if need_validity:1527 if need_validity:
1521 # May find invalid persons1528 # May find teams (teams are not valid people)
1522 origin.append(1529 origin.append(
1523 LeftJoin(ValidPersonCache, ValidPersonCache.id == Person.id))1530 LeftJoin(Account, Person.account == Account.id))
1524 columns.append(ValidPersonCache)1531 columns.append(Account)
1532 conditions = And(conditions,
1533 Or(
1534 Account.status == None,
1535 Account.status == AccountStatus.ACTIVE))
1525 if len(columns) == 1:1536 if len(columns) == 1:
1526 columns = columns[0]1537 columns = columns[0]
1527 # Return a simple ResultSet1538 # Return a simple ResultSet
@@ -1562,8 +1573,14 @@
1562 email = row[index]1573 email = row[index]
1563 index += 11574 index += 1
1564 cache_property(result, '_preferredemail_cached', email)1575 cache_property(result, '_preferredemail_cached', email)
1576 #-- validity caching
1565 if need_validity:1577 if need_validity:
1566 valid = row[index] is not None1578 # valid if:
1579 valid = (
1580 # -- valid account found
1581 row[index] is not None
1582 # -- preferred email found
1583 and result.preferredemail is not None)
1567 index += 11584 index += 1
1568 cache_property(result, '_is_valid_person_cached', valid)1585 cache_property(result, '_is_valid_person_cached', valid)
1569 return result1586 return result
@@ -2409,7 +2426,7 @@
2409 """See `IPerson`."""2426 """See `IPerson`."""
2410 # Also assigned to by self._all_members.2427 # Also assigned to by self._all_members.
2411 store = Store.of(self)2428 store = Store.of(self)
2412 query = AND(SignedCodeOfConduct.ownerID == self.id,2429 query = And(SignedCodeOfConduct.ownerID == self.id,
2413 Person._is_ubuntu_coc_signer_condition())2430 Person._is_ubuntu_coc_signer_condition())
2414 # TODO: Using exists would be faster than count().2431 # TODO: Using exists would be faster than count().
2415 return bool(store.find(SignedCodeOfConduct, query).count())2432 return bool(store.find(SignedCodeOfConduct, query).count())
@@ -2419,7 +2436,7 @@
2419 """Generate a Storm Expr for determing the coc signing status."""2436 """Generate a Storm Expr for determing the coc signing status."""
2420 sigset = getUtility(ISignedCodeOfConductSet)2437 sigset = getUtility(ISignedCodeOfConductSet)
2421 lastdate = sigset.getLastAcceptedDate()2438 lastdate = sigset.getLastAcceptedDate()
2422 return AND(SignedCodeOfConduct.active == True,2439 return And(SignedCodeOfConduct.active == True,
2423 SignedCodeOfConduct.datecreated >= lastdate)2440 SignedCodeOfConduct.datecreated >= lastdate)
24242441
2425 @property2442 @property