/srv/irclogs.ubuntu.com/2013/03/05/#launchpad-dev.txt

=== wedgwood is now known as wedgwood_away
StevenKwgrant: https://code.launchpad.net/~stevenk/launchpad/notify-when-spec-renamed-redux/+merge/15167804:37
wgrantStevenK: k04:50
StevenKwgrant: Too late04:50
StevenKself-reviewed and landed04:50
wgrantAh, so you did04:50
StevenKLooking for another critical04:50
StevenKOpen for suggestions04:50
StevenKwgrant: It's essential? How?05:56
StevenKNBS doesn't apply05:57
wgrantStevenK: Why not?05:58
wgrantThe UI only exposes source deletion05:58
wgrantBut sometimes you need to delete binaries05:58
wgrantBecause they're left over even though their source is long gone05:58
wgrantSo you need to be able to delete an already dead source, if it has live binaries05:59
StevenKwgrant: Sure. But IArchive.getSourcesForDeletion will only include sources if and only if it has a published binary.05:59
StevenKIf we can destroy that check it might actually be performant, because then it will only require SPPH and SPN06:00
wgrant        clauses.append("""06:00
wgrant           (%s OR SourcePackagePublishingHistory.status IN %s)06:00
wgrant        """ % (has_published_binaries_clause,06:00
wgrant               quote(source_deletable_states)))06:00
wgrantOR06:00
StevenKWe still pull in SPR06:00
StevenKAlways06:00
wgrantSure06:00
StevenKAnd the EXPLAIN ALANYZE for the query makes me cry blood.06:00
wgrantHow's that relevant?06:01
wgrantSure06:01
wgrantThe plan is bad06:01
StevenKModulo spelling06:01
wgrantThat doesn't mean the idea of the query is06:01
StevenKwgrant: So it has to stay? :-(06:01
wgrantWe need a way to delete NBS binaries06:01
wgrantThe way we do that today is by permitting deletion of a source if it's published, or if it has published binaries.06:02
StevenKwgrant: Then we're stuck with SPR. :-(06:04
wgrantStevenK: SPR isn't the bad bit06:04
wgrantBut yes06:05
StevenKwgrant: Oh? The EXISTS?06:05
wgrantWell, think about what the query has to do06:05
wgrantI haven't explained it06:05
wgrantBut it is fairly obvious what the issue is06:05
StevenKFROM SourcePackageName,06:06
StevenK     SourcePackageRelease,06:06
StevenK...06:06
StevenKLEFT JOIN SourcePackageRelease AS "_prejoin1" ON SourcePackagePublishingHistory.sourcepackagerelease = "_prejoin1".id06:06
StevenKI'm not sure if Postgres enjoys that much06:06
StevenKwgrant: Which I seem to be missing.06:07
wgrantStevenK: Consider what the query is doing06:09
wgrantThe root condition is very expensive06:10
StevenKSourcePackagePublishingHistory.archive = 24522 ?06:10
wgrantIt's asking for every SPPH in this archive that is published, or has published binaries06:10
wgrantNow, finding the published ones is relatively cheap06:10
wgrantIt's somewhere around O(number of published sources), which is the dataset we actually care about06:11
wgrantSo that's fine06:11
wgrantFinding the published binaries for a source is also *reasonably* inexpensive06:11
wgrantBut the problem is that we have to find the published binaries for not just every source06:11
wgrantEr06:11
wgrantNot just a source06:11
wgrantAnd not just the published sources06:11
StevenKBut every source06:11
wgrantBut every source every in the history of the archive06:12
wgrantSo it's O(entire source history)06:12
wgrantFor a fairly slow operation06:12
wgrantCan you think of any potential optimisations?06:12
wgrantThere might be one we could sort of do.06:12
StevenKCTE of published sources won't help, because we want to include deleted sources06:13
wgrantRight06:13
wgrantFinding published sources is easy06:13
wgrantFinding sources with published binaries is not, because we have to join through several tables and filter from both ends.06:14
StevenKRight06:14
StevenKwgrant: Can we check status first and only drop to EXISTS if we need to?06:15
wgrantStevenK: That won't help in most cases06:15
wgrantStevenK: You'll still have to do the published binaries check for every source that is not published06:15
StevenKAnd we probably want to restrict the EXISTS to BPB.source_package_release = SPPH.spr06:16
wgrantWhich will usually vastly dominate the number of published sources for which you can skip the check06:16
wgrantWhy?06:16
wgrant            SourcePackagePublishingHistory.sourcepackagerelease =06:16
wgrant                SourcePackageRelease.id AND06:16
wgrant                BinaryPackageBuild.source_package_release =06:16
wgrant                    SourcePackageRelease.id)06:16
wgrantThat's already done06:16
StevenKEven in the subquery?06:17
wgrantSourcePackageRelease isn't rebound in the subquery06:17
wgrantThat's why it works at all06:17
wgrantRather than just selecting every publication in the archive ever06:18
StevenKI'm also unsure why this query bothers to link against SPR if it's in the FROM spec06:19
StevenKs/link/JOIN/06:19
StevenKwgrant: So the subquery can't be restricted?06:21
wgrantHMm?06:21
wgrantIt doesn't specifically JOIN it06:21
wgrantRestricted how?06:21
StevenKLEFT JOIN SourcePackageRelease AS "_prejoin1" ON SourcePackagePublishingHistory.sourcepackagerelease = "_prejoin1".id06:21
wgrantAh, a prejoin06:21
StevenKAnd the code has preJoins = ['sourcepackagerelease']06:21
wgrantThat's unrelated06:21
wgrantIgnore it06:21
StevenKwgrant: That we can't restrict the EXISTS subquery against the SPR in the outer query06:22
wgrantIt is06:22
wgrantOtherwise it would return every published binary in archive, rather than just binaries for the relevant source06:22
wgrantaka. completely broken06:23
StevenKThen I'm not sure :-(06:23
wgrantWithout completely redesigning the UI around handling NBS binaries, the only potential optimisation I can think of is to restrict to sources with a null scheduleddeletiondate.06:24
wgrantBecause we don't condemn sources while their binaries are alive06:24
wgrantFor GPL compliance06:24
wgrantThat *may* be enough to fix it in most cases.06:24
wgrant(for now)06:25
StevenKwgrant: In the outer query?06:26
wgrantStevenK: Probably06:27
wgrantFor the other case it doesn't really matter06:27
wgrantsince if we have a Published source that's condemned then we have a big problem06:27
StevenKwgrant: That runs quickly on qas, but I'm not sure if its still hot enough06:29
wgrantYou'll also want to find the most pathological archive06:30
wgrantAnd see if it still works06:30
wgrantWell, most pathological PPA06:30
StevenK32 seconds06:37
StevenKFor ~ubuntu-langpack's PPA.06:37
StevenKwgrant: So I guess the SourcePackagePublishingHistory.scheduleddeletiondate IS NULL doesn't help06:43
wgrantHave you checked the plan?06:47
StevenKwgrant: http://pastebin.ubuntu.com/5587192/06:49
wgrant                           ->  Index Scan using securesourcepackagepublishinghistory__archive__status__idx on sourcepackagepublishinghistory  (cost=0.00..69803.45 rows=51405 width=135) (actual time=0.129..2090.566 rows=3060 loops=1)06:53
wgrant                                 Index Cond: (archive = 32)06:53
wgrant                                 Filter: (scheduleddeletiondate IS NULL)06:53
StevenKAh07:03
StevenKwgrant: So we need another index?08:19
StevenKI thought index scans with filters were still cheap-ish08:19
=== almaisan-away is now known as al-maisan
wgrantStevenK: Think about what we're trying to do.08:30
wgrant"Cheap-ish" depending on the selectivity of the filter08:30
StevenKRight08:36
mpthttps://launchpad.net/~78luphr0rnk2nuqimstywepozxn9kl19tqh0tx66b5dki1xxsh5mkz9gl21a5rlwfnr8jn6ln0m3jxne2k9x1ohg85w3jabxlrqbgszpjpwcmvkb-launchpad-a811i2i3ytqlsztthjth0svbccw8inm65tmkqp9sarr553jq53in4xm1m8wn3o4rlwaer06ogwvqwv9mrqoku2x334n7di44o65qze67n1wneepm is a good test for Launchpad page layouts...10:53
czajkowskimpt: lots of them around10:54
StevenK47510:54
StevenKI'm tempted to re-seed them via their e-mail address so they get renamed to an account name that isn't completly and utterly bonkers.10:55
=== wedgwood_away is now known as wedgwood
=== al-maisan is now known as almaisan-away
=== joey[a] is now known as joey
=== wedgwood is now known as wedgwood_away
=== wedgwood_away is now known as wedgwood
=== wedgwood is now known as wedgwood_away
=== wedgwood_away is now known as wedgwood
=== deryck is now known as deryck[lunch]
=== Ursinha_ is now known as Ursinha
=== deryck[lunch] is now known as deryck
=== BradCrittenden is now known as bac
RoelVdear launchpad devs! I have a question21:48
RoelVwe currently have a problem where we would like an additional "importance" status21:49
RoelV(Percona)21:49
RoelVso that we can register "QA blocking bugs" - so a status would be "QA blocking"21:49
RoelVor even simply "QA request"21:49
RoelVwe would not like to use tags for this21:49
RoelVis there any way to do this - for example by using some API, patch LP code, ... ?21:50
czajkowskiRoelV: http://blog.launchpad.net/bug-tracking/new-bugs-status-opinion21:50
RoelVczajkowski, thanks! so iow "it ain't going to happen?"21:52
RoelVczajkowski, any other "best solutions/practices" you can think off?21:53
czajkowskibesides dropping a mail to the list no afraid not21:54
czajkowskibut the blog post sums it up tbh21:54
RoelVczajkowski, yep thanks21:56
lifelesswgrant: StevenK: rabbitfixture is missing a LICENSE file22:32
wgrantRoelV: Why not just use tags?23:35
=== wedgwood is now known as wedgwood_away

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!