[00:09] wgrant: Down to one failure in my no-more branch [00:11] Excellent. [00:21] How did this fail before? I don't get it [00:25] Hm? [00:26] wgrant: http://pastebin.ubuntu.com/674932/ [00:27] StevenK: It possibly was relying on the branch not existing on disk. [00:27] StevenK: Try checking the output on devel. [00:30] ERROR Job execution raised an exception. [00:30] -> http://localhost:33770/93/bLqgyciAI0wS9vOJ96yNSjGCqMV.txt (Not a branch: "lp-internal:///~person-name-870614/product-name-870620/branch-870616".) [00:32] Whereas that doesn't happen for my change [00:34] So I guess it wants a job for a branch that doesn't exist on disk? [00:35] Right. Use a RevisionAddedJob or so. [00:35] Since RevisionMailJob doesn't touch the disk any more. [00:44] wgrant: Fixed. [00:44] StevenK: Using RevisionAddedJob? [00:44] wgrant: Yes [00:45] Great. [00:45] Now to craft evil queries [00:52] Should be pretty trivial. [00:52] And there's only like 11000. [00:52] Possibly less, if some of them have preview diffs. [00:53] wgrant: what are you doing ? [00:53] lifeless: StevenK is abolishing review diffs. [00:54] SELECT count(distinct(review_diff)) from branchmergeproposal where merge_diff is null; => 5213 [00:55] did you talk to aaron about difftasticd ? [00:55] s/d // [00:56] Not about difftastic, that had utterly slipped my mind [00:56] well, review diffs and difftastic are pretty linked [00:56] They are? [00:56] I'd hate to see that deleted rather than fixed. [00:56] perhaps I misunderstand [00:56] I thought difftastic was used for incremental diffs? [00:57] lifeless: Those are incremental diffs. [00:57] I probably do misunderstand :) [00:57] Review diffs are what came before preview diffs. [00:57] Back in the days of lp:mad. [00:57] Named in DB as *static*diff [00:57] <= 2010/01 [00:57] ah [00:57] cool cool thanks! [00:58] wgrant: So, we have a query to clear review_diff where preview_diff is not null [00:59] launchpad_dogfood=> UPDATE branchmergeproposal set review_diff = null where review_diff is not null and merge_diff is not null; [00:59] UPDATE 6860 [00:59] launchpad_dogfood=> SELECT merge_diff IS NOT NULL, review_diff IS NOT NULL, COUNT(*) FROM branchmergeproposal GROUP BY merge_diff IS NOT NULL, review_diff IS NOT NULL; ?column? | ?column? | count [00:59] ----------+----------+------- f | f | 3061 f | t | 5251 t | f | 36774 t | t | 6860 [00:59] Fail. [00:59] Haha [00:59] But there are 6860 with both, 5251 with just a staticdiff. [01:00] Right, so the update for 6860 rows is fine [01:02] Now a query to create previewdiffs [01:02] Are preview diffs always used in preference to review diffs? [01:02] From my reading of the code, yes [01:06] There's only 746 MPs with a staticdiff, no previewdiffs and aren't merged [01:07] And 4505 where date_merged is not null [01:07] Even merged ones would be nice. [01:07] I'm just a little lost how to loop on branchmergeproposal, creating rows in previewdiff and updating [01:08] Project db-devel build #826: STILL FAILING in 5 hr 29 min: https://lpci.wedontsleep.org/job/db-devel/826/ [01:18] wgrant: StevenK: dumb question of the day - what's the difference between a review diff and a preview diff? [01:18] wgrant: reviewdiffs are static and ancient [01:18] RARGH [01:18] wallyworld_: ^ [01:19] StevenK: ah, so review diffs do not get updated if someone pushes a change to the branch after it is put up for review? [01:20] wallyworld_: They have not been generated in the database for over 18 months, it's probably not worth learning about, TBH [01:20] ok [01:20] delete away.... :-) [01:22] Bah. [01:23] No index on previewdiff(source_revision_id, target_revision_id) [01:24] wgrant: Do we need one? [01:26] StevenK: Probably. We should try on staging without it. [01:26] But DF definitely does. [01:26] http://paste.ubuntu.com/674955/ [01:26] Or you end up with thousands of seq scans of the table. [01:27] Right [01:27] wgrant: Is that in a transaction? [01:28] StevenK: Yes. [01:28] Right [01:28] Should also set review_diff = NULL, I guess. [01:28] But it's fast enough to do all this directly. [01:28] I have a query for that already [01:28] If we do that at the end, we can use it to clean up for us [01:29] We might as well do it in the one UPDATE. [01:29] UPDATE branchmergeproposal SET merge_diff = (SELECT id FROM previewdiff WHERE from_revision_id = source_revision_id AND to_revision_id = target_revision_id) FROM staticdiff WHERE staticdiff.id = review_diff AND merge_diff IS NULL; [01:29] UPDATE branchmergeproposal set review_diff = null where review_diff is not null and merge_diff is not null; [01:29] Are the two of them [01:30] I guess we need to unset all of them, whereas my UPDATE only needed to consider those without a preview_diff. [01:30] So indeed. [01:41] OOPS-2062F5 [01:41] hmm, no bot? I'll have to do this by hand [01:42] those numbers seem small [01:42] not really [01:42] mwhudson_: Which? [01:42] its 0142 [01:42] in the oops [01:42] ah right [01:42] 2 hours in, 60 appservers. [01:42] Date: Aug. 24, 2011, 2:01 a.m. [01:43] which makes it 0101 [01:43] if you use a real tz :P [01:43] :( [01:43] Dropping a foreign key requires a lock on both tables. [01:43] oh or its a couple days back :) [01:43] How stupid. [01:43] either way [01:43] yeah [01:43] wgrant: thats because fk's are triggers. [01:43] I was just making sure the oops had synced :-) [01:43] lifeless: Ah. [01:44] wgrant: (apparently :P) - they get presented differently of course, but are built on that mechanism. [01:44] it [01:44] it's doing the count(*) twice? [01:45] wallyworld_: uhm [01:45] lifeless: ? [01:45] wallyworld_: are you *trying* to make the public -stacked-on-private branch obfuscated? [01:46] no [01:46] why do you need a metaclass then ? [01:46] and __setattr__ [01:47] so that code in storm queries can still go "Branch.private == xxx" [01:47] (the metaclass does this bit) [01:47] yes, but this means that aBranch.private and Branch.private are now totally different htings. [01:47] this is horribly confusing [01:47] and so that exisitng code which creates a branch via Branch(name=xxx, private=True) still works [01:47] (the setattr does this) [01:48] wgrant: Ah, I forgot we need to drop the FK as well [01:48] And then after my evil branch lands we can drop the column and table [01:48] lifeless: the other option then is to s/Branch.private/Branch.explicitly_private [01:48] wallyworld_: please [01:48] wallyworld_: thats what we've done in the past: KISS [01:48] ok. it wasn't confusing to me :-) [01:49] i was trying to do it behind the scenes so to speak [01:49] wallyworld_: its not confusing to me either, cause I've seen the diff. But imagine someone looking at the rest of LP [01:49] I can't find a bug for this timeout, would someone take a look, or should I just file one? [01:49] who comes to this code, and sees no difference visually. [01:49] wallyworld_: unless you rename the column I suggest actually not renaming [01:49] wallyworld_: instead add a property 'effectively_private' or something, which would honour self.private + self.stacking etc. [01:50] wallyworld_: and reference that from security adapters, views etc. [01:50] wallyworld_: this will keep the class .private and the schema .private in sync with each other. [01:50] lifeless: that's a big change [01:50] why not just rename the private property to explicitly private [01:51] and use dbname="private" in the property [01:51] this will have the smallest footprint in terms of change to the code [01:51] and still be quite clear i think [01:52] I think that's better, so that you don't have to remember that the obvious "private" attribute isn't likely what you usually want [01:52] wallyworld_: I'm thinking about the mapping from object to schema [01:52] james_w: file a bug :) [01:52] james_w: agreed. [01:52] wallyworld_: but sure. [01:52] wallyworld_: my main concern is that what folk think they are reading is what they are reading. [01:53] wallyworld_: they should be able to guess at the behaviour of the code, with high accuracy rates, without having read the entire implementation. [01:53] wallyworld_: if private is a property and *not* the db column, then 'foo.private = True' will not do what they expect. [01:54] however, I see that that is not meant to happen here anyway [01:54] folk are meant to call setPrivate [01:54] that bit will do what they expect. ie make the branch private. but if they say foo.private = False it may not [01:54] so +1 [01:55] lifeless: ok. we can iterate later and change the column if needed [01:55] wallyworld_: well, it will only do that with either a property or a setattr [01:55] https://bugs.launchpad.net/launchpad/+bug/834293 [01:55] <_mup_> Bug #834293: Product:+code-index times out < https://launchpad.net/bugs/834293 > [01:55] wallyworld_: ok; so for clarity - you're doing:13:50 < wallyworld_> why not just rename the private property to explicitly private [01:55] 13:50 < wallyworld_> and use dbname="private" in the property [01:55] wallyworld_: dropping the metaclass and setattr [01:55] lifeless: yes [01:56] wallyworld_: to be clear, I don't object to metaclasses and setattr per se; just that they should really be enhancing things, not obfuscating :) [01:56] wallyworld_: thanks! [01:56] james_w: please don't use lp-oops urls [01:56] james_w: just the oops id [01:56] lifeless: np, input is appreciated. i can see how it's clear to me because i'm close to it but not to others who may be looking in from the outside [01:57] lifeless: will you have time to do that loggerhead review before you go and spend your evening changing shitty nappies? [01:57] lifeless, why? [01:58] james_w: a) we linkify anyway so theres no different in click-through, b) its easier to read and copy, c) the garbage collection code ignores url style oops references so they will get deleted. [01:59] ah, I didn't realise there was any integration [01:59] the latter seems like a bug though :-) [02:02] james_w: it is [02:02] james_w: but anyhow, its intended for users that have no access to the reporting UI [02:02] your having access to it is the exception :) [02:03] yeah [02:07] frell [02:07] another regression [02:07] lifeless: Where? [02:09] bug 834266 [02:09] <_mup_> Bug #834266: "831884 is not a valid bug number or nickname" < https://launchpad.net/bugs/834266 > [02:10] lifeless: I think that's the second time you've declared this bug to be a regression. [02:10] lifeless: The case that's handled in marking a master as a dupe. [02:10] s/in/is/ [02:11] Trying to find the dupe.. [02:11] There. [02:35] wgrant: thanks :) [02:36] grar. [02:36] Why do people insist on implementing non-auditable copying. [02:36] Seriously. [02:36] Stupid. [02:55] Hey, good morning everyone! I think I'll implement some non-auditable copying today. [02:59] jtv: No, your squad already did that last week. [02:59] You're too late :( [02:59] Oh, but there's always more to do. [02:59] I insist on implementing non-auditable copying. [03:01] Good, good. [03:01] What could go wrong. [03:06] hi wallyworld___, your collection of underscores is coming along nicely I see. [03:06] Any reviews for me to approve? [03:13] Project devel build #997: STILL FAILING in 6 hr 0 min: https://lpci.wedontsleep.org/job/devel/997/ [03:19] lifeless: O hai -- wgrant and I would like to remove all references to staticdiff -- the queries take approximately 12 seconds on DF and it will be a once-off. Or are you going to make me write a garbo job? [03:20] 12s include an index creation that can be done whenever. [03:20] 4-5s without. [03:24] Over a hundred broken tests in buildbot. That's impressive. [03:25] Yeah. [03:25] A revision was landed that depended on a revision that was reverted a few hours earlier. [03:25] the index creation is nonblocking [03:25] if its done CONCURRENTLY [03:25] Right. [03:26] so, that seems fine to me. [03:26] Hence can be done whenever, including in the patch, if required. [03:26] 4-5s is too long to do as one transaction though if its hitting 6K rows [03:26] We can split it up [03:26] yup [03:26] 2s per transaction or so [03:26] that works for me [03:26] lifeless: 6k merged proposals from before 2010? :) [03:27] Only 5k [03:27] wgrant: FK references. [03:27] wgrant: never underestimate the MVCC lock monster [03:51] lifeless: looks like the django ticket for passing the exception info up has progressed to "Design decision needed". Hopefully that isn't a black hole [03:52] jamesh: It normally is with Django... [03:53] jamesh: win. [03:53] course, we could patch it in Ubuntu :) [03:54] it seems one of the issues is that not all exceptions might make it to handle_uncaught_exception(), so capturing errors there might not be perfect [03:55] I've added a comment that all the errors in my view code generally pass through that code path though [03:55] cool [03:55] if they swallow exceptions and don't reach that code path, that seem like a separate bug :) [03:55] as opposed to a reason not to do this [04:54] wgrant: you asked why I didn't re-implement [BS]PPH.setDeleted in terms of PublishingSet.setMultipleDeleted. The answer is that the latter bypasses the ORM, which is more likely to be harmful in code that requests deletion of individual objects. [04:54] jtv: True. Although it doesn't have to bypass the ORM. [04:54] Do you know about ResultSet.set()? [04:55] It gets far too little use. [04:55] No, I don't! I knew there was something like that for delete, but it gave us endless trouble anyway. [04:55] (There were all sorts of queries it didn't support) [04:55] yeah, but for this sort of thing it's probably fine and a lot less ugly. [04:56] I'll file a bug to fix that. It'd save some test commits. [04:56] Right. [04:59] jtv: Do you like lp.soyuz.model.publishing's wonderful test coverage? [04:59] Truly exemplary stuff. [05:00] How sarcastic are you being? [05:00] wgrant: I'm picking up a hint of sarcasm there, but I'm told that I wouldn't understand because I'm not American. [05:02] Just a bit. [05:02] Thanks. It helps to be clear about these things. [05:02] wgrant: That's like calling a fish 'just a bit wet' [05:03] * wgrant deletes more of Distribution. [05:03] A-khah. This vhat British capitalist call, understatement—da? [05:04] wgrant: Excellent. [05:06] * StevenK fights with Jenkins [05:12] jtv: you managed to create two bugs [05:12] That explains why I got two confirmation boxes. [05:12] jtv: I wonder if we have a bug in our request retry logic or something [05:13] Or a very late failure triggering a retry after db commit maybe? [05:14] I don't suppose the browser would retry the POST unless it got an unambiguous application-level failure response? [05:14] Sure you didn't click submit twice? [05:14] Very. [05:15] POSTs are not permitted to be retried automatically. [05:15] At the browser-level. [05:15] This is chromium though. Full of optimizing cleverness. [05:15] Heh [05:15] BTW I see now that I expressed myself badly: [05:16] I know what you meant [05:16] I meant, I don't suppose the browser would retry the POST just because it did not get an unambiguous application-level failure response. [05:16] and yes I think post-commit retry is the most plausible epxlanation [05:28] jtv: How did you file those bugs? [05:29] wgrant: could you be more specific? [05:29] I only see one bug filing in the appserver logs, and that was 90 minutes ago. [05:29] Are you using edge or something? [05:29] Nope. [05:29] wgrant: appserver bug [05:29] wgrant: we retry requests in the appservers. [05:30] wgrant: on POST, on conflicts. [05:30] lifeless: The bug in question was filed only 30 minutes ago. [05:30] I do not see even one POST for it. [05:30] wgrant: that is how it was posted, no ? [05:30] oh, I see 90 vs 30 [05:30] wgrant: you've got all the appservers ? :> [05:31] actually. EOD. EOW. SOL. [05:31] lifeless: Good plan. See you eventually, I suppose. Good luck! [05:31] lifeless: enjoy your weekend. [05:31] jtv: Weekend? [05:32] wgrant: yes, it's that bit where the office goes nice and quiet so you can work in peace. === almaisan-away is now known as al-maisan [05:32] When that happens, the rest of us are having something called a week-end. [05:32] wgrant: I think jtv is being sensible about disclosure of private data [05:32] Bah. [05:32] which is nice [05:32] I wonder if puppet broke stuff. [05:32] Not all the logs are here. [05:33] Probably means we're missing OOPSes, too. [05:33] that vould be a vorry [05:33] But not surprising. [05:33] hangon, I was going. [05:33] See you. [05:34] wgrant: I'm pushing up my gpfixtures WIP for LP [05:35] lifeless: I may have a poke at it and get it finished. [05:36] lp:~lifeless/launchpad/usegpgfixtures and of course lp:python-gpgfixtures [05:37] it needs a ServerFixture to invoke the process etc, which would live in python-gpgfixtures for now. Its not a perfect demo of the soa test fake layout. [05:37] perhaps it should be, but one step at a time [05:37] Yeah. [05:37] Anyway, shoo. [05:38] both branches pushed. [05:38] enjoy. [05:38] Thanks. [05:39] heh, the keyserver in fixtures is shorter than the one in LP; including the main() stuff and the json API. [05:39] \o/ [05:39] Nice. [05:40] not a totally fair comparison, but shrug [05:51] Right, Jenkins is in money-suck mode [06:12] jtv, hi, can you perhaps give me another short review, mostly for a branch you've already reviewed: https://code.launchpad.net/~danilo/launchpad/bug-826692-take2/+merge/72996 (it was broken for private branches, and the fix is easy, and I add a test for it) [06:12] jtv, you can perhaps just look at the incremental diff in http://paste.ubuntu.com/675046/ [06:12] danilos: okay okay, you've sold me. Don't try to buy it back. :) [06:13] heh, thanks === al-maisan is now known as almaisan-away [06:18] danilos: Given its history, you might want to coerce a LOSA to merge that on staging so you can try it out for real. [06:19] wgrant, sure, though when you do anticipate a problem (or when there are tests for it), it makes a big difference :) [06:20] danilos: Bah, who needs tests. [06:21] not us the real men! our test suite are our users [06:21] wgrant, what was the commercial team name again? I need to find who can make me a few private branches on staging [06:22] danilos: “Merge proposals against private branches are visible to *the* branch owner.” [06:22] danilos: ~commercial-admins and LOSAs. [06:22] danilos: I have access to U1 branches, if that helps. [06:22] wgrant, are they on (qa)staging? I have access to landscape branches, but they ain't there [06:23] danilos: The content doesn't matter, does it? [06:23] jtv, thank you :) [06:23] danilos: is this the branch that caused (some of) those 93 failures and 17 errors we had in devel earlier? [06:23] The DB stuff is there, and that's all that matters. [06:23] wgrant, nope, I just need merge proposals against them [06:23] jtv, nope, the reversion of this branch and gary's revision using some of the stuff in this branch is what caused the failures [06:24] danilos: gary reverted that before you emailed me, btw. [06:24] wgrant, it is? oh right, logging in might help see them [06:24] danilos: oh, I heard about those but didn't think it was the same failure. There are far too many lately. :/ [06:24] jtv, this is just a regular distributed development fallacy, I don't think there's anything we could do about it except make the test suite run shorter thus reducing the chances of something like this happening [06:25] danilos: I agree that that's about all we can do. [06:25] wgrant, yeah, I've seen that as well, sorry if I bothered you :) [06:25] danilos: for a moment I thought you were saying I was completely wrong, but now I suspect "fallacy" isn't really the word you meant. :-) [06:25] If we had a one hour test suite again (which is practical with parallelisation), the occurrence and impact of this sort of thing would be far, far lower. [06:26] Yes. [06:26] Although the past few days, Q/A has played a large role as well. [06:27] jtv: That's made worse by the fact that QA can rarely happen until 10 hours after the branch is submitted. [06:27] Which means a lot more breakage can slip into devel before it's noticed, and it's less likely to be noticed because the wrong people are doing it. [06:28] Absolutely. Getting a branch from submission to Q/A readiness is still an overnight process. [06:28] Which reminds me of another suspect in our current problems: staging updates. [06:28] What staging updates? [06:29] It hasn't updated in a while :) [06:29] :) [06:29] Oh well that's alright then. :) [06:29] The ticket is only 85.. we should probably get it bumped. [06:31] danilos: I rejected the MP based on the missing "the" in the comment. Please try again Monday. [06:31] jtv, fair enough, thank you very much, I'll be back on Monday [06:32] (I'll go spend weekend fixing all the missing articles in the LP tree, however you take that ;) [06:32] np [06:32] danilos: *the* weekend. You did that on purpose, didn't you? [06:32] spend the weekend? [06:32] Yup. :) [06:33] heh, yeah, I can usually find them missing only on re-reading the sentence, and that never happens with comments for tests :P [06:35] Project devel build #998: STILL FAILING in 16 min: https://lpci.wedontsleep.org/job/devel/998/ [06:36] Uh [06:37] bzr: ERROR: http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/.bzr/repository/packs/9e62132dbdc7acb42f1156c7a4128cf2.pack is redirected to https://launchpad.net [06:37] RARGH [06:38] Yay [06:38] wgrant: - 1. [[#william_grant|William Grant ]] ''(133 top-level landings)'' [06:38] + 1. [[#william_grant|William Grant ]] ''(134 top-level landings)'' [06:38] wgrant: lies [06:38] lifeless: I merged a rev from a branch from December :) [06:38] Haha [06:38] I thought I cherrypicked it. [06:38] But apparently not well enough. [06:38] Ah, it was the first rev in the branch. [06:43] lifeless: Are you here? [06:43] lifeless: If so, a prio bump on https://rt.admin.canonical.com/Ticket/Display.html?id=47551 would be nice, since staging hasn't restored for two weeks now and we'll likely be fastdowntime-capable next week... [06:45] * 31NAAXP81 hates doc tests. too hard to debug :-( === 31NAAXP81 is now known as wallyworld_ [06:45] Haha [06:45] shutup [06:45] Underscores, random characters... what next? [06:45] Nice nickname [06:45] i'm already way too grumpy === StevenK is now known as W4|_|_YW0R|_D [06:45] @%^@#%^!@^ [06:45] Project devel build #999: STILL FAILING in 3.5 sec: https://lpci.wedontsleep.org/job/devel/999/ === W4|_|_YW0R|_D is now known as StevenK [06:46] smart arse [06:46] Oh damn it! [06:46] JENKINS! [06:46] 3.5 is pretty short [06:46] The slave was already corrupted [06:52] wallyworld_: Hi! ;) [06:53] henninge: hi there [06:53] i assume you saw my comments :-) [06:54] wallyworld_: yes, thank you ;) [06:54] np. i hope they made sense [06:54] wallyworld_: I replied rather lengthy [06:54] very much so [06:54] ah right. i'll look at my email [07:00] henninge: right, i see the disabling of the form submit button now. and it will be re-enabled if a request build succeeds or partially succeeds? [07:01] wallyworld_: not currently, no. [07:01] i think the original request build form did this? [07:01] wallyworld_: AFAIUI the user is supposed to close the dialog and re-open. [07:01] no [07:01] it did not [07:02] or, let me check that ... ;) [07:02] ok. i thought it was either re-enabled or else the user could click an as yet unbuilt distro series and it would be enabled [07:02] without requiring the form to be closed and reopened [07:03] I just checked, the button is only enabled in the connect function. [07:03]