| mwhudson | wgrant, StevenK: do you guys keep track of how many lines of source code you've killed from lp over the last little while? | 00:08 |
|---|---|---|
| cjwatson | I have a graph I update every now and again | 00:11 |
| cjwatson | http://people.canonical.com/~cjwatson/tmp/loc-cum.png | 00:13 |
| cjwatson | Data per revision in http://people.canonical.com/~cjwatson/tmp/loc | 00:13 |
| mwhudson | it started around 500k loc ? | 00:14 |
| mwhudson | so down about 15% | 00:14 |
| cjwatson | About a million, I think | 00:14 |
| mwhudson | ah | 00:14 |
| mwhudson | still a nice big number | 00:14 |
| wgrant | Depends on how you count, but by most metrics is 700KLOC-1MLOC | 00:15 |
| StevenK | mwhudson: I've killed close to 40k | 00:31 |
| StevenK | That last big drop on http://people.canonical.com/~cjwatson/tmp/loc-cum.png was me killing YUI 2. | 00:32 |
| lifeless | woo | 00:46 |
| lifeless | killing the source code gently | 00:46 |
| wgrant | StevenK: In https://code.launchpad.net/~stevenk/launchpad/index-previewdiff-merge_proposal/+merge/162915 do you also want to make date_created NOT NULL? | 00:52 |
| wgrant | And you need to rename the column in the ALTER TABLE to branch_merge_proposal | 00:53 |
| wgrant | You should do both NOT NULLs in the same statement to get them done in a single table scan. | 00:53 |
| StevenK | wgrant: Do I need DESC in the index too? | 00:56 |
| wgrant | StevenK: It only makes a very marginal difference in this case, so I wouldn't bother. | 00:56 |
| wgrant | It's important for the bug indices where we have a compound sort in opposing directions. | 00:56 |
| wgrant | StevenK: https://code.launchpad.net/~stevenk/launchpad/switch-bmp-to-previewdiff-merge_proposal/+merge/162916 still pulls out all of the preview diffs just to get the last one | 00:59 |
| wgrant | I'd do | 00:59 |
| wgrant | @cachedproperty | 00:59 |
| wgrant | def preview_diff(self): | 00:59 |
| wgrant | return self._preview_diffs.first() | 00:59 |
| StevenK | wgrant: Ah, but isn't first the earliest diff? | 01:00 |
| wgrant | I can think of an easy solution to that. | 01:00 |
| StevenK | Asc(date_created) ? | 01:01 |
| wgrant | (use .last(), or reverse the sort) | 01:01 |
| StevenK | Ah, so .last() does exist | 01:01 |
| === tasdomas_afk is now known as tasdomas | ||
| StevenK | wgrant: Index fixed | 01:03 |
| wgrant | StevenK: You also probably need to alter the preloading to populate preview_diff rather than preview_diffs | 01:03 |
| wgrant | Anything that wants multiple BMPs isn't going to care about their historical diffs, just their current ones. | 01:03 |
| StevenK | wgrant: Except that I'm pulling all previewdiffs | 01:04 |
| wgrant | That there is precisely the problem. | 01:04 |
| wgrant | It's unnecessarily pulling an unbounded volume of data. | 01:04 |
| wgrant | It's potentially a large set, particularly before we have pruning implemented. | 01:05 |
| StevenK | wgrant: I agree, but at the time I couldn't think of a way to make certain we only return the latest diff | 01:07 |
| wgrant | StevenK: DISTINCT ON | 01:13 |
| wgrant | SELECT DISTINCT ON (branch_merge_proposal) * FROM previewdiff WHERE branch_merge_proposal IN (1, 2, 3, 4) ORDER BY date_created; | 01:14 |
| wgrant | ish | 01:14 |
| wgrant | That'll return the oldest for each MP, but you get the idea | 01:14 |
| wgrant | I believe I added the .config(distinct=PreviewDiff.branch_merge_proposal) syntax to storm a while ago, so that should be easy. | 01:15 |
| StevenK | wgrant: Both index and switch-bmp have been pushed, and the diff has regenerated. | 01:41 |
| wgrant | StevenK: ITYM Desc | 01:44 |
| wgrant | In the previewdiff preloading | 01:44 |
| wgrant | Asc is the default | 01:44 |
| wgrant | I believe that what you have there will grab the oldest, not the newest | 01:44 |
| wgrant | Which perhaps suggests that your preloading test is incomplete. | 01:44 |
| StevenK | Preloading test? :-) | 01:44 |
| StevenK | There's a preloading test? | 01:45 |
| wgrant | Hopefully :) | 01:45 |
| wgrant | If not then it's not difficult to add. | 01:45 |
| StevenK | wgrant: You tell funny jokes. I'll add one. | 01:45 |
| === tasdomas is now known as tasdomas_afk | ||
| wgrant | StevenK, lifeless: Do you recall why ~canonical-launchpad exists? | 03:50 |
| lifeless | No | 03:51 |
| lifeless | is it private? | 03:51 |
| wgrant | No | 03:51 |
| wgrant | AFAICT it was only used to own production-auditor, which I've fixed. | 03:51 |
| lifeless | it says ' | 03:51 |
| lifeless | Canonical employees working directly on Launchpad. | 03:51 |
| lifeless | ' | 03:51 |
| wgrant | Which is roughly the definition of ~launchpad | 03:51 |
| lifeless | yeah | 03:51 |
| lifeless | there are specific branch owning teams already | 03:51 |
| lifeless | which the new project policy page references | 03:52 |
| wgrant | Exactly | 03:52 |
| wgrant | And this isn't a member of any teams, and only had privileges in one project | 03:52 |
| wgrant | So I'm not sure why it exists | 03:52 |
| wgrant | Doesn't own any branches, not subscribed to any bugs... | 03:52 |
| === tasdomas_afk is now known as tasdomas | ||
| StevenK | wgrant: switch-bmp is ready for you again. | 04:56 |
| wgrant | StevenK: Oh, that preloading sort should probably actually be on (bmp, date_created) | 05:03 |
| wgrant | It'll work as it is now, but potentially slowly and it doesn't really make sense | 05:03 |
| StevenK | wgrant: .order_by(PreviewDiff.branch_merge_proposal, Desc(PreviewDiff.date_created) ? | 05:24 |
| wgrant | StevenK: Right | 05:24 |
| StevenK | wgrant: Is that your only remaining concern? | 05:37 |
| wgrant | StevenK: I thiiiiiink so, apart from the pruning thing | 05:37 |
| wgrant | Which we at least need a plan for before this lands | 05:37 |
| StevenK | wgrant: Given your idea that we want only one previewdiff per MP in the short term, I'm happy to put a pipe at the end that adds it | 05:38 |
| wgrant | StevenK: Sounds good | 05:41 |
| wgrant | I'll go over the MPs again for a final check in a sec | 05:41 |
| === tasdomas is now known as tasdomas_afk | ||
| === tasdomas_afk is now known as tasdomas | ||
| StevenK | wgrant: I want GROUP BY for this query to only show old previewdiffs? | 06:01 |
| wgrant | StevenK: DISTINCT ON and GROUP BY are usually mutually exclusive | 06:02 |
| wgrant | They perform a similar role, except that GROUP BY gives you aggregates. | 06:02 |
| StevenK | I don't want DISTINCT branch_merge_proposals, though | 06:04 |
| wgrant | StevenK: DISTINCT ON branch_merge_proposal | 06:05 |
| wgrant | Not DISTINCT branch_merge_proposal | 06:06 |
| StevenK | To handle the case that we have 2 old previewdiffs to be pruned. | 06:06 |
| wgrant | Oh | 06:06 |
| wgrant | To show old ones | 06:06 |
| StevenK | Yes | 06:06 |
| wgrant | You want neither | 06:06 |
| wgrant | You'll need window functions or a subquery | 06:06 |
| StevenK | I can see how a window function can help, how does a subquery? | 06:07 |
| wgrant | You can use a subquery to select the latest previewdiff for the MP and then use NOT IN | 06:07 |
| wgrant | Or != | 06:07 |
| StevenK | Bah | 06:16 |
| StevenK | ERROR: window function requires an OVER clause | 06:16 |
| StevenK | And so I add one: | 06:16 |
| StevenK | ERORR: window functions not allowed in WHERE clause | 06:17 |
| wgrant | Sure, you usually have to use a subquery | 06:18 |
| wgrant | So you return the window function result in the select list of a subquery, and filter on that in the outer query | 06:18 |
| StevenK | wgrant: http://paste.ubuntu.com/5646915/ | 06:24 |
| wgrant | StevenK: Right | 06:25 |
| StevenK | It even works | 06:25 |
| StevenK | wgrant: .config(distinct=PreviewDiff.branch_merge_proposal) is not making use of DISTINCT ON | 06:49 |
| wgrant | StevenK: Hm, it possibly needs to be a tuple | 06:51 |
| wgrant | I forget | 06:51 |
| wgrant | I only wrote it | 06:51 |
| wgrant | 39+ if isinstance(select.distinct, (tuple, list)): | 06:52 |
| wgrant | 40+ tokens.append( | 06:52 |
| wgrant | 41+ "ON (%s) " % compile(select.distinct, state, raw=True)) | 06:52 |
| StevenK | Excellent | 06:53 |
| === tasdomas is now known as tasdomas_afk | ||
| cjwatson | StevenK: YUI 2> Yeah, I went and looked at that commit :) | 08:12 |
| StevenK | cjwatson: Haha, as a "What the hell just happened?" sort of check? | 08:13 |
| cjwatson | Not really, I just tend to look at big negative commits out of interest | 08:16 |
| === tasdomas_afk is now known as tasdomas | ||
| === tasdomas is now known as tasdomas_afk | ||
| === tasdomas_afk is now known as tasdomas | ||
| === wedgwood_away is now known as wedgwood | ||
| === tasdomas is now known as tasdomas_afk | ||
| === wedgwood is now known as wedgwood_away | ||
| === wedgwood_away is now known as wedgwood | ||
| === wedgwood is now known as wedgwood_away | ||
| === tasdomas_afk is now known as tasdomas | ||
| === wedgwood_away is now known as wedgwood | ||
| === wedgwood is now known as wedgwood_away | ||
| === wedgwood_away is now known as wedgwood | ||
| === wedgwood is now known as wedgwood_away | ||
| === tasdomas is now known as tasdomas_afk | ||
| === tasdomas_afk is now known as tasdomas | ||
| === tasdomas is now known as tasdomas_afk | ||
| === tasdomas_afk is now known as tasdomas | ||
| === tasdomas is now known as tasdomas_afk | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
