[00:04] wgrant: flat and flat2 ? [00:05] lifeless: 2 is just a second temporary index on that table [00:06] gin (access_grants) and gin (access_policies) respectively. [00:10] lifeless: So, BugSummary v2 needs to avoid the journal write slowness that we see today, which probably means not having an exploded journal, instead just journalling the new and old states. This leaves a couple of options for displaying up to date numbers: rely on garbo-frequently to keep the table up to date, and hope that users don't notice the numbers are a couple of minutes out of date; or use array indices to quickly count the ... [00:10] ... relevant non-exploded rows in the journal. [00:10] Thoughts? === vednis is now known as mars [01:05] wgrant: do we know why its slow? [01:06] wgrant: when we originally did it it was under some time pressure [01:06] so it probably hasn't had even a single pass optimisation run over it [01:14] lifeless: Say I have an 80-task Linux task, because the kernel team are evil. [01:14] It has 6 tags. [01:15] Each change to the bug is likely to generate 960 journal rows [01:15] If it's a security bug, it might be private. [01:15] 5 subscribers [01:15] 5000 journal rows [01:17] This goes into the list of "things that probably don't scale very well" [01:23] wallyworld, wgrant: No complaints about my mail? I'm shocked. [01:26] lifeless: eg. each change to bug #732628 that requires a BugSummary update (eg. adding a tag) takes 650ms to journal 770 rows. [01:26] <_mup_> Bug #732628: TOCTOU in mount.ecryptfs_private

