[00:18] <LPCIBot> Project windmill build #108: STILL FAILING in 1 hr 9 min: https://lpci.wedontsleep.org/job/windmill/108/
[05:06] <LPCIBot> Project windmill build #109: STILL FAILING in 53 min: https://lpci.wedontsleep.org/job/windmill/109/
[09:41] <lifeless> wgrant: if you're around, I'd like a second eyeballs over https://bugs.launchpad.net/launchpad/+bug/736008/comments/12
[09:41] <_mup_> Bug #736008: Product:+code-index timeouts <qa-ok> <timeout> <Launchpad itself:Fix Committed by lifeless> < https://launchpad.net/bugs/736008 >
[10:09] <LPCIBot> Project db-devel build #496: FAILURE in 4 hr 55 min: https://lpci.wedontsleep.org/job/db-devel/496/
[20:32] <lifeless> moin
[20:52] <jkakar> lifeless: Moin. :)
[20:57] <lifeless> jkakar: how goes the new job?
[21:14] <mwhudson> morning
[21:14] <lifeless> mwhudson: hi
[21:15] <lifeless> mwhudson: got a sec to talk (irc) about Product:+code-index
[21:15] <mwhudson> lifeless: sure
[21:15] <lifeless> so https://bugs.launchpad.net/launchpad/+bug/736008/comments/12 is a new query I've put together that really does look better for determining outstanding merge proposals
[21:15] <_mup_> Bug #736008: Product:+code-index timeouts <qa-ok> <timeout> <Launchpad itself:Fix Committed by lifeless> < https://launchpad.net/bugs/736008 >
[21:15] <mwhudson> (if you'll excuse small gaps in which i make toast)
[21:16]  * mwhudson looks
[21:18] <lifeless> there is one *definitional* difference between that and what runs today: both the source and target branch contexts are constrained
[21:19] <mwhudson> well sure, as we discussed before that sounds find
[21:19] <mwhudson> *fine
[21:20] <mwhudson> lifeless: it looks pretty sane to me, the with stuff makes it nice and readable
[21:21] <mwhudson> lifeless: i guess like you said, we should actually dump that query into a temp table, then count it and select .. limit $N from it
[21:26] <lifeless> lp is the worst project in lp for this query AFAICT
[21:26] <lifeless> its 5.5 seconds cold on qastaging
[21:26] <lifeless> the bit I don't like is we're defining unbounded-growth here
[21:27] <mwhudson> lifeless: does lp have the most private branches?
[21:27] <lifeless>    CTE scope_branches
[21:27] <lifeless>      ->  Bitmap Heap Scan on branch  (cost=143.45..11180.08 rows=7634 width=9) (actual time=4.790..29.200 rows=7634 loops=1)
[21:27] <lifeless>            Recheck Cond: (product = 10294)
[21:27] <lifeless>            ->  Bitmap Index Scan on branch__product__id__idx  (cost=0.00..141.55 rows=7634 width=0) (actual time=2.862..2.862 rows=7634 loops=1)
[21:27] <lifeless>                  Index Cond: (product = 10294)
[21:28] <lifeless> we're defining this on all branches for lp ever
[21:28] <lifeless> rather than 'all unmerged' or something similar
[21:28]  * thumper is off to the physio :-|
[21:28] <lifeless> thumper: be well
[21:28] <thumper> sprained my ankle Thursday last week
[21:28] <thumper> playing at camp
[21:29] <mwhudson> lifeless: well sure, but i'm (probably naively) assuming that most of the cost comes from checking the subsciptions of private branches; maybe not
[21:29] <mwhudson> thumper: at least bigjools wasn't there to land on :)
[21:29] <lifeless> mwhudson: yeah, AFAICT it does, yes.
[21:30] <mwhudson> lifeless: i guess you could filter scope_branches with the status filter -- i guess this is what you meant in comment #8
[21:31] <lifeless> mwhudson: thats the sort of thing, yes
[21:31] <mwhudson> which is still theoretically unbounded but much less likely to be a problem in practice
[21:32] <lifeless> mwhudson: a naive query can run this in a few ms; the with clause approaxh is always about 100ms [on lp branches] - but handles the privacy permission check better
[21:33] <lifeless> mm, not as fast as I thought it an
[21:33] <lifeless> *can*
[21:33] <lifeless> anyhow, I think this is an incremental win
[21:33] <mwhudson> yeah definitely
[21:33]  * mwhudson speculates
[21:34] <mwhudson> what would a completely blue sky fast-from-scratch design of this look like... maintaining the set of visible branches in cassandra or something?
[21:34] <mwhudson> i guess the space requirements would be a bit daft for that
[21:34] <lifeless> dropping the owner check halves the query time
[21:34] <lifeless> so the basic costs are:
[21:34] <lifeless>  - merge proposal visibility is independent of the merge proposal
[21:35] <lifeless>  - merge proposal scope is independent of the merge proposal
[21:35] <mwhudson> would not have guessed the owner check as expensive
[21:36] <lifeless> oh, I made a booboo on that test
[21:36] <lifeless> owner isn't incrementally expensive
[21:38] <lifeless> dropping the subscriber check is 40ms off the hot time
[21:38] <lifeless> and 45% off the cost estimate in the plan
[21:39] <jkakar> lifeless: The new gig is turning out to be really fun.  The team is great and we've begun a process to tackle a bunch of tech debt, which is moving quickly and will improve Fluidinfo in many ways.
[21:40] <lifeless> jkakar: cool
[21:40] <lifeless> mwhudson: so I think in cassandra you'd have a merge proposal column family
[21:41] <lifeless> mwhudson: which stores the visibility inline
[21:41] <lifeless> mwhudson: probably tie visibility to the user
[21:42] <mwhudson> and then fun games to update that when the user joins/leaves a team?
[21:42] <lifeless> mwhudson: e.g. users know what they can see [except admins]
[21:42] <lifeless> mwhudson: we already do updates of denormalised stuff
[21:42] <lifeless> mwhudson: e.g. teamparticipation as the primary exemplar
[21:42] <mwhudson> yes indeed
[21:42] <lifeless> which in fact is the only thing making the performance here tolerable
[21:43] <mwhudson> so you'd have to map something like "place that accesses TP today" -> "something that has to be updated when team membership changes"
[21:44] <lifeless> yes
[21:44] <lifeless> a halfway house might be useful - for particularly big teams, dereference to the team
[21:47] <lifeless> mwhudson: or alternatively, 'what is the disk space cost of storing a primary key for every private branchmergeproposal that a user is given access to'
[21:48] <lifeless> for large teams - hundreds or thousands of members, this could be quite large
[21:48] <mwhudson> yeah, indeed
[21:48] <lifeless> e.g. a MB per merge proposal
[21:48] <mwhudson> and similar for bugs, etc
[21:49] <lifeless> an interesting little thing we could do now though
[21:49] <lifeless> is to add a denormalised 'private' field to the bmp
[21:49] <mwhudson> (big table) x (big table) numbers are pretty scary :)
[21:50] <lifeless> that would let us do a union query, and only pay the privacy lookup for merge proposals of private branches
[21:50] <lifeless> anyhow, I'm going to code up this with clause now
[21:50] <lifeless> I might tap you for a review in a few
[21:50] <mwhudson> sure, sorry for the distraction :)
[21:50] <mwhudson> lifeless: sure, note that i'm only working a half day today
[21:51] <lifeless> no problem, totally on topic
[21:51] <mwhudson> cool
[21:51] <lifeless> mwhudson: kk
[21:51] <lifeless> hmm, that reminds me, I still haven't mailed flacoste about memorial day
[22:06] <wallyworld> thumper: morning. did you want a standup?
[22:06] <lifeless> mwhudson: also - https://bugs.launchpad.net/launchpad/+bug/742916
[22:06] <_mup_> Bug #742916: BranchMergeProposal:+index timeouts - slow query plan <dba> <timeout> <Launchpad itself:Triaged> < https://launchpad.net/bugs/742916 >
[22:06] <lifeless> wallyworld: he's at the physio
[22:07] <wallyworld> lifeless: ah ok. thanks
[22:07]  * wallyworld goes to have breakfast then
[22:09] <mwhudson> lifeless: well, branchrevision is its own well known kettle of fail
[22:10] <mwhudson> one might be able to do something clever with a recursive with query on revision there
[22:11] <mwhudson> hang on, why are we filtering by date?
[22:15] <mwhudson> oh right, the comment makes sense now
[22:15] <mwhudson> won't all the revs have already been accessed for the 'unmerged revisions' section?
[22:16] <mwhudson> i guess that query is limited to 10
[22:39] <lifeless> mwhudson: right, so if we can rephrase it to be a traversal on branch
[22:39] <lifeless> mwhudson: or sequence limited or some such, we can probably make it massively more efficient
[22:39] <lifeless> mwhudson: some branches in lp have /huge/ revision counts
[22:40] <mwhudson> lifeless: it would be interesting to see how a recursive with that walked revision parents performed
[22:40] <lifeless> mwhudson: we seem to bring in maybe 10K fresh revision s aday
[22:40] <lifeless> mwhudson: during some days
[22:41] <mwhudson> ah, you mean lp overall here, not lp-the-project?
[22:41] <lifeless> yeah
[22:41] <lifeless> so if you look at the plan
[22:41] <lifeless> the 9772 or whatever it is, is new-revisions-in-that-period
[22:41] <lifeless> which then get looked up in branchrevision
[22:42] <mwhudson> oh
[22:42] <mwhudson> yeah, that doesn't seem very good
[22:42]  * thumper is trawling through email
[22:42] <lifeless> the obvious other vector - loading all revs in br first then filtering, is also not very good
[22:43] <mwhudson> right, that would work better for a smaller project than lp i guess
[22:43] <mwhudson> s/would/might/
[22:43] <lifeless> and cripple mr onion on bigger ones
[22:48] <lifeless> select branch, count(*) from branchrevision group by branch order by count(*) desc limit 10; - still running
[22:51] <mwhudson> i'm not surprised
[22:51] <mwhudson> launchpad is over half of that table iiuc
[22:51] <mwhudson> err
[22:52] <mwhudson> oh i expect the top few will be kernel imports though
[22:52] <wgrant> Morning.
[22:53] <lifeless> mwhudson: the table is 34GB
[22:53] <lifeless> mwhudson: 10% of LP
[22:53] <lifeless> mwhudson: the *indices* are bigger
[22:54] <lifeless> mwhudson: still -  branch | count
[22:54] <lifeless> --------+--------
[22:54] <lifeless>  275684 | 233206
[22:54] <lifeless>  401221 | 222008
[22:54] <lifeless>  409550 | 212471
[22:54] <lifeless> mwhudson: is where we need to scale up to
[23:08] <lifeless> jkakar: still up ?
[23:09] <thumper> lifeless: abentley and jam worked together for a plan to replace branchrevision
[23:09] <thumper> lifeless: we should really bump up the priority on getting that actually in and working
[23:09] <thumper> lifeless: using bzr-historydb
[23:09] <thumper> lifeless: backed by postgresql
[23:09] <thumper> lifeless: all the existing uses for branch revision would be catered for
[23:09] <thumper> lifeless: and much less disk space
[23:10] <mwhudson> then it would be awesome to hook loggerhead into the same data
[23:10] <lifeless> that would be cool, yes.
[23:10] <lifeless> uhm
[23:10] <wgrant> thumper: Is it safe to use on such an untrusted scale?
[23:10] <lifeless> I haven't looked at the detail of that plan
[23:10] <thumper> mwhudson: yes, the plan was to have loggerhead use the same data
[23:10] <mwhudson> wgrant: it's not like the approach we're using is safe :)
[23:10] <thumper> wgrant: don't know
[23:10] <lifeless> I'd like to do so before talking prioritisation of it - we're not using the existing data structure anywhere near as well as we can
[23:10] <wgrant> eg. what happens if I deliberately introduce revid collisons?
[23:11] <wgrant> mwhudson: True.
[23:11] <thumper> lifeless: go and look at it and chat with abentley and jam, they know everything :)
[23:16] <lifeless> mwhudson: you finish in 45 right?
[23:16] <mwhudson> lifeless: probably more like 75
[23:17] <lifeless> mwhudson: so, I've just figured out the voodoo to reuse select expressions in With clauses
[23:17] <thumper> phew
[23:17] <thumper> unread emails all looked at
[23:18] <lifeless> but I'd like your review on the patch proper; how about I push up [when I've done it] the lp changes without the new storm, you review that expecting me to bump the storm egg trivially, and on we go ?
[23:18] <lifeless> this is the bit for storm:
[23:18] <lifeless> https://pastebin.canonical.com/45270/
[23:21] <mwhudson> lifeless: that sounds sane enough
[23:22] <wgrant> What :/
[23:22] <wgrant> I'm running process-upload.py on DF with LP_DEBUG_SQL=1
[23:22] <wgrant> And processing this source upload is going through hundreds of unrelated binarypackagenames.
[23:22] <lifeless> wgrant: \o/
[23:23] <wgrant> Oh.
[23:23] <wgrant> P-a-s, you need to die.
[23:24] <wgrant> So *that's* why it takes several minutes to run.
[23:39] <lifeless> oh damn
[23:39] <lifeless> some constraints should apply to just the source branch
[23:40] <lifeless> e.g. owned by
[23:40] <lifeless> and branchcollection doesn't distinguish
[23:45]  * lifeless swears about BranchCollection some more
[23:51] <thumper> don't dis the collection
[23:55] <lifeless> wgrant: you're qaing?
[23:57] <wgrant> lifeless: We're up to date.
[23:58] <wgrant> There was only one item that wasn't covered over the weekend.
[23:58] <thumper> lifeless: I have a question for you about a review you did before I was off last week