[04:37] <StevenK> wgrant: https://code.launchpad.net/~stevenk/launchpad/notify-when-spec-renamed-redux/+merge/151678
[04:50] <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
[05:56] <StevenK> wgrant: It's essential? How?
[05:57] <StevenK> NBS doesn't apply
[05:58] <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:59] <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.
[06:00] <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:01] <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:02] <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:04] <StevenK> wgrant: Then we're stuck with SPR. :-(
[06:04] <wgrant> StevenK: SPR isn't the bad bit
[06:05] <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:06] <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:07] <StevenK> wgrant: Which I seem to be missing.
[06:09] <wgrant> StevenK: Consider what the query is doing
[06:10] <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:11] <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:12] <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:13] <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:14] <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:15] <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:16] <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:17] <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:18] <wgrant> Rather than just selecting every publication in the archive ever
[06:19] <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:21] <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:22] <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:23] <wgrant> aka. completely broken
[06:23] <StevenK> Then I'm not sure :-(
[06:24] <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:25] <wgrant> (for now)
[06:26] <StevenK> wgrant: In the outer query?
[06:27] <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:29] <StevenK> wgrant: That runs quickly on qas, but I'm not sure if its still hot enough
[06:30] <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:37] <StevenK> 32 seconds
[06:37] <StevenK> For ~ubuntu-langpack's PPA.
[06:43] <StevenK> wgrant: So I guess the SourcePackagePublishingHistory.scheduleddeletiondate IS NULL doesn't help
[06:47] <wgrant> Have you checked the plan?
[06:49] <StevenK> wgrant: http://pastebin.ubuntu.com/5587192/
[06:53] <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)
[07:03] <StevenK> Ah
[08:19] <StevenK> wgrant: So we need another index?
[08:19] <StevenK> I thought index scans with filters were still cheap-ish
[08:30] <wgrant> StevenK: Think about what we're trying to do.
[08:30] <wgrant> "Cheap-ish" depending on the selectivity of the filter
[08:36] <StevenK> Right
[10:53] <mpt> https://launchpad.net/~78luphr0rnk2nuqimstywepozxn9kl19tqh0tx66b5dki1xxsh5mkz9gl21a5rlwfnr8jn6ln0m3jxne2k9x1ohg85w3jabxlrqbgszpjpwcmvkb-launchpad-a811i2i3ytqlsztthjth0svbccw8inm65tmkqp9sarr553jq53in4xm1m8wn3o4rlwaer06ogwvqwv9mrqoku2x334n7di44o65qze67n1wneepm is a good test for Launchpad page layouts...
[10:54] <czajkowski> mpt: lots of them around
[10:54] <StevenK> 475
[10:55] <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.
[21:48] <RoelV> dear launchpad devs! I have a question
[21:49] <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:50] <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:52] <RoelV> czajkowski, thanks! so iow "it ain't going to happen?"
[21:53] <RoelV> czajkowski, any other "best solutions/practices" you can think off?
[21:54] <czajkowski> besides dropping a mail to the list no afraid not
[21:54] <czajkowski> but the blog post sums it up tbh
[21:56] <RoelV> czajkowski, yep thanks
[22:32] <lifeless> wgrant: StevenK: rabbitfixture is missing a LICENSE file
[23:35] <wgrant> RoelV: Why not just use tags?