/srv/irclogs.ubuntu.com/2012/07/19/#juju-dev.txt

niemeyerEMPTY REVIEW QUEUE00:22
* niemeyer steps out to celebrate00:23
davecheneyafternoon04:04
davecheneyanyone in the chan ?04:05
andrewsmedinadavecheney: hi04:16
davecheneyhowdy04:20
davecheneycan I ask, what happens if you run juju status on a freshly bootstrapped environ ?04:21
davecheneylooking at the code, the result should be yamls {}04:21
andrewsmedinajuju status returns a yaml with one machine (bootstrap machine)04:23
davecheneyahh yes, of course04:26
* davecheney goes back to mocking04:27
andrewsmedinadavecheney: http://bazaar.launchpad.net/~juju/juju/trunk/view/head:/juju/control/tests/sample_cluster.yaml04:31
davecheneyandrewsmedina: ta04:31
andrewsmedinadavecheney: sleep time04:46
andrewsmedinadavecheney: see you tomorrow04:46
davecheneyandrewsmedina: np, thanks for your help04:47
andrewsmedinadavecheney: :D04:52
rogdavecheney: hiya06:41
davecheneymorning06:44
fwereade_rog, davecheney, heyhey06:45
rogfwereade_: yo!06:45
fwereade_rog, does this look in any way familiar to you?06:54
fwereade_../state/state.go:8: inconsistent definition for type goyaml._Ctype_struct___9 during import\n\tstruct { start *goyaml._Ctype_yaml_tag_directive_t; end *goyaml._Ctype_yaml_tag_directive_t; top *goyaml._Ctype_yaml_tag_directive_t }\n\tstruct { start *goyaml._Ctype_yaml_simple_key_t; end *goyaml._Ctype_yaml_simple_key_t; top *goyaml._Ctype_yaml_simple_key_t }\n06:54
fwereade_rog, goyaml builds just fine normally, but falls over like that in TestPutTools06:54
rogfwereade_: it's a cgo error, but i don't think i've seen it before06:55
rogfwereade_: have you tried go build launchpad.net/juju-core/... ?06:55
rogfwereade_: because it looks to me like TestPutTools is failing because it can't build the commands.06:56
fwereade_rog, yes indeed, that is the weirdness06:56
fwereade_rog, it only fails when I clear out pkg06:56
rogfwereade_: hmm, i wonder if it's something to do with GOPATH.06:57
rogfwereade_: what happens if you do GOPATH="" go build launchpad.net/juju-core/cmd/... ?06:57
roghmm, it just should fail with "pkg not found" i guess06:58
fwereade_rog, in PutTools (or whatever that calls) I presume?06:58
fwereade_rog, I'll see :)06:58
rogfwereade_: yeah.06:58
rogfwereade_: so you cleaned out pkg by doing rm -r $GOPATH/pkg ?06:59
fwereade_rog, yeah06:59
rogfwereade_: i'll see if i can reproduce. were you testing all packages, or just one?06:59
fwereade_rog, all packages06:59
rogfwereade_: if you clean out pkg, then do "go install launchpad.net/juju-core/...", *then* start the tests, does it then succeed?07:02
davecheneyrog: fwereade_ have eithre of you installed gustavo's patch to the go tool ?07:02
rogdavecheney: nope.07:02
fwereade_davecheney, nope07:03
rogdavecheney: i don't have a huge $GOPATH/src dir, so it's not so important for me07:03
davecheneycool, i have been playing with it, and had some weirdness07:03
fwereade_rog, I expect so, I will check07:03
fwereade_rog, empty GOPATH just won't build as expected07:04
rogfwereade_: hmm, i've got a test that's hanging again :-(07:06
fwereade_rog, installed packages work just fine07:06
fwereade_rog, bah :(07:06
fwereade_rog, which one?07:06
davecheneyfwereade_: unrelated07:06
rogfwereade_: weird07:06
rogfwereade_: i'm not sure - it only prints the test name when the test has timed out!07:06
davecheneythe machine: section in juju status isn't actually state.Machines is it07:06
davecheneyit environ.Instances07:07
rogfwereade_: i'm gonna kill it and see what's happening07:07
davecheneythe machines: section is reporting details about the environ.AllInstances07:08
rogfwereade_: it's the juju package that's hanging07:08
fwereade_rog, huh, weird... won't -gocheck.vv tell you though?07:09
fwereade_rog, not so helpful if it's unreliable07:09
fwereade_davecheney, sorry, I'm not quite following you07:09
fwereade_davecheney, I'm pretty sure status should be telling us about machines, not instances07:09
davecheneythe two keys that are reported 'machines' in juju status07:09
rogfwereade_: yeah, but i'd already run it without gocheck.vv, and i suspect it's a sporadic failure07:09
davecheneyfwereade_: I think they are what we call environ.Instances07:10
davecheneybecause the first field in the yaml is dns-name07:10
davecheneya property which is only availble on the environ.Instance07:10
rogfwereade_: it's actually hanging trying to dial zookeeper07:10
fwereade_davecheney, I'm suspicious of that assertion, but juju/control/status.py is a rats' nest so I'll have to check ;)07:10
davecheneyfwereade_:                 pm = yield self.provider.get_machine(instance_id)07:11
rogfwereade_: i think that dial should have a timeout actually. it's wrong that zk tries to redial indefinitely.07:11
davecheneyi think we have a cleaner separation of machine, the virtual, and instance, the conrete in the go impl07:11
davecheneyrog: zk never times out07:11
davecheneyit sucks07:11
fwereade_davecheney, STM it's only doing that with the machines it already knows about from state07:11
rogdavecheney: +107:12
fwereade_davecheney, look 17 lines before that07:12
davecheneyfwereade_: as you say, it is an uncomforable union of both concepts07:12
fwereade_davecheney, I dunno, I think status info from both is relevant07:13
fwereade_davecheney, I think it's ok to smoosh them together there07:13
davecheneyfwereade_: thanks for the clarification07:13
davecheneyI was hoping to have more to show07:13
rogfwereade_: apart from that one test hangup, all the tests worked ok for me after removing pkg.07:13
fwereade_davecheney, instances are really an internal detail, but their properties apply to the machines the user cares about07:13
davecheneybut all I have tonight is a harness07:13
fwereade_davecheney, status is not going to be all that small I'm afraid07:14
fwereade_davecheney, probably smaller than the python, but still07:14
davecheneyfwereade_: indeed, which is why I put some time into setting up a table driven harness07:14
fwereade_davecheney, +107:14
davecheneyimma signing off in a few minuts07:14
davecheneythey are closing the coworking space07:14
fwereade_davecheney, enjoy your evening :)07:15
davecheneybut I look forward to your comments07:15
rogdavecheney: have fun07:15
rogdavecheney: when do you leave for lisbon?07:15
davecheneyrog: sunday07:16
davecheneybut I don't get there til monday 14:2507:16
rogdavecheney: i saw that07:16
davecheneyI have NFI how to get from the airport to the hotel07:16
rogdavecheney: there's some info on the sprint wiki page07:16
davecheneyneed to get some euros as well07:16
davecheneyrog: I could have go there a day erlier for an extra 800 euro, and a detour via CDG07:17
rogdavecheney: well, i might catch you tomorrow morning then...07:17
rogdavecheney: marvellous07:17
rogdavecheney: well, i'm glad you're making it regardless!07:17
davecheneyfor small values of yay07:17
davecheneyanyway, i'll catch you around07:18
rogdavecheney: i usually order currency from travelex to be picked up at the airport. that way you get the convenience of just picking it up, but a much better rate.07:18
rogfwereade_: does malta use euros?07:19
fwereade_rog, yeah07:19
rogfwereade_: well aren't you lucky then?!07:19
fwereade_rog, luckyish :)07:19
rog:-)07:20
fwereade_rog, my bank is still shit at getting me more money when I need it overseas07:20
rogfwereade_: not good07:20
fwereade_rog, I usually have to bite down the swears and hit up mygradually dwindling UK funds07:20
davecheneyright-o07:21
davecheneyhave a good day gentlemen07:21
rogdavecheney: and you. enjoy the evening!07:21
fwereade_have a good night davecheney :)07:21
rogfwereade_: is that because they don't like you using your card overseas?07:21
fwereade_rog, I have a credit card that *works* but is such an aggressively shitty deal that I don't really use it07:22
fwereade_rog, they're essentially charging me to borrow the money from myself07:22
rogfwereade_: just get cash in advance...07:22
fwereade_rog, and I have no way of telling what my "balance" is on it07:23
fwereade_rog, I have another one which works only in totally arbitrary circumstances07:23
rogfwereade_: another possibility is to get a "foreign currency" card, which you can charge up as required.07:23
fwereade_rog, twas sold to me as "this will let you get cash from HSBC machines in the UK without us charging you"07:23
rogfwereade_: you get the best rate that way.07:23
rogfwereade_: ha07:24
fwereade_rog, in practice it *sometimes* works everywhere *except* HSBC machines in the UK, where it never works07:24
rogfwereade_: i have a possible idea for why your build might be failing07:24
fwereade_rog, I keep meaning to go and shout at them for a host of reasons but I have better things to do with my time ;)07:24
fwereade_rog, oh yes?07:24
rogfwereade_: i think it must be something in your environment. i'm trying to remember what the gcc heuristics are for finding include files.07:25
fwereade_rog, ahhh, yes, that does make perfect sense07:26
rogfwereade_: i'm wondering if it's right that bundleTools runs in a clean environment with just GOPATH, GOBIN and PATH set07:26
fwereade_rog, I suspect it might be better to just use whatever env the developer is using in the first place07:26
rogfwereade_: what happens if you unset everything in your environment except those vars?07:26
rogfwereade_: that's what i'm thinking07:27
rogfwereade_: ... and then run go install launchpad.net/juju-core/cmd/...07:27
rogfwereade_: i'd be interested to find out what var it was, but i think you're right - the fix is to mutate the env, not come up with a new one07:28
rogfwereade_: (assuming i've got the right diagnosis, of course!)07:28
fwereade_rog, I'm peering at my env and can't see what could be the issue :/07:31
rogfwereade_: i suspect it might be $HOME07:31
rogfwereade_: try HOME="" go install launchpad.net/juju-core/cmd/...07:32
fwereade_rog, sorry, just a mo07:32
fwereade_rog, empty HOME works fine :(07:39
rogfwereade_: try applying lp:~rogpeppe/juju-core/go-build-env and see if it works then07:42
rogfwereade_: (your go test, that is)07:42
rogfwereade_: (the thing that was failing before)07:42
fwereade_rog, will do in a sec, thanks07:43
fwereade_rog, I have a side issue to ponder: in https://codereview.appspot.com/6402048/ niemeyer suggests a RelationHandler type07:45
fwereade_rog, and I'm not actually sure whether *that* has a benefit over putting the methods directly onto Relation07:45
rogfwereade_: yeah, i saw that. i haven't thought it over yet though.07:45
rogfwereade_: yeah, what fields are in RelationHandler other than the Relation itself?07:46
rogfwereade_: oh, hold on, it's got a particular set of relation end points in it07:47
fwereade_rog, that's the only way we have to identify/get a Relation in the first place isn't it?07:47
fwereade_rog, the endpoint list is equivalent to the relation's name07:47
* rog does a godoc to remind himself of the API07:48
rogfwereade_: yeah that does sound right07:48
fwereade_rog, there is *something* up there07:49
fwereade_rog, possibly we ought to be doing it with endpoint identifiers rather than actual RelationEndpoints07:49
fwereade_rog, and doing the translation internally07:49
rogfwereade_: why's that?07:50
fwereade_rog, but I'm not sure that entirely fits expected usage07:50
fwereade_rog, mainly because that will be a lot more convenient for add-relation and remove-relation07:50
fwereade_rog, which is the only time I'm aware of when we do stuff with relations *other than* via something that already exists within state07:51
fwereade_rog, which can identify them how it likes internally07:51
fwereade_rog, same as the name/key distinction07:51
fwereade_rog, btw, go-build-env works great07:51
rogfwereade_: ah good. so *something* in your environment is the culprit!07:52
rogfwereade_: i'm not sure i get it. RelationEndpoint is part of the public API; why not use it?07:53
rogfwereade_: or are you suggesting removing it from the public API?07:53
fwereade_rog, I'm not quite sure :)07:55
fwereade_rog, possibly it is theright approach, and we just need a State.Endpoint(name string) method07:55
fwereade_rog, except, hmm, that's actually State.Endpoints...07:56
rogfwereade_: what would it return?07:56
fwereade_rog, meh, it'll become clearer as I do more with it07:56
fwereade_rog, look, you need a state to construct a meaningful RelationEndpoint, right?07:57
rogfwereade_: BTW i'm all for the idea of reducing the number of types in state07:57
rogfwereade_: do you?07:57
fwereade_rog, yeah, it's all information that comes from the charm07:58
rogfwereade_: well, i suppose you hav... yeah07:58
rogfwereade_: a relation endpoint is always associated with exactly one unit, right?07:59
fwereade_rog, no, with a service07:59
fwereade_rog, the user expresses endpoints as service_name[:relation_name]08:00
fwereade_rog, and we need to do magic to figure out what endpoints are actually referred to by those strings08:01
rogfwereade_: and for a scope=local relation, there's still only one endpoint for the service?08:01
fwereade_rog, and possibly barf if they're ambiguous and ask the user to specify the [:relation] bit08:02
fwereade_rog, yeah, the endpoints are purely about the connections between services08:02
rogfwereade_: so the RelationEndpoint type is about isolating that magic, so that you're dealing with unambiguous things?08:02
fwereade_rog, the details of which units actually talk to one another is handled completely separately, although the choice of how to do so is ofc governed by the endpoint's scope08:03
fwereade_rog, or rather the narrowest scope present in the endpoints of the relation as a whole08:03
rogfwereade_: so you do some magic to produce an endpoint, then use that in the API?08:03
fwereade_rog, I don't do anything, we don't yet have add-relation ;p08:03
rogfwereade_: will do, then :-)08:03
fwereade_rog, but, yeah, that's what we'll have to do08:04
fwereade_rog, I'm saying we should have AddRelation(epnames ...string)08:04
fwereade_rog, ...probably, anyway08:04
fwereade_rog, I'm not 100% sure08:04
rogfwereade_: where epnames are potentially ambiguous?08:05
fwereade_rog, quite possibly we should actually have State.RelationEndpoints(epnames ...string) ([]RelationEndpoint, error)08:05
rogfwereade_: that's Service.AddRelation, presumably?08:05
fwereade_rog, yes08:05
fwereade_rog, no, State.AddRelation08:06
rogfwereade_: ah.08:06
rogfwereade_: i'm not sure if that magic needs to be in state08:08
rogfwereade_: it seems to me like it's something that can be built using the primitive already available.08:08
rogprimitives08:08
fwereade_rog, yeah, reasonable perspective08:09
rogfwereade_: it seems to me like that's the meat of the logic of add-relation08:10
rogfwereade_: and it could sit happily in the juju package08:10
fwereade_rog, ok, I think I'm convinced, thanks :)08:10
rogfwereade_: then the state API is still very general08:10
rogfwereade_: cool08:11
fwereade_rog, (also the meat of remove-relation)08:11
rogfwereade_: but... i'm not sure we've answered the original question08:11
fwereade_rog, (although I suspect we should have a slightly different algorithm there)08:11
rogfwereade_: should RelationHandler just be Relation?08:11
rogfwereade_: ah, no, it can't be08:12
fwereade_rog, I'm leaning towards "yes"08:12
fwereade_rog, really? bother08:12
rogfwereade_: because it's more than a relation - it encapsulates a set of units cooperating in a relation08:12
fwereade_rog, all that is implicit in the *Unit params to the methods, innit?08:13
rogfwereade_: so for locally scoped relations, you'll have a RelationHandler for each machine running the service08:13
fwereade_rog, no you won't08:14
rogfwereade_: no?08:14
fwereade_rog, you just do rh.Join(localUnit); rh.Watch(localUnit), surely?08:14
fwereade_rog, ok, the name of Watch is sl. misleading08:14
fwereade_rog, WatchAsThoughYouWere(localUnit) ;)08:15
fwereade_rog, the rh itself is the "same" object regardless of what UA is running it08:15
rogfwereade_: but if all machines have also doing rh.Join, won't we see changes from all machines?08:15
fwereade_rog, well, in a global relation, yes; in a locally scoped relation, no08:16
rogoh i think i see08:16
fwereade_rog, it figures out what scope to watch from the unit we pass in08:16
fwereade_rog, ie it finds all the units in the same scope as the one suggested08:16
rogfwereade_: you're relying on the fact that every time we get a relation, it's freshly made08:16
fwereade_rog, expand please08:16
rogfwereade_: are you saying that the Join method doesn't actually affect the state at all?08:18
roghmm, i think i'm confused08:18
fwereade_rog, no, I'm not, but I can't connect that with what you're asking08:19
fwereade_rog, that would not surprise me, I *think* I now get relations, but it has been a somewhat arduous path08:19
rogfwereade_: i'll try and recap; tell me where i go wrong.08:19
fwereade_rog, cool08:20
rogfwereade_: someone does add-relation svc1:relation1 svc2:relation2; we look up the endpoints for both of those, and call State.AddRelation with those.08:22
fwereade_rog, yes08:22
rogfwereade_: the unit agent on each machine sees the new relation and calls Join on it.08:23
fwereade_rog, yes08:24
rogfwereade_: the unit agent runs the relation1-relation-joined hook.08:24
fwereade_rog, no08:24
rogah, no i see08:25
fwereade_rog, a *different* unit agent runs that hook when it detects that the first UA called Join08:25
rogof course08:25
rogfwereade_: ok, the unit agent see *anouther* machine that has called Join on the new relation.08:25
fwereade_rog, yep08:25
rogfwereade_: so it runs the relation1-relation-joined hook.08:25
fwereade_rog, (ofc it only sees *that* because it's watching the relation from its own perspective, by having called Watch(itsOwnUnit)08:26
fwereade_rog, yes08:26
rogfwereade_: but how does this work with locally scoped relations?08:26
fwereade_rog, the state.unitScopePath is at the heart of it08:27
fwereade_rog, are you familiar with the contents of the /relations/relation-XXX node?08:27
rogfwereade_: remind me :-)08:27
fwereade_rog, if it's a globally scoped relation, it has a settings subnode, and a subnode for each role in the relation08:28
fwereade_rog, if it's container-scoped, it contains a subnode for each container that holds some unit participating in the relation, and each of those container nodes contains the role and settings nodes08:28
fwereade_rog, when watching, or joining, on behalf of a given unit, we figure out what the role-and-settings-parent node is for that unit, based on the relation's scope and (maybe) the unit's container08:29
fwereade_rog, and then we just watch/write to the contents of that tree08:29
fwereade_rog, (note: the top-level relation node of a container-scoped relation does *not* have role or settings nodes)08:30
fwereade_rog, make sense?08:30
fwereade_rog, (similarly, the Settings(*Unit) method does the same thing)08:31
rogfwereade_: ah, so the Join method does the magic based on the unit08:32
fwereade_rog, yes; really, *every* Relation(Handler)? operation depends on a specific unit to get the appropriate perspective08:32
rogfwereade_: BTW unitScopePath doesn't actually seem to be used anywhere08:32
fwereade_rog, update your tree :)08:33
rogfwereade_: ah ha!08:33
fwereade_rog, and then for examples of actually constructing a useful one, take a look at the relation-unit branch itself08:33
fwereade_rog, in Unit.AgentJoin08:34
fwereade_rog, it's very small but I'm rather pleased with it08:36
fwereade_rog, the python seems to do path-hackery all over the place :)08:36
rogfwereade_: ok, so i *think* i understand now, and it seems reasonable then that the methods should be directly on the Relation.08:36
fwereade_rog, cool08:36
fwereade_rog, I think I'll talk about it with niemeyer too, but it's good to know I'm not *obviously* all underpants-on-head about it ;)08:37
rogfwereade_: how would Relation.Join(unit) differ from Unit.AgentJoin(relation) BTW?08:38
fwereade_rog, it's only responsible for the Pinger08:39
fwereade_rog, AgentJoin does both the pinger *and* the watching *and* gives access to the settings node08:39
fwereade_rog, I *suspect* we'll still end up with something similar in effect to the RelationUnit but I can't see far enough into the future to be sure08:40
fwereade_rog, the precise types we need to sling around will become clearer as I work on the queuing and tie it into cmd/jujuc/server.ClientContext08:41
rogfwereade_: if RelationHandler is actually Relation, then it seems like we've managed to lose a type, which sounds like a good thing.08:42
rogfwereade_: well, lose a public type anyway08:42
fwereade_rog, yeah, I think so08:42
rogfwereade_: i'm definitely +1 on that08:43
fwereade_rog, cool :)08:43
rogfwereade_: i think Join, Settings and Watch on Relation is an easier thing to understand08:43
rogfwereade_: i hadn't quite grasped what a RelationUnit actually *was* :-)08:43
fwereade_rog, and we don't have ServiceRelation any more, I never really understood what that was either ;)08:44
rogfwereade_: cool :-)08:44
* rog likes it when an API comes together08:45
rogfwereade_:  https://codereview.appspot.com/642104509:05
fwereade_rog, LGTM09:06
* fwereade_ tuts in niemeyer's general direction09:07
fwereade_(config gofmt ;))09:07
* fwereade_ suddenly grasps the full horror of how the tests will look if he tries:09:08
fwereade_var hookQueueTests = []struct {09:08
fwereade_    adds []state.RelationUnitsChange09:08
fwereade_    gets []relationer.HookInfo09:08
fwereade_}{09:08
fwereade_...given that:09:09
fwereade_/ RelationUnitsChange holds settings information for newly-added and -changed09:09
fwereade_/ units, and the names of those newly departed from the relation.09:09
fwereade_type RelationUnitsChange struct {09:09
fwereade_    Changed  map[string]UnitSettings09:09
fwereade_    Departed []string09:09
fwereade_}09:09
fwereade_/ UnitSettings holds information about a service unit's settings within a09:09
fwereade_/ relation.09:09
fwereade_type UnitSettings struct {09:09
fwereade_    Version  int09:09
fwereade_    Settings map[string]interface{}09:09
fwereade_}09:09
fwereade_...and:09:09
fwereade_/ RelationUnitsChange holds settings information for newly-added and -changed09:09
fwereade_/ units, and the names of those newly departed from the relation.09:09
fwereade_type RelationUnitsChange struct {09:09
fwereade_    Changed  map[string]UnitSettings09:10
fwereade_    Departed []string09:10
fwereade_}09:10
fwereade_/ UnitSettings holds information about a service unit's settings within a09:10
fwereade_/ relation.09:10
fwereade_type UnitSettings struct {09:10
fwereade_    Version  int09:10
fwereade_    Settings map[string]interface{}09:10
fwereade_}09:10
* fwereade_ ponders grumpily09:10
TheMuemorning09:34
TheMuerog: Btw, your log-tracking-testing will soon fly into the trunk. Just doing the final steps.10:23
rogTheMue: cool. i saw gustavo's remarks, and breathed a sigh of relief that he thought it was ok :-)10:24
rogTheMue: morning, BTW!10:24
TheMuerog: morning and yes, it's indeed a nice idea. didn't thought so in the beginning, but seeing it in real life feels good.10:25
rogTheMue: it went through a couple of more complex iterations before it arrived at the paste i gave you. i'm quite happy how it turned out actually.10:26
rogfwereade_: erm, you pasted the same thing twice10:28
rogfwereade_: did you mean to post the HookInfo type definition?10:28
fwereade_rog, oh, blast, yeah10:28
fwereade_type HookInfo struct {10:28
fwereade_    Name       string10:28
fwereade_    RemoteUnit string10:28
fwereade_    Members    map[string]map[string]interface{}10:28
fwereade_}10:28
fwereade_rog, but not to worry, it's readable now10:28
fwereade_rog, there are rather a lot of cases to deal with, though ;)10:29
rogfwereade_: what are the names in RelationUnitsChange.Departed, BTW? unit names?10:39
TheMueAnd once again heavy rain. Hey, weather god, it's enough!10:39
rogTheMue: you'll be too hot next week!10:42
TheMuerog: Yes, I've seen, and I promised my family to bring some good weather with me like from Oakland.10:42
rogfwereade_: also, it seems odd that RelationUnitsChange gives no way of telling *which* unit changed. is that because we don't need to know?10:43
TheMuerog: Tomorrow the school holidays begin, so it *has* to change. ;)10:43
hazmatec2 ssd instances!10:50
hazmathttp://aws.typepad.com/aws/2012/07/new-high-io-ec2-instance-type-hi14xlarge.html10:50
rogTheMue: we haven't really had a summer here. it's been continual rain. and when i got back, found that there'd been some serious rain while we were away: http://www.youtube.com/watch?v=huCnxI8qDb0 http://www.youtube.com/watch?v=lpJVTCLHKUE10:52
roghazmat: sounds expensive :-)10:53
hazmatrog, depends on if you need it.. netflix reduced their costs by half for the same throughput on a benchmark.. http://techblog.netflix.com/2012/07/benchmarking-high-performance-io-with.html10:54
roghazmat: interesting10:55
TheMuerog: Hard storms, yes. We had some of those in July too. May/June have been some wonderful warm days where I worked on the veranda.10:55
TheMueHmm, Gavin Harrison really motivates me to realize an old dream: learn to play drums.10:58
TheMueHe's the drummer of Porcupine Tree.10:58
rogfwereade_: i don't think you'd need an intention log per relation11:01
rogfwereade_: i *think* that one log would suffice11:01
rogTheMue: one forecast i saw had it at 37 degrees in lisbon on one day11:03
TheMuerog: ouch11:09
rogTheMue: but mostly around 30, which is bearablew11:09
rogs/w//11:09
TheMuerog: Yes, just took a look too. 30 is fine11:16
fwereade_rog, sorry, lunch :)11:24
rogfwereade_: np!11:24
fwereade_rog, RelationUnitsChange has Changed map[unitname]info, Departed []unitname11:25
fwereade_rog, you're expected to know which relation changed because you started watching a particular relation11:26
rogfwereade_: ah, i think the documentation should mention that the key of Changed is the unit name :-)11:26
fwereade_rog, sensible, cheers11:26
rogfwereade_: i glanced at it my brain said "attribute name"11:27
fwereade_rog, I thought it kinda went without explicitly saying when I wrote it but it could certainly be clearer11:27
rogfwereade_: some of are stupider than you might think :-)11:27
fwereade_rog, I'm interested to hear about your further thoughts on intention logs though11:28
fwereade_rog, (sorry I had to dash yesterday)11:29
rogfwereade_: np11:29
rogfwereade_: i'm thinking that when we get an error executing an intention, we can write "broken" to the intention log.11:30
rogfwereade_: that doesn't stop the updates coming through the queue for that relation, of course.11:30
rogfwereade_: but i'm reconsidering if having the queue in a separate goroutine is the right approach after all11:30
fwereade_rog, sorry, I'm not coming up with any responses11:33
rogfwereade_: that's fine, it's all vapour anyway :-)11:34
rogfwereade_: just my vague thoughts on the matter. you're much closer to the problem.11:34
fwereade_rog, I am definitely having difficulty visualising an intention log with the unit lifecycle state and all the relation lifecycle states and brokens and dones and collapses and resolve-handling :)11:35
fwereade_rog, collapses and resolves, in-memory, for a single relation, is occupying most of my brain ATM :)11:35
fwereade_rog, I think I have something that will work and be easy enough to extend to handle process bounces, but we'll see11:37
rogfwereade_: this kind of API, perhaps? http://paste.ubuntu.com/1099903/11:44
fwereade_rog, ATM I have http://paste.ubuntu.com/1099905/11:46
rogfwereade_: i'm not sure just RelationUnitsChange is enough, as it doesn't hold the name of the relation that changed.11:48
rogfwereade_: of course, it could get that11:48
fwereade_rog, I'm assuming one of these per relation11:49
fwereade_rog, and that the client should know what relation they're dealing with11:49
rogfwereade_: that sounds good actually11:50
fwereade_rog, it's about all I can fit in my head ATM :)11:50
rog:-)11:50
rogniemeyer: yo!12:28
niemeyerMorning!12:29
TheMueniemeyer: Heya12:30
rogniemeyer: what if there's already a setting for GOBIN in the environment? are later entries guaranteed to override earlier ones?13:10
fwereade_niemeyer, heyhey!13:12
fwereade_niemeyer, rog, I have an in-memory HookQueue for your perusal: https://codereview.appspot.com/642204913:13
rogfwereade_: cool13:14
rogfwereade_: (not entirely sure about "relationer" as a name, even though we already have the dubious "machiner")13:15
niemeyerrog: I guess we shouldn't trust it indeed13:16
rogfwereade_: how about just "hook"13:16
niemeyerrog: It works, but who knows13:16
fwereade_rog, because this is only one of many components of a system that will deal purely with unit agents' participation in relations?13:16
rogniemeyer: yeah, it seems impl dependent to me13:16
rogfwereade_: i'd really expect to see "uniter" :-)13:17
fwereade_rog, uniter will come13:17
rogfwereade_: under worker/, anyway13:17
fwereade_rog, that has a whole bunch of other responsibilities, including kicking off stuff in relationer13:17
rogfwereade_: maybe worker/uniter/relation ?13:18
fwereade_rog, like how we have worker/provisioner/firewall? :p13:18
rogfwereade_: in a way, firewaller is an independent worker in its own right.13:19
rogfwereade_: i'm not *sure* that can be said for relationer13:19
rogfwereade_: the fact that the firewaller happens to run in the PA is historical accident IMO13:19
fwereade_rog, I guess I could live with worker/uniter/relation/hookqueue.go, but the unit stuff and the relation stuff are really pretty distinct13:21
rogfwereade_: "hooks" seem fairly closely tied to the unit agent to me13:22
rogfwereade_: i'd be happy for all this to live inside the unit agent, tbh13:22
rogfwereade_: not entirely sure what we gain by having a separate package13:23
fwereade_rog, mainly what we gain is an opportunity to avoid the ickiness of python, in which we have relation stuff and unit stuff smeared across the same package together despite the connections between the two being very very tenuous13:24
rogfwereade_: i'd've thought that we could make that distinction very clear by the types we use13:24
fwereade_rog, the unit stuff starts and stops the relation stuff, it is true, but the only other point of contact that I can recall is a shared hook executor, which feels to me much like the environ shared between the provisioner and the firewaller13:25
fwereade_rog, uniter.UnitWorkflow/RelationWorkflow, uniter.UnitLifecycle/RelationLifecycle feels nasty to me13:26
rogfwereade_: that's ok, i think. we already have fairly self-contained modules within the same package.13:26
fwereade_rog, compared to uniter.Workflow/Lifecycle, relationer.Workflow/Lifecycle13:26
rogfwereade_: does any of this stuff need exporting?13:26
rogfwereade_: i think of all this as internal details of the unit agent/worker.13:27
fwereade_rog, certainly the workflow stuff13:27
fwereade_rog, or, hmm, maybe we can get away without that13:27
rogfwereade_: by workflow stuff, you mean?13:27
fwereade_rog, ZK storage of unit/relation state, used by status, but that will probably want to be a separate package in itself13:28
rogfwereade_: feels to me like that would live in state13:28
fwereade_rog, also disk storage, I think, although we my come up with some nice way around that13:29
fwereade_rog, anyway, I think we have now discussed this enough to make the necessary move half a dozen times if it becomes apparent that it is misplaced ;)13:30
rogfwereade_: tbh, i'm happy having the relation hook queue stuff in separate package if it's really self-contained, but "relationer" isn't the right name. it's not an "er" :-)13:30
fwereade_rog, I am impressed by your precognitive ability :/13:30
rogfwereade_: ok, i may be wrong!13:30
rogfwereade_: as always...13:31
fwereade_rog, not to worry, but the code in there is about 1000 times more interesting than the name of the package IMO13:31
rogfwereade_: indeed, i'm looking through it. just thought i'd convey my naming discomfort...13:32
fwereade_rog, no worries :)13:33
rogfwereade_: any particular reason you need to go through the changed units in alphabetical order, BTW?13:34
fwereade_rog, consistent tests13:34
* rog just noticed the comment, duh13:34
fwereade_:)13:34
* fwereade_ thinks he's spotted a bug13:35
* fwereade_ writes a test13:35
* fwereade_ stops and tries to figure out how to13:36
* fwereade_ thinks it's not a bug13:39
fwereade_niemeyer, ping13:39
niemeyerfwereade_: Yo13:40
fwereade_niemeyer, I was wondering if there was any particular reason to have RelationHandler separate from Relation?13:40
* niemeyer looks at what Relation is today and how it's used13:40
fwereade_niemeyer, and I also think that Watch() should really return a RelationUnitsWatcher, which would then be an exported type, and could be tested in a more pleasing fashion13:42
niemeyerfwereade_: Both suggestions look sane13:43
fwereade_niemeyer, sweet, thanks13:43
rogfwereade_: the hook queue code looks reasonable13:47
rogfwereade_: one question: do we actually need a queue there?13:47
fwereade_rog, sorry, where?13:47
rogfwereade_: could we just keep a bag of stuff waiting to be done?13:47
rogfwereade_: in the hook queue13:47
rogfwereade_: does it matter which order the relation hooks are executed (between relations, that is)?13:48
fwereade_rog, across different relations, no it doesn't13:48
fwereade_rog, within one relation yes it does13:48
rogfwereade_: i was wondering about a data structure like this: http://paste.ubuntu.com/1100094/13:49
fwereade_rog, needs versions but that's by the by13:50
rogfwereade_: yeah13:50
fwereade_rog, the prospect of entirely dropping ordering seems odd to me13:51
rogfwereade_: it's probably crack, mind13:51
rogfwereade_: well, we're not *entirely* dropping it :-)13:51
rogfwereade_: and if we restart, the order is going to be arbitrary anyway...13:51
fwereade_rog, it feels like we'd need to add a bunch of icky heuristic stuff to ensure we didn't just, say, not bother to handle a departed for 20 minutes because there were lots of interesting changes going on13:51
rogfwereade_: statistically that would be very unlikely :-)13:52
rogfwereade_: BTW shouldn't Done call remove() ?13:55
fwereade_rog, remove() is dead13:55
fwereade_rog, proposal is updated13:56
fwereade_rog, and the queue is updated in Next13:56
fwereade_rog, anyway I'm not *sure* but it feels like the unordered solution would be more churny with a heavily loaded queue14:01
fwereade_rog, and I think there's something to be said for predictability14:01
rogfwereade_: here's a more predictable solution in the same vein: http://paste.ubuntu.com/1100115/14:01
rogfwereade_: we have a simple linked list of units14:01
fwereade_rog, and I think ensuring joined-then-immediately-changed might also be kinda icky14:02
rogfwereade_: i know it's probably utterly irrelevant, but i'd like to see the queue operations be O(1), and i don't think it's hard.14:03
fwereade_rog, that's pretty nice, I'm still not sure it does anything other than push the complexity around, but I'll look into it some more14:04
rogfwereade_: i'm pretty sure it makes it more efficient. but simpler... i dunno.14:04
rogfwereade_: i'm hoping so.14:04
fwereade_rog, maaaaybe :)14:06
fwereade_rog, it's missing membership and settings information14:07
fwereade_rog, and I'm not sure it's entirely trivial to add them cleanly14:08
fwereade_rog, (also, list needs to be doubly linked, but that is also by the by)14:08
rogfwereade_: i thought a single link would probably be ok14:09
fwereade_rog, how do we reorder in O(1)?14:09
rogfwereade_: we do we reorder?14:09
rogs/we/why/14:09
rogfwereade_: members looks like it's applied only by Next, and probably wouldn't change that much.14:11
rogfwereade_: settings is in unitStatus.settings14:11
fwereade_rog, because some earlier events are replaced by later events, eg depart on top of change, or change on top of depart14:11
fwereade_rog, yeah, but you're missing all the units that aren't in the queue14:11
rogfwereade_: no reordering required there, i think.14:11
rogfwereade_: and i *think* that the unitStatus map can contain all units, not just ones in the queue14:13
fwereade_rog, hmm, the python did need it, but I think you're right; this implementation doesn't14:13
rogfwereade_: (ok, i realise that's definitely a change from where i started :-])14:13
fwereade_rog, sure, that's the nature of the beast14:14
fwereade_rog, serializing that with all those pointers will be icky, but serialization and reconciliation is going to be interesting regardless I think14:15
rogfwereade_: serialisation would be easy i think, but i'm still not convinced we want to serialise this.14:16
rogfwereade_: interesting, yes14:16
fwereade_rog, this has all the state we need to serialize wrt the hook queue, I think14:18
rogfwereade_: i'm still thinking that incremental might be the way to go, but that may be entirely crack14:19
rogfwereade_: http://paste.ubuntu.com/1100165/14:31
rogfwereade_: i'll get back to what i'm supposed to be doing now, erk!14:32
rogfwereade_: when you run a departed hook, does it have access to the old settings of the remote unit?14:51
fwereade_rog, no14:51
rogfwereade_: hmm, that seems a pity in a way. i can see that it might be useful.14:52
fwereade_rog, I think it's just *gone*, too bad, is the thinking :)14:52
rogfwereade_: it means that if you had any resources associated with some attribute exposed by the remote unit, you'd have to keep track of it yourself. but actually, i can see stronger arguments for the settings not being available - if you restart, you won't have access to them.14:53
TheMueAargh! What's that? My proposal ended with a "can't upload base of .lbox: ERROR: Checksum mismatch" and the original issue is messed up. I thought I did it right with the prerequisite this time. *hmpf*14:55
* TheMue softly switches into panic mode15:01
TheMue*phew*15:14
TheMueWrote down a recipe ones and it worked.15:14
* niemeyer => lunch15:26
fwereaderog, fwiw, there is definite ugliness related to restarting when there's an inflight hook in which some members no longer exist15:41
fwereaderog, haven't quite figured out what to do about that yet15:42
rogfwereade: hmm, how is that awkward?15:43
fwereaderog, we don't really want to just remove members from the relation without executing departed hooks15:44
fwereaderog, we can't sanely execute the departed hooks while we're in a screwed state from the previous errored hook15:44
rogfwereade: ah! you mean resolving, not restarting?15:45
fwereaderog, I mean restarting, I was using inflight to denote resolviness15:45
rogfwereade: ah, so we're in an error state when we restart?15:45
fwereaderog, yeah15:46
fwereaderog, or, well, we might be15:46
fwereaderog, the hook queue doesn't have to worry about *that* at least15:46
rogfwereade: hmm, maybe the --retry could be interpreted as "retry that hook eventually" :-)15:48
rogfwereade: so you'd run the departed hooks first15:48
fwereaderog, and what if it's just in limbo?15:48
rogfwereade: sorry, what if _what_ is just in limbo?15:49
fwereaderog, the queue... sitting there for an arbitrary amount of time, waiting for resolution15:50
rogfwereade: well, we know that the queue is waiting for resolution, right? i'm not sure i see the problem.15:51
fwereaderog, the relation should surely still contain its original members until we have executed departed hooks?15:52
fwereaderog, but we don't have access to those members, because they don't exist any more15:52
rogfwereade: what can access those members? we're not running any hooks for that relation, no?15:53
fwereaderog, doesn't the hook that errored (or just was not completed) want them?15:53
fwereaderog, forget resolved entirely15:53
rogfwereade: if we forget resolved, then we have to assume that the hooks all complete successfully, no?15:54
fwereaderog, ha ha :)15:54
fwereaderog, if we haven't said Done(), we can't assume that, can we?15:54
fwereaderog, it is *important* that we don't skip hooks ;)15:55
rogfwereade: i think that if the unit agent goes down while executing a hook, we could treat that as an error and require resolution15:55
rogfwereade: after all, we won't *know* if the hook failed or not15:55
fwereaderog, could do, might make life easier for us15:56
fwereaderog, but there *should* be nothing wrong with re-executing a hook15:56
rogfwereade: here's another little question: what happens if a relation gets departed while its join hook is executing. are we still required to execute the change hook?15:58
fwereaderog, no, tested15:58
rogfwereade: so we can execute joined then immediately departed15:59
fwereaderog, yeah, the python knowingly does that15:59
rogfwereade: cool. i thought that *should* be the case, but...15:59
rogfwereade: so there *is* something wrong with re-executing a hook, in general.15:59
fwereaderog, sorry, why is that so?16:00
rogfwereade: if we crashed while a change hook is executing, and the relation is departed while we're down, we don't want to re-execute the change hook, but the departed hook, i think.16:00
fwereaderog, you know what? all these are specific cases of the general "we sometimes need access to relation unit settings after the units are gone" problem16:02
fwereaderog, the python addresses this by saying LALALA I CAN'T HEAR YOU when you point this out16:03
rogfwereade: i'm not sure16:03
fwereaderog, andtrusting that nobody ever actually deletes relation unit settings16:03
rogfwereade: i'm not sure i see any places above where we need access to relation unit settings after the units are gone.16:08
fwereaderog, that happens *all* the time16:09
fwereaderog, whenever two units depart at the same time?16:09
fwereaderog, we execute 2 departed hooks16:09
rogfwereade: in fact... we always provide the "latest" version of a unit's settings. what happens if that unit has departed already but we haven't seen the departed hook yet16:09
rog?16:09
fwereaderog, the second unit is still apparently present during the first depart16:09
SpamapSFYI, http://www.oscon.com/oscon2012/public/content/video sabdfl keynoting about juju16:10
fwereaderog, that should not be such a problem, we just use the ones we already have16:10
rogfwereade: ah, but not if we've restarted, right?16:10
fwereaderog, indeed16:10
rogfwereade: ah, and that's what you meant by "nobody ever actually deletes relation unit settings"16:10
rogfwereade: ... from zk, yes?16:10
* TheMue watches Mark16:13
* rog too16:13
fwereade_rog, sorry, the last thing I said was "rog, but that does kinda offend my sensibilities a little"16:14
rogfwereade_: last thing i saw was "indeed"16:15
fwereade_rog, unless we do what we do in python and just hope they exist, at least16:15
fwereade_rog, which they will, as long as we don't GC them until the whole relation is gone16:15
fwereade_rog, but that does kinda offend my sensibilities a little16:15
rogfwereade_: yeah, me too16:15
fwereade_the *easy* solution, of which niemeyer is not fond for entirely sensible reasons, is to store all the settings as well16:15
fwereade_rog, blast, gtg :(16:16
rogfwereade_: k16:16
fwereade_rog, I am interested to hear any brainwaves you may have16:16
rogfwereade_: i'll continue thinking on it16:17
fwereade_rog, cheers16:17
rogfwereade_: i'm around late this evening BTW16:17
rogfwereade_: though i should pack!16:17
fwereade_rog, I *might* be, my later plans remain unclear16:17
rogTheMue: crossed fingers the demo works :-)16:19
TheMuerog: Yes ;) Always the hard part.16:19
rogessentially16:21
TheMuerog: That has been nice.16:21
TheMuerog: Ha, here he smiles.16:21
rogTheMue: why bother checking that machineUnitsChanges is closed at all? we never close it - there's no need to check closed on *every* channel receive, i think.16:35
TheMuerog: Which line?16:40
rogTheMue: line 6416:40
rogTheMue: case change, ok := <-fw.machineUnitsChanges:16:41
rogTheMue: why not just: case change := <-fw.machineUnitsChanges ?16:41
rogTheMue: it's trivially verifiable that we don't close the channel16:42
TheMuerog: just a habbit, add a note16:42
TheMueafk16:58
niemeyerfwereade__: ping17:44
niemeyerI'll step out for a coffee break, back soon20:13
niemeyerBack20:58
fwereade__niemeyer, heyhey, I'm around for a bit21:00
niemeyerfwereade__: yo21:00
fwereade__niemeyer, what can I do for you?21:00
niemeyerfwereade__: I just wanted to run a seed concept by you21:01
fwereade__niemeyer, cool21:01
niemeyerfwereade__: I haven't reviewed your branch yet, but hopefully will do that today still (just working a bit on config)21:01
niemeyerfwereade__: But,21:01
niemeyerfwereade__: One thing that we talked about at some point in the past21:01
niemeyerfwereade__: and that seems curious in the context of the "hook queue"21:01
niemeyerfwereade__: Is that, in fact, we may not really need a *queue* per se21:02
niemeyerfwereade__: The only thing we need to know is what to run next21:02
niemeyerfwereade__: That is, we don't have to take action on events coming from ZooKeeper immediately, as we're doing something else21:03
niemeyerfwereade__: Because we have to handle the possibility of losing events in case of errors anyway21:03
* fwereade__ continues to listen with interest21:04
niemeyerfwereade__: So, in a way, this means we can be more comfy and simply consume the next event until we have a decision to do something at hand21:04
niemeyerfwereade__: and then let the next queue of events completely unhandled while we do that21:05
niemeyerfwereade__: Does that ring any bells?21:05
fwereade__niemeyer, so essentially we just snapshot the state whenever we think it might be a good time to run some hooks, and at that point figure out the diff?21:06
niemeyerfwereade__: Not even snapshot21:06
niemeyerfwereade__: We consume events from the queue until we have a decision on what to do21:07
niemeyerfwereade__: But the "queue" is the network itself, and the zookeeper library.. we don't have to be processing everything in advance21:08
niemeyerfwereade__: Of course, we still need the logic for keeping track of where we stand, etc21:09
fwereade__niemeyer, I am having a little bit of trouble seeing the distinction21:09
niemeyerfwereade__: and recording that for error recovery21:09
niemeyerfwereade__: Maybe there's none..21:10
fwereade__niemeyer, rog had some interesting ideas about it not really actually needing to be ordered, which I thought were worthy of a bit of exploration21:10
niemeyerfwereade__: But if nothing else, I see a small conceptual difference in trying to maintain a queue of upcoming events, and simply computing what's the *next* event21:11
niemeyerfwereade__: Uh.. that's not what I mean21:11
niemeyerfwereade__: Order is important, I suspect21:11
fwereade__niemeyer, that is my *instinct* but I can't quite figure out *why*, apart from it being an awful lot easier to get my head around21:12
fwereade__niemeyer, be that as it may, I don't want to derail21:12
niemeyerfwereade__: That'd be good enough of a reason to keep it ordered, but a unit departing and then joining sounds very different from joining and then departing :)21:12
fwereade__niemeyer, what the hook queue *does*, though, both here and in python, is just keep track of the next event per unit21:12
niemeyerfwereade__: Next event, or next eventS21:13
fwereade__niemeyer, and I'm not sure we can reduce that to just the next event without dropping considerations of order21:13
niemeyer?21:13
fwereade__niemeyer, next event singular21:13
fwereade__niemeyer, it collapses operations aggressively21:13
fwereade__niemeyer, if there's more than one queued event per unit we've done it wrong ;)21:15
niemeyerfwereade__: Is there ever a case where we have more than one pending operation?21:15
fwereade__niemeyer, joined/changed but that's handled internally21:15
niemeyerfwereade__: Okay, cool21:15
niemeyerfwereade__: So maybe it's already optimal21:15
fwereade__niemeyer, well, it's surely not *optimal*, I definitely intend to look into a couple of rog's less surprising ideas21:16
niemeyerfwereade__: That's really it. I wasn't sure if there was anything to adapt, but I had this thing in my head that I pondered if could be useful or not21:16
fwereade__niemeyer, I'm pretty sure it's a little nicer than the python21:16
fwereade__niemeyer, but I actually recommend you not worry about that review until tomorrow21:17
fwereade__niemeyer, because it *might* get much better21:17
niemeyerfwereade__: Ohh, exciting :-)21:17
fwereade__niemeyer, and even if it doesn't, a day's delay won't hurt :)21:17
niemeyerfwereade__: True21:18
niemeyerfwereade__: Have plenty to do on the config side21:18
niemeyerfwereade__: I'm hoping to push the whole thing for review today still21:18
fwereade__niemeyer, I confess to being a little relieved that you haven't done the whole config thing in 5 minutes flat ;)21:18
niemeyerfwereade__: Haha :)21:19
niemeyerfwereade__: I'm taking the chance to clean up a few other edges too, such as bring into life that old StringMap type21:19
fwereade__niemeyer, I do have one worry about the relation hooks, though21:19
fwereade__niemeyer, nice21:20
niemeyerfwereade__: I'll also twist Schema so it handles defaults by itself21:20
fwereade__niemeyer, +121:20
niemeyerfwereade__: Oh, what's that?21:20
fwereade__niemeyer, you remember we discussed sending settings through that whole pipeline rather than just versions?21:20
niemeyerfwereade__: Right21:21
fwereade__niemeyer, I'm not sure we actually really gain very much from that21:21
niemeyerfwereade__: Hmm21:21
niemeyerfwereade__: Why's that?21:21
fwereade__niemeyer, because I can't see any way around us *sometimes* needing to access settings from departed units21:21
niemeyerfwereade__: I'm missing both ends of the problem I think21:22
fwereade__niemeyer, ok, simplest possible case21:22
fwereade__niemeyer, two units depart at once21:22
niemeyerfwereade__: Ok21:22
fwereade__niemeyer, we run hooks for the first one, claiming the second one is still part of the relation, and so it's perfectly legitimate for the hook to go through the membership and do something based on the settings for the departed unit that it doesn't yet know is departed21:24
fwereade__niemeyer, with settings cached in memory this is nice and easy21:24
niemeyerfwereade__: I actually think we should allow units to query settings for departed units21:24
niemeyerfwereade__: But not sure if that's relevant for the problem you're explaining21:25
fwereade__niemeyer, I think it's very relevant21:25
fwereade__niemeyer, the idea that we then can't really GC any relation settings node until the whole relation is down bothers me a little, because it feels somehow untidy21:26
fwereade__niemeyer, *but* I don;t think there's any way past that21:26
fwereade__niemeyer, excpet to cache the whole damn relation state to disk all the time, which is just too much to deal with21:27
niemeyerfwereade__: It would not solve either way.. the unit could have changed its settings before departure21:27
niemeyerfwereade__: I think it's fine to leave the garbage uncollected for a while21:27
fwereade__niemeyer, true21:27
niemeyerfwereade__: We can GC it eventually in the corrective agent hinted at over the lifecycle conversation21:28
fwereade__niemeyer, yeah, I'm happy enough not worrying about that really, it hasn't seemed to hurt in python too much21:28
niemeyerfwereade__: Okay, but what's that issue that makes sending settings with the version non-useful?21:29
fwereade__niemeyer, ok, so, we end up with a situation where any relation hook might relation-get some departed unit (whether or not it is known to be departed according to the "current" membership)21:29
fwereade__niemeyer, so we need both the get-arbitrary-settings code that we currently have in python *and* the use-cached-settings code we're working towards here21:30
niemeyerfwereade__: Okay, and you feel the get-arbitrary-settings always would be simpler21:31
fwereade__niemeyer, and I'm not really sure that the somewhat reduced network access is really worth the extra code plus memory budget21:31
fwereade__niemeyer, I don't feel sure either way21:31
niemeyerfwereade__: I'm happy to go with whatever you feel would result in less code and less mental trickiness21:32
fwereade__niemeyer, but using sheer projected simplicity of code as a heuristic I tend *slightly* towards the always-get-arbitrary21:32
fwereade__niemeyer, I don;t think I want to toss the caching *yet*21:32
fwereade__niemeyer, but consider this early warning that I *might*21:32
niemeyerfwereade__: Thanks for the hinting, simplicity rocks :)21:33
fwereade__niemeyer, and I'm pleased to hear you're ok exploring the alternatives if it does start to bother me seriously ;)21:33
fwereade__niemeyer, anyway, I think I'm going to try to rework the RelationUnit tests in terms of Join, Watch and Settings21:34
niemeyerfwereade__: Superb21:35
niemeyerfwereade__: Btw,21:35
niemeyerfwereade__: While you're there,21:35
fwereade__niemeyer, if you happen to see a propose of that stuff land while you're still around, it would be higher-value than the hook queue for now :)21:35
niemeyerfwereade__: When looking at the tests I was vaguely considering how tricky it would be to break those tests down into independent tests, or even a table-styled test that used "scripting" rather than that long sequence of contiguous operations21:36
fwereade__niemeyer, I think that they will get a *lot* cleaner now that the functionality isn't all hidden in one magic type with freaky action-at-a-distance21:37
niemeyerfwereade__: I wouldn't be specially bad if we had to merge them in that long chain, because they're quite well documented (thanks) and sensible21:37
niemeyerfwereade__: Having them independent would just be a nice bonus21:38
fwereade__niemeyer, cheers :)21:38
fwereade__niemeyer, (well, action at a distance is reasonable and expected, it's the bidirectional action at a distance that makes it a little tricky to follow I think)21:39
niemeyerfwereade__: Yeah21:39
fwereade__niemeyer, btw, re the principalKey change21:40
niemeyerfwereade__: Ah, yeah?21:40
fwereade__niemeyer, the reason to have an empty principal denoting unit-is-principal it the *topology* is because topology units don't know their own keys21:41
fwereade__niemeyer, so, with *just* a topology unit, we can't immediately tell whether its principal differs from itself21:42
niemeyerfwereade__: Interestingly, it's not the only reason21:42
niemeyerfwereade__: We've debated this very recently in the context of mstate21:43
fwereade__niemeyer, ah, ok, that is the reason that made me feel it was more trouble than it was worth to change it21:43
fwereade__niemeyer,  go on21:43
niemeyerfwereade__: myKey == principalKey requires a full scan21:43
niemeyerfwereade__: principalKey == "" is indexable21:43
fwereade__niemeyer, ah, ys21:43
niemeyerfwereade__: Aram had suggested the same change you suggested in the database side, and we ended up rolling back because of that21:44
fwereade__niemeyer, I looked at mstate briefly, and it didn't seem to need any changes to work nicely as is21:45
niemeyerfwereade__: For that reason, I suggested keeping principalKey in the *Unit as "" too21:45
fwereade__niemeyer, ok, that SGTM21:45
niemeyerfwereade__: Simply because it's easier to keep in mind what to expect in the principalKey field21:45
fwereade__niemeyer, yeah, very sensible21:45
fwereade__niemeyer, ok, I'll do that now :)21:45
niemeyerfwereade__: Cheers!21:46
fwereade__morning davecheney21:55
fwereade__ok, this one is starting to piss me off:21:56
fwereade__----------------------------------------------------------------------21:56
fwereade__FAIL: watcher_test.go:44: WatcherSuite.TestContentWatcher21:56
fwereade__test 021:56
fwereade__watcher_test.go:55:21:56
fwereade__    c.Fatalf("didn't get change: %#v", test.want)21:56
fwereade__... Error: didn't get change: watcher.ContentChange{Exists:false, Version:0, Content:""}21:56
fwereade__OOPS: 3 passed, 1 FAILED21:56
fwereade__--- FAIL: TestPackage (6.43 seconds)21:56
fwereade__FAIL21:56
fwereade__I've seen it 3 times today, and it disappears like mist in the sun when I try to look for it :/21:57
* niemeyer looks at the test21:58
niemeyerfwereade__: 200ms seems on the short side21:59
fwereade__niemeyer, yeah, I suspect that's it, but it is one of those ones that bothers me because it only seems to show up in a full test run22:00
fwereade__niemeyer, on its own it seems 100% reliable22:00
niemeyerfwereade__: Full test run == JVM on ZK with data, Go runtime with data, etc22:00
niemeyerfwereade__: Sum up two GCs, plus the stars in the right location..22:00
fwereade__niemeyer, yeah, indeed22:00
niemeyerfwereade__: It's the first test, interestingly22:01
fwereade__niemeyer, I just like to imagine that if I run it alone often enough the stars should eventually align right22:01
niemeyerfwereade__: I'll have a run through the code just in case22:01
fwereade__niemeyer, before you get too deep, I'm just proposing unit-principal-key again22:02
fwereade__niemeyer, https://codereview.appspot.com/642104922:03
niemeyerfwereade__: Looking22:03
fwereade__niemeyer, and if you decide the ContentWatcher test just needs a timeout bump, please assume a pre-emptive LGTM on that :)22:03
niemeyerfwereade__: LGTM!22:03
fwereade__niemeyer, cheers22:03
niemeyerfwereade__: Thanks :)22:03
niemeyerfwereade__: We should be printing the watcher Err() with those messages22:05
fwereade__niemeyer, good point22:05
niemeyerfwereade__: Looks quite straightforwardly correct on the ContentWatch side at least22:11
niemeyerfwereade__: I was just pondering about something too22:11
niemeyerfwereade__: Do you have an SSD?22:11
fwereade__niemeyer, yeah, it seemed sane to me22:11
fwereade__niemeyer, nope :)22:11
niemeyerfwereade__: Yeah22:12
niemeyerfwereade__: That may be it22:12
niemeyerfwereade__: We'll get the "All good!" from ZK before it's actually ready to take action22:12
fwereade__niemeyer, ahhhh, I did not know that22:12
fwereade__thanks zookeeper :/22:12
niemeyerfwereade__: The fact it's the first test seems to point in that direction22:12
fwereade__niemeyer, I have never seen it on any test other than the first22:13
niemeyerfwereade__: If zk was on the way to attending your request, the After() ticker would already be running22:13
niemeyerfwereade__: It also explains why repeating rarely catches the bug22:13
niemeyerfwereade__: At this point it's all cached22:14
fwereade__niemeyer, yeah, sounds pretty plausible to me22:14
fwereade__niemeyer, timeout it is then :)22:14
niemeyerfwereade__: I bet that if you flush disk and memory buffers, you'll be able to repeat it easily22:14
niemeyerTry this: sync; echo 3 | sudo tee /proc/sys/vm/drop_caches22:16
niemeyerfwereade__: and then run the tests again22:16
niemeyerfwereade__: Btw, do run sync first :)22:19
niemeyerfwereade__: This isn't entirely safe22:19
fwereade__niemeyer, I was reading around it and decided I could probably live with the risks22:20
niemeyerfwereade__: LOL22:20
fwereade__niemeyer, not reproed yet, but it does seem to goose the occasional ssh and store tests into failure pretty well22:22
fwereade__niemeyer, I'm comfortable bumping the timeout and seeing whether it happens again :)22:23
niemeyerfwereade__: Sounds good :)22:23
niemeyerAll those "ok" in go test ./... feel so great :-)22:30
niemeyerfwereade__: Are you up for a quick review?22:30
fwereade__niemeyer, sure22:30
niemeyerfwereade__: You already reviewed most of it before, actually.. I'm just reviving it22:30
fwereade__niemeyer, even better ;)22:31
niemeyerlbox propose churning22:32
niemeyerAaaaaaand22:33
niemeyerfwereade__: https://codereview.appspot.com/642306222:34
* fwereade__ looks22:34
fwereade__niemeyer, LGTM22:45
niemeyerfwereade__: Cheers!22:45
niemeyerfwereade__: Will add default handling now22:45
davecheneyniemeyer: thanks for your review22:47
niemeyerdavecheney: np!22:47
niemeyerdavecheney: Morning!22:47
davecheneyi will have to repropose it as i've back ported from my next branch22:48
davecheney(as it addressed some of the things you raised)22:48
davecheneybut the change is quite large22:48
davecheneywell, more than a LGTM will cover22:48
niemeyerdavecheney: I'm happy for this branch to go in as-is and these changes be done in the follow up22:53
davecheneyniemeyer: i'd like to spend a little more time on it22:54
davecheneyobtained string = "error: json: unsupported type: map[int]interface {}\n"22:54
davecheneyto make sure it's a good skelton to build upon22:54
niemeyerdavecheney: Aha, ok22:54
davecheneyniemeyer: json doesn't support map[int]interface{}, keys must be strings ... :(23:12
niemeyerdavecheney: Indeed23:12
niemeyerdavecheney: Do we trust that?23:12
niemeyers/trust/depend on/23:12
davecheneyniemeyer: possibly23:12
niemeyerdavecheney: We shouldn't23:12
niemeyerdavecheney: The json spec itself forbids anything else but strings23:13
niemeyerdavecheney: So if we're doing that, it's fine to fix it23:13
davecheneywonderful23:13
davecheneyas long as yaml formats a string key without quotes, this will be fine23:14
davecheneyniemeyer: would you recommend I do with structs to describe the status data, or nested maps ?23:20
davecheney/s/do/go23:21
niemeyerdavecheney: I'd personally prefer structs, unless the dynamism of fields being present or not turns out to make it more messy than anticipated23:21
davecheneyi'm hoping ,omitempty will get us most of the way23:22
niemeyerdavecheney: The struct gives a very nice descriptive view in one location of what the map is composed of23:22
davecheneyalso, for test stability, structs are probably necessary23:22
davecheneyhowever, the names like Relation, Result are already declared in the cmd pkg's namespace23:23
niemeyerdavecheney: Indeed. Might be worth having a nice prefix on all of hem23:25
niemeyerthem23:25
niemeyerDinner.. biab23:25
davecheneykk23:25
davecheneyniemeyer: structs won't work, the yaml output expects the output of each machine to be a map23:39
davecheneyso machines => map[int]map[string]string23:39
niemeyerdavecheney: Oops23:44
niemeyerdavecheney: Ok23:45
davecheneyit's not the end of the world23:45
davecheneyniemeyer: some things can be structs, like Service, Unit, Relation, but Machine must be a map to match the python output23:46
niemeyerdavecheney: Hmm23:47
niemeyerdavecheney: It can be a map like map[int]Foo, though23:48
davecheneyniemeyer: yes23:48
niemeyerfwereade__: ping23:52

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!