Add 3 tags, 2970 rows total. [01:37] StevenK: haven't read it yet [01:37] Typical. :-P [01:39] StevenK: seems a reasonable summary. devil will be in the detail [01:44] wgrant: how many are -1+1 pairs ? [01:46] lifeless: count | count [01:46] -------+------- [01:46] 1 | 1650 [01:46] -1 | 1320 [02:07] so, lots [02:07] is there some way we could filter those out ? [02:08] Sure. [02:08] IIRC we call two helpers, one to un-count thebug and one to recount it [02:08] We can use our sensible data access layer to not do this in the DB :) [02:08] *cough* [02:08] simple but apparently high overhead [02:09] lifeless: The difficulty is that we don't know when the update is done. [02:09] So we can't avoid the writes. [02:10] we can reduce them [02:10] If our data access layer existed we could implement it there. [02:10] indeed [02:10] so, at the moment [02:10] UPDATE FOO [02:10] -> trigger which gets old, new [02:10] Yes [02:10] old gets dereferneced, new gets referenced [02:10] that trigger could optimise [02:11] How? [02:11] It can't potentially delete the old ones. [02:11] Oh, I guess it could. [02:11] Since they're not committed yet. [02:12] That would almost halve the writes. [02:12] we could have the helpers return the tuples, not add them, calculate in-memory, then write out the remainder [02:16] We could. [02:16] would that be sufficient? [02:17] There's no persistent cross-statement storage for that purpose, apart from temp tables. [02:17] its all one statement [02:18] the UPDATE (or add/delete in the case of tags) [02:18] True. No cross-trigger storage, then. [02:19] will it truely optimimal it pathological cases? (email command sequences) - no, but does that matter? [02:20] lifeless: INSERTing two tags will fire each trigger twice. [02:20] How do you propose to optimise that? [02:21] do we need to? do we know we need to? [02:21] I don't understand how your optimisation thing can work at all. [02:21] INSERT INTO BugTag knws it only needs the +1s [02:22] Oh, for that case, sure. [02:22] But now I change an attribute of the bug. [02:22] thats an UPDATE [02:22] And it has to journal both sides. [02:22] why ? [02:22] Because it's removing from some counts, and adding to others? [02:23] it has to journal -1s for the no longer relevant summary rows [02:23] but it doesn't have to journal -1s for every summary row [02:23] Anyway, do we know that this journalling approach has benefits over my proposal? [02:24] Mine is a lot easier on writes and probably easier on reads too. [02:25] so your proposal is 'journal the old,new inputs' ? [02:25] Yes. [02:25] that will be pretty big too [02:26] The number of writes scales linearly with number of tasks. [02:26] + subscribers + tags [02:26] And each will only be slightly larger than a single existing journal row [02:26] Yes. [02:26] Well, not subscribers, grants/polciies. [02:26] which is a lot larger than a single journal row [02:27] For things with a lot of grants or tags, the row will be a fair bit larger, yes. [02:27] 2*bugtask + 2*tags + 2*policies [02:28] so this case, will be 160 bugtasks, + the other bits [02:28] Currently it's bugtask*tags*(policies+subscribers) [02:28] * summary width [02:28] *2 [02:29] (for non-delete/add cases) [02:30] journaling the state will be 2*(bugtasks*bigtask width+area(tags) + policies*grant width) [02:32] wgrant: anyhow, I've no objection if you want to journal an array of bugtasks + array of tags + array of policies and adjust accordingly. [02:32] wgrant: I'm not convinced its better (or worse) [02:32] wgrant: but thats because I haven't modelled it sufficiently [02:32] lifeless: The journalled thing would be a subset of bugsummary + array of tags. [02:33] So roughly (subset of bug, subset of bugtask, [tags], [grants], [policies]) [02:36] so when a tag changes, you'll journal multiple rows? [02:37] lifeless: Yes, since I can't efficiently index into inhomogenous arrays like an array of task attributes. [02:38] (or even filter, if we have few enough rows to not care about indices) [02:38] so, how much space does 160 of these rows take [02:39] for someone adding a tag and deleting a tag [02:43] I can never remember the precise disk format of arrays. [02:43] But IIRC it's just a 4 byte size followed by the fields. [02:44] So the row size increase for the bug I mentioned above will be (#tasks * (task width + 4 bytes + all the tags)). [02:44] Currently we have approx (#tasks * #tags * (task width + avg tag size)) [02:47] Also the base postgres per-row overhead is not insignificant :) [02:48] The postgres overhead makes up >30% of the average bugsummary tuple size now. [02:49] Likely around 50%, in fact. [02:50] Average bugsummary tuple is 62 bytes (including header), header is 27 bytes + a couple of bytes + 8 byte alignment. [03:23] could we do away with bugsummary ? [03:24] No. [03:25] lifeless: After you spent all that time writing it, and then rewriting it six times? [03:27] I suspect, due to writing just a few bugsummaries, that coalescing around statements would be a bigger win overall, but it may be more work [03:27] StevenK: Its a tool, not an end goal. [03:27] StevenK: dropping it if its no longer a win would be entirely appropriate [03:28] lifeless: Coalescing around statements? [03:29] lifeless: I'm teasing [03:29] wgrant: what I described before [03:30] wgrant: anyhow, the constraints I see are: [03:30] lifeless: I don't see how thta does away with bugsummary... [03:30] - alterations do not timeout [03:30] - things work. [03:30] done [03:30] beyond that I've no particular view on it [03:31] 13:23:39 < lifeless> could we do away with bugsummary ? [03:31] 13:27:21 < lifeless> I suspect, due to writing just a few bugsummaries, that coalescing around statements would be a bigger win overall, but it may be more work [03:31] Are those two statements unrelated? [03:35] no [03:35] you said no [03:35] so I went back to discussing how to evolve it [05:06] Hm [05:07] Does https://launchpad.net/ubuntu/+patches do anything that a bug search for latest_patch_uploaded IS NOT NULL ordered by latest_patch_uploaded and showing a new "Patch age" field wouldn't do? [05:09] I think I might add the 4 lines of code needed to support that on +bugs, and delete the view. [05:10] Is it linked from anywhere, and do people use it? [05:10] It is linked from the portlet, and people do use it. [05:10] I would change the link to point to a search, like the rest of them already do. [05:10] Oh, right. [05:10] Sounds good, then. [05:12] Ah, hm. [05:12] The Patch age column is a link with a tooltip. [05:12] And dynamic bug listings pull all the data every time, so it would be a little inefficient to pull all the LFAs. [05:31] steven@undermined:~% indicator-weather [05:31] Another instance of this program is already running [05:31] zsh: exit 1 indicator-weather [05:31] RARGH [05:31] * StevenK stabs it. [05:32] I gave up with that [05:33] It keeps crashing and bringing up apport too [05:33] sent an error report ? [05:34] wgrant: can annotate the LFAs in a post-load hook, but yea, not trivial [05:35] lifeless: We can, but it's still a fair bit more data to get from the DB and send to the browser. [05:35] For something that's used on probably 0.1% of requests. [05:35] lifeless: I've sent two or three [05:36] wgrant: right, thats why I'm saying load it only when used [05:37] lifeless: The infrastructure doesn't support that :( [05:37] Would need significant changes. [05:38] wgrant: I don't think so: we do a very similar thing for linkification [05:39] wgrant: get page -> after render -> send request -> markup results [05:39] lifeless: Oh, true, could do it like that. [05:40] is it pretty and integrated and all that - no [05:40] would it be nice to do that? yes. [05:40] but ... [05:40] it would be doable and fairly small to code [05:40] and likely more efficient overall [05:40] Yeah. [05:41] wgrant: voice call - bugsummary2 ? === jtv1 is now known as jtv [05:41] Sure [05:42] I'm online [07:28] does db-link-sppu bypass auditord? [07:29] Oh my god, its full of jargon... [07:29] lifeless: No? [07:29] It is complementary, it just doesn't depend on it [07:35] StevenK: ah, it just being on the bug that the service was discussed on confused me. [07:35] StevenK: I thought it was one of the things we'd be storing in auditord [07:35] lifeless: It is certainly related to the bug. [07:36] lifeless: No, we will be storing details relating to the PU in auditor, so we need a way to get at the PU from SPPH. [07:36] cool cool [07:36] The PG 9.2 feature list in the beta2 announcement does look nice. Even the short version. [07:39] Oh, that's actually been released? [07:39] I was looking through commit logs to see what had changed. [07:39] Because the only reference to a changelog was "oh, I'm here's a draft announcement, but I'm still writing the changelog" [07:40] Heh: "I have completed the Postgres 9.2 release notes I started seven days ago." [07:42] hi all, anyone able to review https://code.launchpad.net/~danilo/launchpad/bug-999040/+merge/105637 for me? [07:44] stub: Array stats, yay! [07:44] and sensible prepared statement plans. [07:45] And of course index-only scans, but they go without saying. [07:46] "Allow analyze statistics to be collected for foreign tables" could be interesting [07:47] I hope we never use foreign tables :) [07:47] Also, DROP INDEX CONCURRENTLY. [07:47] Oooh [07:47] WANT [07:47] And quick CHECK adding [07:48] The ALTER TABLE lock reduction stuff didn't make it in, though. [07:52] good morning [07:59] stub: "Have pg_upgrade create a script to incrementally generate more accurate optimizer statistics" [07:59] So no need for waiting for ANALYZE [08:01] wgrant, gmb: you guys are still listed as on-call reviewers for Tuesday on https://dev.launchpad.net/ReviewerSchedule, so I hope you don't mind if I pester you to get https://code.launchpad.net/~danilo/launchpad/bug-999040/+merge/105637 reviewed :) [08:05] danilos: Done [08:06] wgrant, thanks, will you land it for me as well please? (as for LOC addition, before we started this work we [salgado] removed a bunch of LOC, and I am pretty sure we are still a net negative; I'd have to check though) [08:08] wgrant: question. in a test case, i can makeAccessArtifactGrant for a bug but for a branch i also have to call makeAccessPolicyArtifact in order for findArtifactsByGrantee to work. why is that? [08:09] wallyworld: Branches don't have triggers to set up the APAs. [08:09] wallyworld: Bugs do (for now) [08:10] ok. will put a note in the test. thanks [08:10] danilos: Sure. [08:10] wgrant, thanks [08:11] danilos: You need to set a commit message. [08:15] wgrant, sorry, done [08:16] danilos: It's on its way. [08:16] cool, ta === almaisan-away is now known as al-maisan === gmb_ is now known as gmb [08:34] wgrant, btw, I believe bug 998052 is still at 'fix committed' instead of released [08:34] <_mup_> Bug #998052: Upcoming Work View now showing all desired work items < https://launchpad.net/bugs/998052 > [08:36] danilos: Ah, it was deployed but rolled back shortly afterwards due to an unrelated regression. It should be redeployed in a few hours if all goes well. [08:36] wgrant, ok, that explains it, thanks === Topic unset by gmb on #launchpad-dev [10:18] Oh arse === gmb changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: gmb | Firefighting: - | Critical bugs: 3.47*10^2 [10:18] I fail at IRC. Again. [10:19] Blame jetlag. [10:19] Always jetlag. === rick_h_ changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: gmb, rharding | Firefighting: - | Critical bugs: 3.47*10^2 [12:10] danilos: ec2 seems to hate you. Let me retry it. [12:10] wgrant, :( [12:11] wgrant, I'll pretend to like it if that's going to help [12:37] rick_h_: Thanks for the review! I'll get to work :) [12:40] allenap: thanks for all the fresh code [12:40] talk about a wake up in the morning [12:41] and I feel I lacked as a reviewer to not have anything to complain about on something new/large like that :P [13:06] jcsackett: ping [13:06] hola rick_h_ [13:06] jcsackett: so couple of things [13:06] never good, that. :-P [13:06] 1) I bet the jsbuild_watch doesn't work for you because you don't have the combo loader FF enalbed in dev? [13:07] that auto build only builds into the combo load directory and doesn't rebuild launchpad.js [13:07] too slow to do that [13:07] aah. [13:07] didn't realize that. [13:08] and no, i didn't set any feature flags. [13:08] 2) Just a heads up, in checking the YUI widget source, the default for the visible property is true, wonder if that was giving you grief after seeing the comments on not listening to the html/etc [13:08] rick_h_: that's one thing, but i set it to false in ATTRS. [13:08] jcsackett: ok, that makes me feel better (in a way) and sorry I didn't think of it until last night [13:08] rick_h_: dig, so i'll set that up, b/c i'm doing more js hacking. :-P [13:09] jcsackett: yea, I feel like this is a combo issue, at some point I hope to poke at it. I was going to note in my reivew that you didn't need to have the ATTR, but I saw you were setting it false vs true [13:09] jcsackett: awesome, hopefully it goes smoother then [13:09] rick_h_: as to the visible, the *other* thing is that listening "on" means the visible change event has happened, but it's not guaranteed visible *has* changed yet. [13:09] thus, listening to "after" means you're trigger based on the change, but the change is definitely done. [13:09] jcsackett: yea, there's also afterChange [13:09] jcsackett: right [13:10] rick_h_: yeah, it seems stable with this.after('visibleChange'). i owe reading longpoll code for figuring that one out. :-P [13:10] heh [13:11] cool, well glad it's fitting together better. Hate it when things *should* work but don't [13:14] rick_h_: it's not they don't work--it's that i insufficiently understand the parameters in which they *should*. :-P [13:14] rick_h_: incidentally, apologies for the blitzkrieg of js branches in the queue today. :-P [13:14] jcsackett: hah, all good, these look pretty now that there's nice module/code reuse [13:14] yeah, i'm way happier with them. [13:59] rick_h_: reply to your comments on privacy-banner branch. in particular as regards the build/ in tests. happy to talk here after you've read the comments, or we can converse via MP. :-) [14:00] jcsackett: k, loading it up [14:02] jcsackett: so the build thing was something we chatted about in our squad when doing combo loader/test updates. [14:02] rick_h_, abentley, adeuring -- sorry, missed it was stand-up. coming now.... [14:02] the idea is that those are actually 'deps' and should come from a static/standard location like the download-cache kind of idea [14:02] deryck: ooh, me too [14:03] rick_h_: except that totally breaks code/reload. i don't know if bin/test rebuilds js everytime, but if you're checking things in the browser (which is way easier than trying to read the error messages from bin/test) you now have to rebuild every time. [14:03] jcsackett: and that we want to make sure that files don't break during 'build' so that they fail in tests as they would in production [14:04] if something causes a file to not get put into the build directory right, tests will pass, but production usage will fail [14:04] jcsackett: right, but the _watch should fix that up tbh [14:04] and combo loader is coming [14:05] so it's kind of a convention to help make sure that building will work correctly along with tests passing I guess [14:05] you're probably right that a -dev discussion is a good idea [14:05] rick_h_: right, but _watch has undocumented provisos to it working (as we demoed :-P) and combo loader isn't going to fix coding/reloading a test_html file, is it? [14:06] jcsackett: right, that's what I mean. I'm telling you the idea behind the enforcement, but admitting it's not something that's been vetted/put into stone [14:06] rick_h_: i think if you want this to be a policy to keep people from doing it the way i have, we *def* need to talk about it and get group buy in. :-) [14:06] jcsackett: no, but you're not linking to a test file, but a dependency which is copied to the build directory [14:06] rick_h_: dig. to be clear, i got the idea behind your request--i'm just pointing out there are some non-trivial disadvantages. [14:06] jcsackett: I'm not totally clear on the issue though [14:07] so you change code, reload, your test is linking to deps in a build dir [14:07] you're not editing the deps, just the current module under test which is ok to be relative since it's right there [14:08] the only time it requires a rebuild is if you edit a dependency which should have its own test file [14:08] rick_h_: hm. i suppose in this instance, i was dealing with test files that were/are poorly separated. so yeah, my experience may be nonstandard. [14:09] jcsackett: yea, I guess the idea is that deps come from build and only the code 'under test' is linked directly as a build step sanity check [14:10] rick_h_: fair. okay, i'll make those changes since outside of something like what i was doing where i had to modify and test a bunch of files it isn't actually such an issue. [14:24] jcsackett: ok, sorry, stand up. The rest of that thanks for the notes/feedback to the questions there. [14:24] rick_h_: all good. i'm in the process of changing over to build paths. [14:25] jcsackett: thanks, does the idea click after the discussion? I'll look to add some notes somewhere on the jsbuild_watch stuff and can bring up the build link discussion to -dev [14:25] you're getting to be the first user that didn't write the stuff so lucky you with the growing pains :) [14:25] rick_h_: yeah, i think the problem is it is still a gotcha if you're doing work over several modules and their tests. [14:25] but that's a less common workflow, so i'm making the update. [15:08] gmb: Before I ask for a review of the changes I have made for Bug #999717, I need to get a test written. It will require a JS test I think, but I would appreciate some pointers. [15:08] <_mup_> Bug #999717: upcomingwork view blueprints should be initially expanded if incomplete < https://launchpad.net/bugs/999717 > [15:23] jamestunnicliffe, Okay. Let me take a look at the branch. === al-maisan is now known as almaisan-away [15:35] jamestunnicliffe, So, I think that rather than doing the detection of not-100%-completeness in the JS we can do it in the view code and make the testing a bit easier. Submit it for review and I'll suggest a patch for it (easier than me explaining it line-by-line in IRC). [15:36] gmb: Will do, thanks. [15:36] np === matsubara is now known as matsubara-lunch === matsubara-lunch is now known as matsubara === Topic unset by gmb on #launchpad-dev [17:09] AAAAAAAA === gmb changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: - | Firefighting: - | Critical bugs: 3.47*10^2 === rick_h_ changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: rharding | Firefighting: - | Critical bugs: 3.47*10^2 [17:11] rick_h_, do you have time to review https://code.launchpad.net/~sinzui/launchpad/halt-when-done-2/+merge/105859 [17:11] sinzui: loading now [17:13] * deryck steps away to eat [17:17] sinzui: r=me [17:17] thank you [18:56] Is there a polite way of attracting the attention of any webops who may be around? [18:56] (re #launchpad current conversation; psusi wants to know why he can't access bug 999790 despite receiving email about it) [19:39] rick_h_, do you have time to review https://code.launchpad.net/~sinzui/launchpad/daily-build-form/+merge/105884 [19:52] abentley: this is the stuff i have so far for the test if a job with a retry error is queued again: http://paste.ubuntu.com/989542/ . Note the "poor man's debug attempt" in BaseRunnableJob.xxxRunExtraCommit() (the exception): Allows me to check if the fleature flag is correctly set. WHat I get in a test run: Exception: waaaaa2 TestJobWithRetryError False None . In other words: the feature flag "vanished"... Any idea? [19:54] adeuring: looking. [20:00] adeuring: Feature flags are memoized, so it's possible that it could start with None, then the flag could get updated, but the worker would still have the stale value. Are you running this test by itself or with other tests? [20:00] abentley: just the two tests in the module. The flag is set in both tests... [20:02] abentley: and the flag is also missing if i run just the one test... [20:03] adeuring: My new task_init code just landed in devel, and that explicitly clears the feature flag cache, so could you try merging devel? [20:03] abentley: ok, I'll try it. [20:04] has anyone succeeded in getting the python-oops-tools tests passing locally? [20:04] sinzui: loading [20:05] I can't get them to pass before I've changed any code [20:06] abentley: yay, that did it! [20:06] james_w: sinzui r=me [20:06] adeuring: great! [20:07] possibly because of the db being emptied after each test [20:07] thank you rick_h_ [20:28] lifeless, https://code.launchpad.net/~james-w/python-oops-tools/remove-lp-branding/+merge/105892 === rick_h_ changed the topic of #launchpad-dev to: http://dev.launchpad.net/ | On call reviewer: - | Firefighting: - | Critical bugs: 3.47*10^2 === matsubara is now known as matsubara-afk === Ursinha` is now known as Ursinha === Ursinha is now known as Guest8786 [21:20] sinzui: Bah, I thought I tracked down all the ES5 array ops :( [21:47] lifeless, I have to run to dinner but fwiw, I'm still seeing the odd -tags. They don't hurt anything except themselves, but... http://pastebin.ubuntu.com/989769/ gotta run