=== wedgwood is now known as wedgwood_away [04:37] wgrant: https://code.launchpad.net/~stevenk/launchpad/notify-when-spec-renamed-redux/+merge/151678 [04:50] StevenK: k [04:50] wgrant: Too late [04:50] self-reviewed and landed [04:50] Ah, so you did [04:50] Looking for another critical [04:50] Open for suggestions [05:56] wgrant: It's essential? How? [05:57] NBS doesn't apply [05:58] StevenK: Why not? [05:58] The UI only exposes source deletion [05:58] But sometimes you need to delete binaries [05:58] Because they're left over even though their source is long gone [05:59] So you need to be able to delete an already dead source, if it has live binaries [05:59] wgrant: Sure. But IArchive.getSourcesForDeletion will only include sources if and only if it has a published binary. [06:00] If we can destroy that check it might actually be performant, because then it will only require SPPH and SPN [06:00] clauses.append(""" [06:00] (%s OR SourcePackagePublishingHistory.status IN %s) [06:00] """ % (has_published_binaries_clause, [06:00] quote(source_deletable_states))) [06:00] OR [06:00] We still pull in SPR [06:00] Always [06:00] Sure [06:00] And the EXPLAIN ALANYZE for the query makes me cry blood. [06:01] How's that relevant? [06:01] Sure [06:01] The plan is bad [06:01] Modulo spelling [06:01] That doesn't mean the idea of the query is [06:01] wgrant: So it has to stay? :-( [06:01] We need a way to delete NBS binaries [06:02] The way we do that today is by permitting deletion of a source if it's published, or if it has published binaries. [06:04] wgrant: Then we're stuck with SPR. :-( [06:04] StevenK: SPR isn't the bad bit [06:05] But yes [06:05] wgrant: Oh? The EXISTS? [06:05] Well, think about what the query has to do [06:05] I haven't explained it [06:05] But it is fairly obvious what the issue is [06:06] FROM SourcePackageName, [06:06] SourcePackageRelease, [06:06] ... [06:06] LEFT JOIN SourcePackageRelease AS "_prejoin1" ON SourcePackagePublishingHistory.sourcepackagerelease = "_prejoin1".id [06:06] I'm not sure if Postgres enjoys that much [06:07] wgrant: Which I seem to be missing. [06:09] StevenK: Consider what the query is doing [06:10] The root condition is very expensive [06:10] SourcePackagePublishingHistory.archive = 24522 ? [06:10] It's asking for every SPPH in this archive that is published, or has published binaries [06:10] Now, finding the published ones is relatively cheap [06:11] It's somewhere around O(number of published sources), which is the dataset we actually care about [06:11] So that's fine [06:11] Finding the published binaries for a source is also *reasonably* inexpensive [06:11] But the problem is that we have to find the published binaries for not just every source [06:11] Er [06:11] Not just a source [06:11] And not just the published sources [06:11] But every source [06:12] But every source every in the history of the archive [06:12] So it's O(entire source history) [06:12] For a fairly slow operation [06:12] Can you think of any potential optimisations? [06:12] There might be one we could sort of do. [06:13] CTE of published sources won't help, because we want to include deleted sources [06:13] Right [06:13] Finding published sources is easy [06:14] Finding sources with published binaries is not, because we have to join through several tables and filter from both ends. [06:14] Right [06:15] wgrant: Can we check status first and only drop to EXISTS if we need to? [06:15] StevenK: That won't help in most cases [06:15] StevenK: You'll still have to do the published binaries check for every source that is not published [06:16] And we probably want to restrict the EXISTS to BPB.source_package_release = SPPH.spr [06:16] Which will usually vastly dominate the number of published sources for which you can skip the check [06:16] Why? [06:16] SourcePackagePublishingHistory.sourcepackagerelease = [06:16] SourcePackageRelease.id AND [06:16] BinaryPackageBuild.source_package_release = [06:16] SourcePackageRelease.id) [06:16] That's already done [06:17] Even in the subquery? [06:17] SourcePackageRelease isn't rebound in the subquery [06:17] That's why it works at all [06:18] Rather than just selecting every publication in the archive ever [06:19] I'm also unsure why this query bothers to link against SPR if it's in the FROM spec [06:19] s/link/JOIN/ [06:21] wgrant: So the subquery can't be restricted? [06:21] HMm? [06:21] It doesn't specifically JOIN it [06:21] Restricted how? [06:21] LEFT JOIN SourcePackageRelease AS "_prejoin1" ON SourcePackagePublishingHistory.sourcepackagerelease = "_prejoin1".id [06:21] Ah, a prejoin [06:21] And the code has preJoins = ['sourcepackagerelease'] [06:21] That's unrelated [06:21] Ignore it [06:22] wgrant: That we can't restrict the EXISTS subquery against the SPR in the outer query [06:22] It is [06:22] Otherwise it would return every published binary in archive, rather than just binaries for the relevant source [06:23] aka. completely broken [06:23] Then I'm not sure :-( [06:24] Without 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] Because we don't condemn sources while their binaries are alive [06:24] For GPL compliance [06:24] That *may* be enough to fix it in most cases. [06:25] (for now) [06:26] wgrant: In the outer query? [06:27] StevenK: Probably [06:27] For the other case it doesn't really matter [06:27] since if we have a Published source that's condemned then we have a big problem [06:29] wgrant: That runs quickly on qas, but I'm not sure if its still hot enough [06:30] You'll also want to find the most pathological archive [06:30] And see if it still works [06:30] Well, most pathological PPA [06:37] 32 seconds [06:37] For ~ubuntu-langpack's PPA. [06:43] wgrant: So I guess the SourcePackagePublishingHistory.scheduleddeletiondate IS NULL doesn't help [06:47] Have you checked the plan? [06:49] wgrant: http://pastebin.ubuntu.com/5587192/ [06:53] -> 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] Index Cond: (archive = 32) [06:53] Filter: (scheduleddeletiondate IS NULL) [07:03] Ah [08:19] wgrant: So we need another index? [08:19] I thought index scans with filters were still cheap-ish === almaisan-away is now known as al-maisan [08:30] StevenK: Think about what we're trying to do. [08:30] "Cheap-ish" depending on the selectivity of the filter [08:36] Right [10:53] https://launchpad.net/~78luphr0rnk2nuqimstywepozxn9kl19tqh0tx66b5dki1xxsh5mkz9gl21a5rlwfnr8jn6ln0m3jxne2k9x1ohg85w3jabxlrqbgszpjpwcmvkb-launchpad-a811i2i3ytqlsztthjth0svbccw8inm65tmkqp9sarr553jq53in4xm1m8wn3o4rlwaer06ogwvqwv9mrqoku2x334n7di44o65qze67n1wneepm is a good test for Launchpad page layouts... [10:54] mpt: lots of them around [10:54] 475 [10:55] I'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. === 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 [21:48] dear launchpad devs! I have a question [21:49] we currently have a problem where we would like an additional "importance" status [21:49] (Percona) [21:49] so that we can register "QA blocking bugs" - so a status would be "QA blocking" [21:49] or even simply "QA request" [21:49] we would not like to use tags for this [21:50] is there any way to do this - for example by using some API, patch LP code, ... ? [21:50] RoelV: http://blog.launchpad.net/bug-tracking/new-bugs-status-opinion [21:52] czajkowski, thanks! so iow "it ain't going to happen?" [21:53] czajkowski, any other "best solutions/practices" you can think off? [21:54] besides dropping a mail to the list no afraid not [21:54] but the blog post sums it up tbh [21:56] czajkowski, yep thanks [22:32] wgrant: StevenK: rabbitfixture is missing a LICENSE file [23:35] RoelV: Why not just use tags? === wedgwood is now known as wedgwood_away