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