[00:26] Bug #1493598 opened: dblogpruner uses *state.State

[00:29] * thumper is filing bugs for the workers [00:44] Bug # opened: 1493600, 1493601, 1493602, 1493604 [00:47] Bug # changed: 1493600, 1493601, 1493602, 1493604 [00:53] Bug # opened: 1493600, 1493601, 1493602, 1493604 [00:56] Bug #1493606 opened: envworkermanager uses *state.State

[00:59] Bug #1493606 changed: envworkermanager uses *state.State

[01:06] what's the deal with the n.mu mutex protecting assignment and reading tag_ in ./apiserver/apiserver.go ? are assignments not atomic operations in go? [01:08] Bug #1493606 opened: envworkermanager uses *state.State

=== akhavr1 is now known as akhavr === natefinch-afk is now known as natefinch [01:31] sarnold: assignments aren't guaranteed to be atomic, no. There's sync/atomic if you need some specific atomic assignments, or for most things, there's locks. [01:34] natefinch: can you aim me towards some documentation? that seems like it might be fairly subtle, and I'd like to know more [01:35] sarnold: https://golang.org/ref/mem [01:35] natefinch: thanks! [01:36] sarnold: welcome. The tldr is right there at the top: If you must read the rest of this document to understand the behavior of your program, you are being too clever. Don't be clever. [01:36] natefinch: indeed :) it's just that tag() and login() both look like they ought to work fine without the mutex, and the fact that it has one is surprising... [01:39] sarnold: the string assignment isn't atomic. If you call login from one goroutine, which modifies the tag, and call tag() from another goroutine, it's possible that login could have only half-written the pointer inside the string, and tag() would return garbage [01:44] sarnold: also, what's more likely is that the pointer gets updated but the length doesn't get updated... so you could read past the end of the string [02:00] thumper: wallyworld did someone justa dd a worker/uniter/relations package ? [02:02] natefinch: writes to word sized quantities really should be atomic in the sense that another thread will either see all the write or none of it [02:02] *of [02:03] the thing about the pointer vs length write is 100% valid [02:03] mwhudson: yeah, I was thinking that - it's not guaranteed, but it really should be [02:03] natefinch: ot [02:03] natefinch: mwhudson word aligned writes are atomic [02:03] another thread will not see a torn write [02:03] to be fair i've only read the arm64 manual closely on this [02:03] but it is also true that another thread may not see the write at all [02:04] davecheney: yes, that was me i think when i merged in the uniter v2 work [02:11] http://paste.ubuntu.com/12318103/ [02:11] really unstable [02:11] and there are data races [02:11] sorry, i meant to say [02:12] the tests are unstable and have races [02:13] worst test failure output ever? http://pastebin.ubuntu.com/12318026/ [02:14] s/worst/longest [02:14] davecheney: I guess it's not actually incorrect or misleading... just useless and long [02:14] more impressive with the line returns in my terminal [02:15] * natefinch wonders how hard it would be to embed some ascii art as a test failure output without making it too obvious in the code [02:16] natefinch: i am still scarred by the fact that the mongodb package build logs have/had a line that is something like 120k long [02:16] mwhudson: wow, winning. [02:16] totally [02:16] scons for the win, indeed [02:17] I built mongodb from source... once. It was horrible. [02:17] https://bugs.launchpad.net/juju-core/+bug/1493623 [02:17] Bug #1493623: worker/uniter/relation: relationsSuite.TestCommitHook tests fail [02:17] Bug #1493623 opened: worker/uniter/relation: relationsSuite.TestCommitHook tests fail [02:24] thumper: you able to chat? [02:27] menn0: sure [02:29] thumper: standup? [02:29] ah... yeah. [02:35] gah, I hate it when rebase gives me a merge conflict that I know has nothing to do with the changes I made [02:35] don't use rebase :-) [02:35] rebase sucks [02:36] wallyworld: I don't really have a choice. I pushed some code based off of what was evidently an old copy of master... it's either rebase or have a merge commit in my branch [02:36] wallyworld: or create a new branch and cherry pick my changes. ... I mean, if anyone has a fix that is not rebase, I'm all ears. [02:36] merge commit isn't so bad is it? [02:36] it's just an extra commit [02:37] wallyworld: sometimes it makes your branch look like it has changed everything in that merge commit. [02:37] gad, i wish we still used bzr [02:37] wallyworld: sometimes it's fine and sometimes it screws me. I never really now which it'll end up being [02:37] that crap just doen't happen [02:38] I hear betamax was pretty good too, but what can you do? [02:44] * natefinch goes with door #2 - new branch and cherry pick [02:49] dunno why git rebase master is different than making a new branch off master and cherry-picking my changes... but the latter works 100% of the time, whereas rebase is batting about 50% for me. [02:52] ahh, I think the difference is git rebase vs. git pull --rebase [03:50] thumper: why mark that bug as a blocker? has CI failed? they need fixing sure, but not a blocker [03:50] wallyworld: it's a blocker if people can't get tests passing locally, surely? [03:51] this is the first i've hear dof them not passing for anyone, they passed for all of us on the sprint [03:51] and the bot [03:51] wallyworld: dave has them failing every time for him [03:51] hmmm, ok [03:51] a key question then would be "what's different?" [03:51] indeed [03:52] ppc maybe? [03:52] wallyworld: also, there are races in the code that landed [03:52] wallyworld: did you check with -race? [03:52] no :-( the races need fixing [03:52] I have a few tests on master failing 100% [03:52] but just races is not a blocker, but if there's a genuine failure there... [03:53] natefinch: uniter/relations? [03:54] wallyworld: no, weird random crap... /cmd/plugins/local and /mongo ... both seems to be some difference in quoting [03:54] ok, not related then i don't think [03:54] ccd [03:55] damn... [03:55] I can't get the tests to run at all [03:55] ahh, I'm dumb, didn't update godeps [03:56] davecheney: any ideas ? http://paste.ubuntu.com/12318539/ [03:56] thumper: i don't see the need to block people landing stuff for the sake of fixing tests in one package [03:56] especilly if CI is not broken [03:56] wallyworld: the alternative that was agreed on is reverting the revision [03:57] well i don't agree it's a blocker [03:57] thumper: that looks like a difference in stdlib [03:57] bot passes, CI is not failed [03:57] wallyworld: if you want to make that call, change it, bit know that I'm grumpy [03:57] tests only fail in one instance for one person so far [03:58] you're always grumpy :-) [03:58] I can't get them to run either [03:58] oh, ok [03:58] damn [03:58] wtf did the bot pass then [03:58] * thumper shrugs [03:58] and everyine else testing that branch [03:58] I have the lxd ppa [03:58] sigh [03:58] which brings in a later golang [03:59] ah [03:59] that could wee be it [03:59] i think we're still on 1.4 [03:59] we're on 1.2.x last I heard [03:59] the ppa builder is [03:59] working on upgrading to 1.5 [03:59] most devs are on 1.4 [03:59] 1.4.2 is my local version [04:00] the official version we must build with is 1.2 [04:00] I wonder why my stdlib changed? [04:00] natefinch: "we" = ppa packager yes [04:01] for now [04:01] yes, sorry.. I meant, juju is officially built with 1.2 [04:01] we as devs can build with whatever we want, but may introduce problems if we rely on stuff that isn't in older versions of go [04:02] yep [04:03] ....like references to stuff that isn't in older versions of the stdlib [04:05] however, worker/uniter/relation builds fine with go 1.2.2 on my machine [04:05] and evidently the bot [04:06] thumper: what is really bizarre is that you're getting undefined symbols inside the standard library itself [04:06] thumper: I'd say your standard library is hosed somehow [04:10] thumper: can you update loggo to import gopkg.in/check.v1 rather than launchpad.net/gocheck pls? [04:13] thumper: i see a data race in one place. maybe fixing that will solve your issue [04:13] mwhudson: sure [04:15] I may well reboot to see if it fixes the issues :) [04:15] works for windows right? [04:19] thumper: do you have GOROOT set? [04:19] doh [04:20] hmm.. rebooting didn't fix it [04:20] perhaps I need to reinstall golang? [04:21] thumper: probably a good idea [04:22] thumper: build from source, it's better [04:22] no, I'm not that kinda person [04:22] it's really easy, but ok :) [04:23] hmm... [04:23] reinstall brought in 1.2.1 [04:23] which didn't fix the problem [04:23] natefinch: it is more a philisophical reason [04:23] not because I can't, but I shouldn't have to [04:24] thumper: do you have GOROOT set? [04:24] oops need to run [04:24] * thumper looks [04:25] it just makes it easier to switch between versions, for the most part... plus then you're not tied to whatever ubuntu ships [04:25] no just GOPATH [04:25] thumper: that's good, you generally shouldn't set goroot [04:25] natefinch: I understand, and I was working more with Go and cared, I would [04:25] but Juju needs to use the version in the distro [04:26] so best to use the version in the distro to compile locally [04:26] thumper: certainly [04:26] knowing that some are using later versions to test for incompatibility [04:26] I bet I have part of newer versions installed [04:26] ... [04:26] yep [04:27] if you installed from source you could do git status and see what was out of place ;) [04:27] blow it away and reinstall [04:28] it would be good to know how it got that way, though... if there's some package stomping on the go install, that's a really bad thing [04:29] thumper: works for me, but that doesn't mean it will work for you http://reviews.vapour.ws/r/2613/ [04:29] ok, way past EOD for me (literally and figuratively) g'night all [04:35] yep... [04:35] * thumper now has golang 1.5 [04:35] I had disabled the lxd ppa before [04:35] and that left me in a weird state [04:35] reenabled [04:36] upgraded, and now seems to be compiling at least [04:37] thumper: that fix eliminates the race (well test --race is happy) so maybe it will solve your problem with go 1.5 [04:37] i still can't reproduce [04:38] my build problem was a local issue [04:38] I've fixed my build problem [04:38] running all the tests now [04:39] thumper: could you take a look at the pr and i'll land if you're happy [04:47] wallyworld: one big problem, and one stylistic [04:47] ok [04:48] I think I must be going crazy [04:50] mwhudson: where are you looking? github.com/juju/loggo does use gocheck from gopkg.in [04:54] wallyworld: I get worker/uniter/relation tests failing every time too [04:54] wallyworld: when you have tweaked the load, I'll test your fix [04:54] thumper: pushed [04:55] wallyworld: is the int32 cast really needed? does golang complain? [04:55] thumper: sadly yes [04:56] i didn't have it at first [04:56] well, you could change the expected arg to be int32 instead of int [04:56] that would get the compiler doing the right thing [04:56] thumper: hang on, i removed it and it worked, ffs [04:57] ah no [04:57] it didn't, tests fail [04:57] gc.Equals complains [04:57] if one arg is int and oter is int32 [04:58] changing arg worked [04:59] still fails for me [04:59] wot [04:59] works for me with --race and without [04:59] why is the test running asynchronously? [04:59] it is checking in parallel [05:00] one is happening before the other [05:00] you can't guarantee synchronisation across go routines [05:00] unless you synchronise them [05:00] this is why it is failing [05:00] what's running aync? [05:01] http://paste.ubuntu.com/12318735/ [05:01] request 6 is being checked [05:01] and while it is saying it is failing [05:01] request 7 is being checked [05:01] and fails [05:01] both are expecting the other value [05:01] but the test just sts up a mich api caller and makes some api calls to it in order [05:02] there's no go routines in the tests [05:02] i'll dig into it [05:02] there are go routines somewhere [05:02] * thumper looks too [05:02] wallyworld: btw, OOPS: 15 passed, 7 FAILED [05:02] for the relations package [05:03] and yet passes for me and other and bot [05:03] they aren't using golang 1.5 [05:03] so it is passing by accident [05:07] wallyworld: FWIW, `go test -check.f relationsSuite.TestHookRelationDeparted` I've had pass, and fail in two different ways [05:07] the tests are all synchronous so it must be in code somewhere [05:08] thumper: are the failures always Stop/Next ordering? [05:08] nope [05:09] how did you install go 1.5 i sthere appa? [05:09] http://paste.ubuntu.com/12318767/ [05:09] i looked for a ppa a while back and didn;t see one [05:09] it's in wily now [05:09] add-apt-repository ppa:ubuntu-lxc/lxd-stable [05:09] apt-get update [05:09] apt-get dist-upgrade [05:09] get it if you want lxd :) [05:10] wallyworld: seems to be always ordering related [05:10] i could skip the tests on go 1.5 [05:10] but not always Stop/Next [05:10] and then fox [05:10] fix [05:10] nah [05:11] to unblock [05:11] that's terrible [05:11] i do want to remove the blocker tag [05:11] i'll get 1.5 and reproduce [05:12] thumper: if i get lxd, will it stuff up anything else in juju? [05:12] why not just get the deb out of the ppa by downloading directly and install? [05:13] sure, but lxd looks interesting :-) === urulama_ is now known as urulama [05:25] wallyworld: if there aren't async calls, why would I get request 7 written to the test log before request 6? [05:26] the only way that would happen is if the active goroutine switched after incrementing the value but before writing to the test log [05:26] there must be in the underlying code, but no tthe tests which is what i thought you were referring to [05:26] I've not been able to find it [05:26] but I need to head out shortly [05:27] dinner date, anniversary [05:27] i just installed go 1.5 and now go is broken, so i'll look into that [05:27] sorry I couldn't be more help [05:27] waigani: here is a simple one https://github.com/juju/juju/pull/3236 [05:32] waigani: actaully scratch that [05:33] no [05:33] actually, i think this is ok [06:29] waigani: http://reviews.vapour.ws/r/2615/ [06:29] slightly larger change [07:30] fwereade: running a bit late [08:30] dimitern: ping [08:31] TheMue, pong [08:31] dimitern: inside the space commands I wanted to differentiate between "not supported" and other API errors [08:32] dimitern: today the API client simply passes the error through [08:32] TheMue, yes [08:32] dimitern: so also the params.NotSupported [08:32] TheMue, you mean in the feature tests or? [08:32] dimitern: a errors.IsNotSupported so doesn't match [08:32] dimitern: the feature test is working, it only compares output [08:33] TheMue, the equivalent of errors.IsNotSupported as a satisfier is params.IsCodeNotSupported (when testing an api error result) [08:33] dimitern: but inside e.g. the list subcommand, here an API error could be this not support, but also a connection error or else [08:34] TheMue, yes, that's true, but for the end user shouldn't matter - it's just an error we should display [08:34] dimitern: yep, but I dislike the usage of params outside the API. so my proposal: check this inside the API client and in this case convert the error to a regular IsNotSupported [08:35] dimitern: surely using params.IsNot... makes it more simple for me ;) [08:35] TheMue, sorry, I'm not sure I follow you [08:36] TheMue, let's discuss this at standup? [08:36] dimitern: ok, we can do, it's better then [08:37] dimitern: simply to describe my concerns, params to me is only a package to transfer data between api and apiserver and it shouldn't used outside (clean interfaces) [08:38] TheMue, that's not entirely correct though - params types are used as well for anything that's returned from an api call (at the client-side), e.g. params.Life === ashipika1 is now known as ashipika [08:39] dimitern: I know, but exactly this is my concern :D it's not ... clean [08:40] TheMue, the client-side api method (e.g. returning one result/error) that's calling the apiserver facade bulk method can return the result and an error, which can still effectively be a params.Error [08:42] dimitern: yes, and so the user has to know, if an "errors" error or a "params" error is returned. my wish is that the API client always returns "errors" or own errors (phew, so many errors *lol*) [08:43] TheMue, params.Error is just an error - you shouldn't expect the client-side api to hide its origin (a wrapped errors.NotSupported for examle) [08:43] example* [08:44] TheMue, so an error you get from the api is not the same as calling the respective state package method directly (which returns e.g. errors.NotSupported) [08:48] dimitern: yes, exactly, that's why my errors.IsNotSupported failed and I wondered. a %#v then showed that it's a params and I looked into the API client [08:49] TheMue, right :) - well, that's as expected; I don't consider it "unclean" :) - i.e. we shouldn't act like the API layer's not even there [08:50] dimitern: yes, I think that's the way. but in my "optimal" world it would be kind of transparent *dream* [08:51] dimitern: that's IMHO the task of the API client package, otherwise we could call the server directly like the UI does [09:02] fwereade, jam, hey guys - are you joining standup? [09:02] dimitern: I'm there [09:50] dimitern: to get this one in before my vacation I'll add a card and TODOs and continue with params.IsNot... [09:50] dimitern: will assign this card to me [09:50] TheMue, TODOs about what? [09:51] dimitern: as we discussed a help to convert params errors into their according counterparts (missed the word, unpacking?) [09:52] TheMue, unwrapping :) [09:53] dimitern: yeah, exactly, that's what I meant, thanks [09:53] TheMue, well, it's a couple of lines of code for SupportSpaces, but ok I'm ok with a TODO+follow-up [09:54] dimitern: for this one yes, it's small. thought about a more global approach later, that can be used elsewhere too [09:55] dimitern: but I could start with the IsNotSupported as a first and only one *g* [09:55] TheMue, I'm -1 on a global approach [09:55] TheMue, as discussed - it's the api client-side method's job to do the conversion, if needed [09:56] TheMue, doing it automagically sounds like opening a deep deep can of worms :) [09:56] dimitern: yes,only thought about our generic errors we've got in juju/errors AND params [09:57] dimitern: hehe, no, not automatically, only as a helper for inside the API client [09:57] dimitern: kind of params.Unwrap(error) error [09:58] TheMue, which will do something like if params.IsCodeXYZ(err) { return errors.NewXYZ(nil, err.Error()) ? [09:59] TheMue, but it's simpler to do this as needed I think, rather than having a giant switch block in a helper like this [10:00] dimitern: yeah, you may be right, feels better and more targeted. especially if there are also API specific errors that always have to be handled extra [10:00] dimitern: so yes, will do it directly, no card, no todo. *lol* [10:01] TheMue, cheers :) [10:09] dimitern: is there a standup on Thursdays? I don't see anything in my calendar. I have a conflict (P&C) so will skip either way. [10:10] frobware, there is, I'll add you [10:10] frobware, it was scheduled separately, because it used to overlap with the core leads call at some point [10:20] fwereade, jam, so, about the retrying hooks on startup thing [10:20] fwereade, jam, it seem it would be more productive to talk here [10:20] fwereade, jam, does it sound like a good idea? [10:21] sure [10:26] bogdanteleaga, sure [10:45] fwereade, jam: ping [10:45] hi rogpeppe [10:45] jam: hiya [10:46] jam: we're just thinking of doing a reasonably wide-ranging change to juju/cmd/... [10:46] jam: just thought i'd run the idea past you before we do the work [10:46] jam: in fact, have you got a moment or two for a hangout? [10:46] I can listen on a hangout, but I have a bit of a cough so I try not to talk too much. [10:47] jam: ok, understood. https://plus.google.com/hangouts/_/canonical.com/gogogo?authuser=1 [10:47] jam: or we can keep it here if you'd prefer [10:48] joining, that way you can talk [10:48] jam, rogpeppe, can't join you just now, will drop in when I can in case you're still there [11:41] jam, rogpeppe: I presume you're not still there? conversations extended... [11:41] jam, rogpeppe: would love to hear the high points [11:44] fwereade: i can only try to recap [11:45] fwereade: but the thing is that for the macaroon based login to work we need to persist macaroons and discharges in a cookiejar.. and to do that we placed the logic in the environCommandWrapper, which now has a Run method [11:46] and that method loads the cookie jar, creates a httpbakery client and uses it to establish an API connection [11:46] but now all commands need to be wrapped.. and test do not do that.. [11:47] and that was the whole point of the debate.. whether to create a constructor for each command that would return a wrapped command and use wrapped commands in tests.. [11:48] fwereade: ^ [11:52] ashipika, my comfort level is proportional to how explicit we're being -- if all the commands are now constructed via funcs that accept explicit httpbakery clients -- so they can be tested by explicitly passing a mocked client -- then great [11:53] ashipika, the more magic/globals/whatever is involved, the less I'll like it [11:54] fwereade: it's no more magic that what's there already [11:54] fwereade: we're putting the logic inside EnvCommandBase [11:54] * fwereade suddenly gets nervous because he can't remember how the magic is distributed ;p [11:55] rogpeppe, but I jest, that sounds like the right place for it [11:55] fwereade: cool [12:10] ericsnow: ping me when you are back please [13:22] Bug #1493850 opened: 1.22 cannot upgrade to 1.26-alpha1: run.socket: no such file or directory <1.22>

[13:25] Bug #1493850 changed: 1.22 cannot upgrade to 1.26-alpha1: run.socket: no such file or directory <1.22>

[13:34] Bug #1493850 opened: 1.22 cannot upgrade to 1.26-alpha1: run.socket: no such file or directory <1.22>

[13:55] anyone else having trouble with the tests not passing on master? [13:56] natefinch: go 1.5? [13:56] ashipika: nope, running 1.2.2 like a good boy ;) [13:56] natefinch: the tests do not pass on master [13:57] mgz: ok, good to know [13:57] mgz: should I make bugs for the failing tests? [13:57] natefinch: ack.. asking because there were some test failures with 1.5 [13:58] hm, one of the failing ones actually passed on a retest [13:58] natefinch: you should file bugs for any that CI does not have in the most recent run [14:22] dimitern, do you have time for a 5 minute HO, though probably more. :) [14:23] frobware, sure :), i'll be in the standup HO in ~2m [14:31] Bug #1493877 opened: TestImplicitRelationNoHooks fails intermittently

=== natefinch is now known as natefinch-afk [14:35] mgz: When you create an issue, remember to link the bug. [14:37] abentley: going back and doing it now [14:46] Bug #1493887 opened: statusHistoryTestSuite teardown fails on windows