Too much repeated SQL in send-bug-notifications job
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Launchpad itself |
Fix Released
|
Critical
|
Gary Poster |
Bug Description
This issue was found when investigating the incident about slow sending of bug emails, see https:/
When the job is run, for each user to be notified, the following SQL is run:
SELECT <stuff> FROM Person WHERE Person.id = 1259057 LIMIT 1
SELECT <stuff> FROM EmailAddress WHERE EmailAddress.person = 1259057 AND EmailAddress.status = 4 ORDER BY EmailAddress.email
and
SELECT <stuff> FROM BugNotification
There are 100s of such statements. These should be done as a single SQL select eg where person.id in (...)
I'm marking as critical because it resulted from an incident report. It may be that further analysis means this can be reprioritised lower.
Related branches
- j.c.sackett (community): Approve
-
Diff: 450 lines (+183/-67)6 files modifiedlib/lp/bugs/interfaces/bugnotification.py (+3/-3)
lib/lp/bugs/model/bug.py (+1/-1)
lib/lp/bugs/model/bugnotification.py (+73/-34)
lib/lp/bugs/scripts/bugnotification.py (+42/-14)
lib/lp/bugs/scripts/tests/test_bugnotification.py (+29/-12)
lib/lp/bugs/tests/test_bugnotification.py (+35/-3)
- j.c.sackett (community): Approve
-
Diff: 429 lines (+151/-74)8 files modifieddatabase/schema/security.cfg (+2/-0)
lib/canonical/launchpad/helpers.py (+5/-28)
lib/canonical/launchpad/tests/test_helpers.py (+0/-34)
lib/lp/bugs/scripts/bugnotification.py (+4/-5)
lib/lp/registry/model/person.py (+64/-0)
lib/lp/registry/tests/test_person.py (+69/-1)
lib/lp/services/mail/notificationrecipientset.py (+5/-4)
lib/lp/soyuz/scripts/ppareport.py (+2/-2)
- Brad Crittenden (community): Approve (code)
-
Diff: 467 lines (+189/-115)5 files modifiedlib/lp/bugs/interfaces/bugnotification.py (+9/-3)
lib/lp/bugs/model/bugnotification.py (+61/-10)
lib/lp/bugs/scripts/bugnotification.py (+31/-41)
lib/lp/bugs/scripts/tests/test_bugnotification.py (+4/-3)
lib/lp/bugs/tests/test_bugnotification.py (+84/-58)
Changed in launchpad: | |
status: | New → Triaged |
Changed in launchpad: | |
assignee: | Launchpad Yellow Squad (yellow) → Gary Poster (gary) |
Changed in launchpad: | |
status: | Triaged → In Progress |
Changed in launchpad: | |
status: | Fix Committed → In Progress |
tags: |
added: qa-ok removed: qa-needstesting |
Changed in launchpad: | |
status: | Fix Committed → Fix Released |
If you need to ensure 100s of objects are cached, you will likely need to increase the Storm cache size too. I think it will be set to 500 at the moment - it is controlled by the storm_cache_size property in the current launchpad.conf database config section. I don't know which database config section is in use by this script though - looks like the database username is configured in the [malone] section (as well as being shared by bug expiration... grrr...), so this looks non-standard and it is probably inheriting the default from the [database] section.