| twobottux | aujuju: How do I add and call a helper script in a Juju charm? <http://askubuntu.com/questions/149240/how-do-i-add-and-call-a-helper-script-in-a-juju-charm> | 03:04 |
|---|---|---|
| rogpeppe | davecheney, fwereade_, hazmat: mornin' all | 06:20 |
| TheMue | Morning. | 06:22 |
| davecheney | mornig! | 06:23 |
| rogpeppe | TheMue: hiya | 06:27 |
| TheMue | rogpeppe: Are you an early bird? Or do you have your notebook beside your breakfast? ;) | 06:28 |
| rogpeppe | TheMue: i'm not historically an early bird, but these days i do seem to be waking up earlier. | 06:28 |
| rogpeppe | TheMue: keen to get on with all the stuff that's been rumbling round my head all weekend without access to a computer... | 06:30 |
| TheMue | rogpeppe: No access? You've been on tour? Or did Carmen shut it away? | 06:30 |
| rogpeppe | TheMue: it's boring for Carmen if i spend the weekend hacking :-) | 06:31 |
| TheMue | rogpeppe: Understandable. | 06:31 |
| davecheney | crap, how did I suddenly get signed up for the intel team spam | 06:36 |
| davecheney | OOPS: 19 passed, 1 FAILED, sooo close | 06:42 |
| davecheney | rogpeppe: if I call dummy.StopInstances(nil), should it emit OpStopInstances ? | 06:43 |
| rogpeppe | davecheney: i guess so | 06:43 |
| davecheney | but passing 0 instances to stop, stops nothing | 06:44 |
| rogpeppe | davecheney: is it not doing so? | 06:44 |
| rogpeppe | davecheney: but the emit is about the StopInstances action; it's not OpStopInstance. | 06:44 |
| rogpeppe | davecheney: i can see an argument for changing that, but that's how it is currently | 06:44 |
| davecheney | so it's saying I used that method, not that it did anything | 06:44 |
| rogpeppe | davecheney: yeah | 06:45 |
| rogpeppe | davecheney: i think. | 06:45 |
| rogpeppe | davecheney: let me have a looki. | 06:45 |
| davecheney | rogpeppe: i already knwo the answer :) | 06:45 |
| rogpeppe | davecheney: yes, the idea is that you want to know if the method has been called, even if it, for instance, returns an error. | 06:46 |
| davecheney | how could it return an error if you pass no instances to stop | 06:46 |
| davecheney | arguably that is a NOOP | 06:46 |
| rogpeppe | davecheney: dummy is recording method calls, not actions. | 06:47 |
| davecheney | rogpeppe: nm I stuck a comment in to explain, we can haggle about it in the review | 06:48 |
| rogpeppe | davecheney: sure | 06:48 |
| davecheney | FAIL: provisioning_test.go:288: ProvisioningSuite.TestProvisioningRemovesUnknownMachineAfterRestart | 06:48 |
| davecheney | close now | 06:48 |
| davecheney | man, that multiple PA's running inside the test process had me going spare this morniing | 06:49 |
| davecheney | also, i'm not sure that defer c.Assert(p.Stop, IsNil) is safe | 06:49 |
| davecheney | TheMue: does gocheck have a way of running a single test ? | 06:52 |
| rogpeppe | davecheney: go test -gocheck.f regexp | 06:55 |
| rogpeppe | (f for "filter") | 06:55 |
| davecheney | nice | 06:55 |
| rogpeppe | TheMue: a review for you: https://codereview.appspot.com/6307072 | 06:55 |
| TheMue | rogpeppe: Thx | 06:56 |
| rogpeppe | davecheney: and one for you: https://codereview.appspot.com/6305063 | 06:59 |
| davecheney | rogpeppe: why thank you good sir | 06:59 |
| TheMue | rogpeppe: And a LGTM back. | 07:00 |
| rogpeppe | TheMue: thanks | 07:00 |
| fwereade_ | morning all | 07:02 |
| TheMue | fwereade_: Moin. | 07:03 |
| rogpeppe | fwereade_: yo! | 07:26 |
| rogpeppe | davecheney: there's no point in doing defer c.Assert(p.Stop, IsNil) | 07:30 |
| davecheney | rogpeppe: not a lot | 07:30 |
| rogpeppe | davecheney: the result will always be the same as not deferring it | 07:30 |
| davecheney | rogpeppe: won't the Assert failure be recorded somewhere ? | 07:30 |
| rogpeppe | davecheney: because p.Stop is evaluated when you make the defer, not when the defer runs | 07:30 |
| rogpeppe | davecheney: so why bother delaying the inevitable? | 07:31 |
| davecheney | o_o! | 07:31 |
| rogpeppe | davecheney: you might wanna do defer func() {c.Assert(p.Stop, IsNil}() though :-) | 07:32 |
| davecheney | of course! | 07:32 |
| davecheney | anyway, that is enough for me, i'm going downstairs to finish off the last tallboy in the fridge | 07:35 |
| fwereade_ | TheMue, did you have any comments on my review of https://codereview.appspot.com/6305067/ -- I suspect I may be missing something | 08:03 |
| TheMue | fwereade_: The idea behind the issue has just been that in the first draft some checks are made after (!) the first node is created. Now all tests are made before so that we have no inconsistent state in ZK. | 08:05 |
| TheMue | fwereade_: https://bugs.launchpad.net/juju-core/+bug/1007373 | 08:10 |
| fwereade_ | TheMue, understood: my contention is that the inputs to that function can only sanely come from service/charm state, and that we should be able to assume that if they don't contain real values we would have errored out already | 08:11 |
| fwereade_ | TheMue, it may be that this slapdash cowboyish thinking of the worst sort | 08:11 |
| TheMue | fwereade_: We allways should be able that a caller only uses valid values as arguments of a function. But practice show's we can't. So in case of creating node through atomic functions (sadly no transaction for the whole change) we should check. | 08:15 |
| fwereade_ | TheMue, rogpeppe: the stuff of yours that I know of LGTM | 08:23 |
| TheMue | fwereade_: Cheers. | 08:23 |
| rogpeppe | fwereade_: thanks | 08:23 |
| rogpeppe | fwereade_: am just working on tests for the upgrade stuff, and hoping that gustavo thinks it's ok... | 08:24 |
| fwereade_ | rogpeppe, I know the feeling :) | 08:24 |
| TheMue | fwereade_: Btw, most of my little change is already covered in Rogers proposal. | 08:24 |
| rogpeppe | TheMue: which proposal? | 08:24 |
| rogpeppe | TheMue: which change? | 08:25 |
| TheMue | rogpeppe: My verification of endpoints in AddRelation() before doing any change in ZK. | 08:25 |
| rogpeppe | TheMue: oh, i didn't think i had any CL about relation endpoints | 08:26 |
| TheMue | rogpeppe: No, but regarding relations in https://codereview.appspot.com/6303060/ | 08:26 |
| rogpeppe | TheMue: that's fwereade_'s proposal | 08:27 |
| TheMue | rogpeppe: Ooops, yep, just seen. :) | 08:28 |
| rogpeppe | TheMue: np! | 08:28 |
| Aram | morning. | 10:04 |
| TheMue | Aram: Moin. | 10:08 |
| rogpeppe | Aram: yo! | 10:23 |
| rogpeppe | fwereade_: just having a look at https://codereview.appspot.com/6305070 | 10:27 |
| rogpeppe | fwereade_: i can't quite see what VersionWatcher gives us that ContentWatcher doesn't already have | 10:27 |
| rogpeppe | fwereade_: doesn't a ContentWatcher | 10:30 |
| rogpeppe | delivers a notification | 10:30 |
| rogpeppe | 95 // whenever the note is created, written to, or deleted. | 10:30 |
| rogpeppe | ? | 10:30 |
| rogpeppe | oops, intended to edit before pasting :-) | 10:30 |
| fwereade_ | rogpeppe, it's more what it doesn't do | 10:35 |
| rogpeppe | fwereade_: ah, what doesn't it do? | 10:36 |
| fwereade_ | rogpeppe, I don't think there's any need to read and parse the settings node when the actual data is going to be packed up and dumped in a queue somewhere | 10:36 |
| fwereade_ | rogpeppe, sorry, "actual data" == "only thing we care about, being the fact of a change" | 10:37 |
| * fwereade_ suddenly changes his mind, maybe | 10:37 | |
| fwereade_ | rogpeppe, ok, the issue is this | 10:37 |
| rogpeppe | fwereade_: ah, i see. | 10:37 |
| Aram | eh, I see that I can't do a plain 'bzr pull' in bzr, do I have to specify branch name all time? | 10:38 |
| fwereade_ | Aram, --remember | 10:38 |
| fwereade_ | rogpeppe, I think there is a case to be made that we *do* care about the exact settings of a node, and want to persist them, so that when we execute foo-relation-changed we already have access to the remote unit's settings data *at the tinme of the change event*, rather than some time after | 10:39 |
| fwereade_ | rogpeppe, ie when the hook happens to execute | 10:39 |
| rogpeppe | fwereade_: aren't we *always* seeing data from some time after? | 10:40 |
| rogpeppe | fwereade_: due to the inherent nature of networking | 10:40 |
| fwereade_ | rogpeppe, yeah, but we're not waiting anarbitrary amount of time and then casually getting settings, just assuming the node is still there | 10:41 |
| Aram | fwereade_: thanks | 10:41 |
| fwereade_ | rogpeppe, which is what we do in python | 10:41 |
| fwereade_ | rogpeppe, and which bugs me somewhat | 10:41 |
| rogpeppe | fwereade_: this seems to me like an argument *for* using ContentWatcher | 10:42 |
| fwereade_ | rogpeppe, hence the * fwereade_ suddenly changes his mind, maybe | 10:42 |
| rogpeppe | fwereade_: but the problem is that we throw away the content we just read | 10:42 |
| rogpeppe | fwereade_: because all the State methods go back to the db rather than using the data we just read | 10:42 |
| rogpeppe | fwereade_: (that's probably not true of settings nodes though) | 10:43 |
| fwereade_ | rogpeppe, just keeping the content around as a string is all we need | 10:43 |
| fwereade_ | rogpeppe, we can load it and use it when we come to actually execute a hook | 10:44 |
| rogpeppe | fwereade_: doesn't a ConfigNode keep the original data around? or are you interested in other kinds of node? | 10:45 |
| fwereade_ | rogpeppe, I'm interested specifically in presenting to a hook an environment that reflects the state that caused the hook to be triggered, rather than an arbitrary unrelated state | 10:46 |
| fwereade_ | rogpeppe, but then I may be on crack here | 10:46 |
| rogpeppe | fwereade_: no, that sounds very reasonable to me. | 10:46 |
| rogpeppe | fwereade_: and i've been wittering on about something similar | 10:47 |
| rogpeppe | fwereade_: (last time i think you said you found the old model ok to work with) | 10:47 |
| fwereade_ | rogpeppe, we do already keep track of the "current" members of the relation; I presume we'd actually really also want to keep track of the "current" settings for all members | 10:48 |
| fwereade_ | rogpeppe, well, it was ok to work with, but only by coincidence, because we never cleaned up after ourselves and never had nodes disappear on us | 10:49 |
| rogpeppe | fwereade_: could you briefly explain to me how relation members are stored in zk? | 10:49 |
| fwereade_ | rogpeppe, ok, peer example feels easiest | 10:49 |
| fwereade_ | rogpeppe, /relations/relation-XXXXX has 2 children | 10:50 |
| fwereade_ | rogpeppe, "settings" and "peer" | 10:50 |
| rogpeppe | fwereade_: oh, i thought that "peer" was an attribute rather than a key | 10:51 |
| fwereade_ | rogpeppe, each of those contain config nodes and presence nodes respectively, each keyed on the unit id responsible for maintaining those noes | 10:51 |
| fwereade_ | rogpeppe, in a pro/req case, it'd have 3 children: "provides", "requires", "settings" | 10:51 |
| fwereade_ | rogpeppe, containing the same data, but with the presence nodes distributed amongs provides and requires depending on which end of the relation they were on | 10:52 |
| fwereade_ | rogpeppe, but in general the /path/to/<role> and /path/to/settings structure is the important one | 10:53 |
| rogpeppe | fwereade_: ok | 10:54 |
| fwereade_ | rogpeppe, there is variation in the /path/to container-scoped relations -- this structure is duplicated for every affected primary unit in the relation | 10:54 |
| fwereade_ | rogpeppe, so in practice sometimes /path/to is /relations/relation-XXXX and sometimes it's /relations/relation-XXXX/unit-YYY-ZZZ | 10:55 |
| rogpeppe | fwereade_: what does the "settings" node contain? | 10:55 |
| fwereade_ | rogpeppe, the unit's private address, at least; but in general anything that is meant to be communicated over that relation, AIUI | 10:56 |
| fwereade_ | rogpeppe, the settings nodes are the conduits by which information flows across a relation | 10:56 |
| rogpeppe | fwereade_: oh, i'm not sure i understand the distinction between "settings" and "<role>" | 10:56 |
| fwereade_ | rogpeppe, the children of the <role> node are presence nodes | 10:57 |
| fwereade_ | rogpeppe, when a unit agent joins a relation, it should set its own role node in that relation to alive | 10:57 |
| fwereade_ | rogpeppe, when the role node is alive, we watch for settings changes, and stop when the role node is no longer alive | 10:58 |
| rogpeppe | fwereade_: all this is as it is in the python? | 10:58 |
| fwereade_ | rogpeppe, the actual code is very different but I think I have translated what actually happens with reasonable fidelity | 10:58 |
| rogpeppe | fwereade_: the zk structure, i meant | 10:59 |
| fwereade_ | rogpeppe, yes | 10:59 |
| rogpeppe | fwereade_: so for relationship between two services each with one unit, the structure might look like this: http://paste.ubuntu.com/1035291/ | 11:02 |
| rogpeppe | ? | 11:02 |
| fwereade_ | rogpeppe, yep | 11:02 |
| rogpeppe | fwereade_: ok, cool, that's useful thanks. | 11:03 |
| fwereade_ | rogpeppe, ok, so, I shall dump version-watcher; unpropose unit-relation-watcher, and rework it to send settings content rather than version; and repropose without reqs | 11:03 |
| fwereade_ | rogpeppe, sound good? | 11:03 |
| rogpeppe | fwereade_: it's perhaps interesting that presence *could* be conveyed through the config node too, presumably. | 11:03 |
| fwereade_ | rogpeppe, it probably could but I'm not entirely sure it should ;) | 11:04 |
| fwereade_ | rogpeppe, another question | 11:04 |
| fwereade_ | rogpeppe, ChildrenWatcher errors out when the watched node doesn't exist | 11:04 |
| * rogpeppe goes to look at unit-relation-watcher again | 11:04 | |
| fwereade_ | rogpeppe, this feels to me inconsistent with other watchers | 11:05 |
| rogpeppe | fwereade_: are you actually using ChildrenWatcher? | 11:05 |
| fwereade_ | rogpeppe, in an as-yet-unprposed branch, yes | 11:05 |
| rogpeppe | fwereade_: to watch what? | 11:06 |
| fwereade_ | rogpeppe, related units | 11:06 |
| rogpeppe | fwereade_: that info isn't held in the topology? | 11:06 |
| fwereade_ | rogpeppe, not the current set of connected units, which updates as agents respond to topology changes, no | 11:06 |
| fwereade_ | rogpeppe, the topology is saying "this unit should be in this relation; come on, unit agent, get involved" | 11:07 |
| rogpeppe | fwereade_: ah, and then the unit agent creates the nodes under /relations/relation-xxxx ? | 11:08 |
| fwereade_ | rogpeppe, the presence node is saying "I, the unit agent, am actively participating in this relation" | 11:08 |
| fwereade_ | rogpeppe, and the units on the other side need t know what agents are actually around, not what units we'd like to be around | 11:08 |
| rogpeppe | fwereade_: so... in this case, you are guaranteed that the parent node does exist, right? | 11:09 |
| fwereade_ | rogpeppe, the way I've written it I'm not | 11:09 |
| rogpeppe | fwereade_: because /relations/relation-0000/{provides,requires,settings} is created when the relation is created, no? | 11:09 |
| fwereade_ | rogpeppe, it could be but I don;t see a good reason to do so | 11:10 |
| rogpeppe | fwereade_: why *wouldn't* you create those node immediately? | 11:10 |
| rogpeppe | s/node/nodes/ | 11:10 |
| fwereade_ | rogpeppe, in container-scoped relations, we create the role node as the unit joins because that's the only point at which we know we need a container-scoped role node under that key | 11:10 |
| fwereade_ | rogpeppe, it seems neater to me to *always* create the role node just as the unit joins | 11:11 |
| fwereade_ | rogpeppe, just because it doesn't involve smearing the responsibility for it into two rather separate places | 11:11 |
| fwereade_ | rogpeppe, so, then, *if* ChildrenWatcher were to treat "node doesn't exist" as "no children, it's cool, I'll wait" rather "OMGBBQWTF", I could use it very nicely | 11:12 |
| rogpeppe | fwereade_: BTW why do we care about the presence of the unit at the other end of a relation, rather than just its relation settings? | 11:12 |
| fwereade_ | rogpeppe, because a bunch of settings set 3 hours ago on a machine that is now molten slag are of limited value in determining active participation (ie the ability to respond to changes) | 11:13 |
| rogpeppe | fwereade_: sorry, what does "responding to changes" mean in this context? | 11:14 |
| fwereade_ | rogpeppe, seeing that there's a new unit on the other end of the relation and updating its own settings to match | 11:14 |
| rogpeppe | fwereade_: still at sea. "updating its own settings to match" ? | 11:15 |
| rogpeppe | fwereade_: apologies for my lack of knowledge in this area! | 11:15 |
| fwereade_ | rogpeppe, I am hadoop-master and I just saw a hadoop-slave come online; I consequently perform whatever hadoopy magic is necessary to start me distributing jobs to that slave in addition to all the others | 11:16 |
| fwereade_ | rogpeppe, alternatively, I am hadoop-slave and I just saw a hadoop-master appear; I'll set some stuff in my settings so he knows about me and starts sending me jobs | 11:17 |
| rogpeppe | fwereade_: ah, so if the presence node isn't active, we assume that the unit has left? | 11:17 |
| fwereade_ | rogpeppe, exactly | 11:17 |
| rogpeppe | fwereade_: but its settings remain around? | 11:18 |
| fwereade_ | rogpeppe, the way I see it, the validity of a settings node is contingent on an active presence node | 11:18 |
| fwereade_ | rogpeppe, it should be the UA's responsibility to create its settings node before becoming active | 11:19 |
| rogpeppe | fwereade_: but if the unit comes back online, its settings will be as before? | 11:19 |
| fwereade_ | rogpeppe, ^^ or update its settings node -- whatever | 11:19 |
| rogpeppe | fwereade_: for instance, if there was a network outage and then the node gets reconnected | 11:19 |
| fwereade_ | rogpeppe, it's the UA's resposibility to provide sane settings if it's going to declare itself active | 11:20 |
| rogpeppe | fwereade_: ok | 11:20 |
| rogpeppe | fwereade_: another question: why store the presence nodes under <role> ? | 11:20 |
| fwereade_ | rogpeppe, so in that case for example, the UA should surely always update the settings node's private-address when it comes online | 11:20 |
| rogpeppe | fwereade_: why not just have a single "presence" directory containing presence nodes for all units participating in that relation? | 11:21 |
| fwereade_ | rogpeppe, so that provider units can watch the children of the requirer node, and requirer units can watch the chidren of the provider node, without settings on the same side of the relation causing confusion | 11:21 |
| fwereade_ | rogpeppe, with peer relations it works how you describe, but it filters out just its own changes | 11:21 |
| rogpeppe | fwereade_: ah, i understand | 11:22 |
| fwereade_ | rogpeppe, it's really quite neat once you appreciate the details :) | 11:22 |
| fwereade_ | rogpeppe, although honestly I had a couple of aha! moments only just this weekend | 11:23 |
| fwereade_ | rogpeppe, seeing what was really going on through the thick fog of twisted was somewhat challenging ;) | 11:23 |
| rogpeppe | fwereade_: for subordinate relations what name is used for the presence node directory? | 11:23 |
| fwereade_ | rogpeppe, subordinate relations use the unit key of the principal to store the unit relations that exist within that principal unit | 11:24 |
| rogpeppe | fwereade_: hazmat suggested that we should copy some of the internal docs to juju-core, and i think i agree. | 11:24 |
| fwereade_ | rogpeppe, yeah, that makes a lot of sense | 11:24 |
| rogpeppe | fwereade_: example full directory name? | 11:25 |
| rogpeppe | fwereade_: is it stored under /units? | 11:25 |
| fwereade_ | rogpeppe, /relations/relation-X/unit-A-B/settings/unit-A-B | 11:26 |
| fwereade_ | rogpeppe, /relations/relation-X/unit-A-B/settings/unit-C-D | 11:26 |
| rogpeppe | fwereade_: ah, so the unit key takes the place of <role?> | 11:26 |
| rogpeppe | s/\?// | 11:26 |
| fwereade_ | rogpeppe, in which unit-A-B is a unit of the principal service, which has its own container | 11:26 |
| fwereade_ | rogpeppe, no | 11:26 |
| fwereade_ | rogpeppe, /relations/relation-X/unit-A-B/provider/unit-A-B | 11:27 |
| rogpeppe | oh i see | 11:27 |
| rogpeppe | gotcha | 11:27 |
| fwereade_ | rogpeppe, /relations/relation-X/unit-A-B/requirer/unit-C-D | 11:27 |
| fwereade_ | rogpeppe, ...and unit-C-D is the unit of the subordinate service which shares unit-A-B's container | 11:27 |
| rogpeppe | fwereade_: this has been *really* useful BTW. i only had a very very handwavy idea of what was going on before. | 11:27 |
| fwereade_ | rogpeppe, it's firmed up my own confidence a lot, there's nothing like someone asking questions to test whether you really know something ;) | 11:28 |
| fwereade_ | rogpeppe, so... how would you feel about me making that change to ChildrenWatcher? IMO it's consistent with stuff like ContentWatcher returning {false, ""} for a node that doesn't exist | 11:29 |
| rogpeppe | fwereade_: i'm just thinking about it... | 11:29 |
| fwereade_ | rogpeppe, and it makes it a lot more useful to me | 11:29 |
| rogpeppe | fwereade_: given that this is the first genuine use case for ChildrenWatcher, i'd tend towards +1 | 11:32 |
| fwereade_ | rogpeppe, the other use case is /machines, I think | 11:33 |
| fwereade_ | rogpeppe, but it doesn't affect that case | 11:33 |
| rogpeppe | fwereade_: i don't *think* so. i think that the provisioning agent watches the topology, not /machines | 11:34 |
| fwereade_ | rogpeppe, goodness me, you're absolutely right | 11:34 |
| rogpeppe | fwereade_: but as you say, it doesn't affect that case even so | 11:34 |
| fwereade_ | rogpeppe, cool, thanks | 11:35 |
| rogpeppe | np | 11:35 |
| Aram | hi niemeyer | 12:47 |
| niemeyer | Hello there! | 12:47 |
| niemeyer | Aram: Hey man | 12:47 |
| rogpeppe | niemeyer: yo! | 12:48 |
| rogpeppe | niemeyer: welcome back. | 12:48 |
| niemeyer | rogpeppe: Heya | 12:48 |
| niemeyer | Thanks | 12:48 |
| niemeyer | It was a great break indeed | 12:48 |
| niemeyer | How're you guys doing? | 12:48 |
| * Aram is drinking his tea. | 12:49 | |
| rogpeppe | niemeyer: all good here. in particular i'm eagerly awaiting your reaction to my upgrade proposal | 12:49 |
| niemeyer | Aram: Me too, kin dof | 12:49 |
| niemeyer | Aram: Chimarrão, which is a tea, but we don't call it as such :) | 12:49 |
| Aram | heh. | 12:49 |
| niemeyer | rogpeppe: How did it change? | 12:50 |
| rogpeppe | niemeyer: did you see my post on juju-dev? | 12:50 |
| niemeyer | rogpeppe: I haven't read it yet | 12:50 |
| niemeyer | rogpeppe: But I was expecting to see it close to what we talked | 12:50 |
| rogpeppe | niemeyer: well, yeah, it's changed, but in a good way, i think. i decided that what we'd arrived at wasn't sufficient. | 12:50 |
| niemeyer | (s/was/am/) | 12:50 |
| niemeyer | rogpeppe: Ok, will read it | 12:51 |
| rogpeppe | niemeyer: it's not good to just exit and let upstart deal with that. | 12:51 |
| niemeyer | But first let me warm up the chair :) | 12:51 |
| rogpeppe | niemeyer: :-) | 12:51 |
| niemeyer | rogpeppe: How so? | 12:51 |
| niemeyer | Well, you probably explained it in the mail | 12:51 |
| rogpeppe | niemeyer: yeah, hopefully sufficiently... | 12:51 |
| Aram | niemeyer: a simple branch for you to review: https://codereview.appspot.com/6298062/ I have some more in the pipe, but everything depends on this. | 12:55 |
| niemeyer | Aram: Looking | 12:55 |
| hazmat | niemeyer, g'morning | 12:57 |
| niemeyer | hazmat: Heya | 12:59 |
| niemeyer | Aram: Going through the description | 12:59 |
| niemeyer | Aram: Can we talk a bit about which issues you've found that required the several attempts to be done | 13:00 |
| Aram | well yes, sure, I did 1, 2, and 3, 4 was out of the question because preserving Path uniqueness was too important. | 13:02 |
| Aram | usually the most direct way of doing these things is 3, make each node remember its children. | 13:02 |
| Aram | but this is not too great. | 13:02 |
| niemeyer | Aram: Where's the dot int hat sentence? : | 13:03 |
| niemeyer | :) | 13:03 |
| niemeyer | Aram: I guess you did (1, 2, and 3)? | 13:03 |
| Aram | yes | 13:03 |
| niemeyer | Ok | 13:03 |
| niemeyer | Aram: So what were the issues you've found with 3? | 13:04 |
| Aram | You need to update parent when you create/delete a child, this is not optimal. | 13:04 |
| niemeyer | Aram: What were the issues you've found with that? | 13:06 |
| Aram | it can be unreliable and makes the code significantly more complex. in my experience with namespace-like things, it is usually a design error for a high level call to generate multiple independent exported operations. having a 1:1 API-atomic operation model is useful and easier to debug and understand. | 13:07 |
| niemeyer | Aram: Okay, but I'd appreciate understanding more about the decision. You're saying it's unreliable and complex, and breaks a property that you like, but why? | 13:08 |
| niemeyer | Why is it unreliable and complex, and why do we need such a property | 13:08 |
| Aram | it has more points of failure, e.g. it can create the node but fail to update its parent and the code to deal with such failures is complex. | 13:09 |
| niemeyer | Aram: Ok, agreed | 13:10 |
| niemeyer | Aram: Why do we need a parent node within a child? | 13:10 |
| niemeyer | Erm.. s/node/field/ | 13:10 |
| Aram | so we can query a node for its children efficiently. if we have a node 'path' and we want its children we query the DB for nodes with parent 'path'. it's very fast. | 13:14 |
| Aram | if we don't have a parent field it's more complex, in /a/b/c a query for ^/a yields both b and b/c, and we want only b, so we need to filter direct descendants client side. | 13:14 |
| Aram | we could construct a more complex regexp, but it would not use the index, so it would be slow to use. | 13:15 |
| Aram | this slight bit of redundancy was the only way I could come with so that we could use the index at all times. | 13:15 |
| niemeyer | Aram: We'd use the index at all times anyway. What you're avoiding is the cost of excluding entries at either side | 13:17 |
| Aram | yes. | 13:17 |
| niemeyer | Aram: Probably not worth it | 13:17 |
| rogpeppe | presumably for getting children of /, the query would return all entries in the tree | 13:17 |
| Aram | yes | 13:18 |
| niemeyer | Hmm | 13:19 |
| rogpeppe | but for our use case, perhaps we don't care too much, as we only do GetChildren on shallow nodes. | 13:19 |
| niemeyer | Fair enough.. it'd suck to have to walk through the entire tree for these cases.. | 13:20 |
| niemeyer | A bit sad that we'll need an extra index, though | 13:20 |
| niemeyer | Aram: Did you put some thinking onto what we briefly talked about at London? | 13:21 |
| Aram | the watchers thing? yes, working on it. | 13:21 |
| niemeyer | Aram: Regarding the fact a parent doesn't have a well-defined number of children for a given version | 13:22 |
| Aram | ah, that one, sure. it's in the pipe. | 13:22 |
| Aram | I decided to Create to behave like mkdir -p | 13:22 |
| niemeyer | Aram: Well, I don't know if we care yet.. I just would like to understand whether we do or not, and what we're ignoring | 13:22 |
| niemeyer | Aram: in which sense? | 13:23 |
| Aram | to create all necessary nodes. e.g create /a/b/c creates /a and /a/b if required.\ | 13:23 |
| Aram | I'm not sure if we care that much, but ATM we always return a stat of the parent when querying for children. | 13:24 |
| Aram | so it's easy if the parent really exists. | 13:24 |
| niemeyer | Aram: It's a bit more involved than that | 13:25 |
| niemeyer | Aram: In the design we're leaning towards, the same parent version may have a different number of children | 13:25 |
| niemeyer | Aram: Which I don't think can happen on real zk | 13:25 |
| niemeyer | Aram: Maybe we don't care about that, but that should be a conscious decision if nothing else | 13:26 |
| rogpeppe | i think having Create work like mkdir -p is quite a big semantic change | 13:26 |
| Aram | well, if we decide that old parents are immutable in the sense that only new versions may aquire new children, it would be a simple thing to arrange for. I'm not sure if we care about that, but I could look at how we use ZK today. | 13:28 |
| Aram | rogpeppe: yes, it's quite a big semantic change, but I think it's for the better, it should make things easier, not harder. | 13:29 |
| niemeyer | Aram: Thanks, the key is really to understand which path we're going down towards.. not suggesting we should implement the exact semantics at this point | 13:30 |
| rogpeppe | i'm trying to remember the awkward case i came up with before that meant that it wasn't great to make directories implicitly | 13:31 |
| * rogpeppe fails | 13:35 | |
| niemeyer | Aram: Review in | 13:37 |
| Aram | thanks | 13:39 |
| Aram | reading | 13:39 |
| Aram | niemeyer: what race? (probably I have a wrong assumption about mongodb). | 13:43 |
| niemeyer | Aram: Duh, sorry, the comment went into the wrong location | 13:46 |
| niemeyer | Aram: The race I was alluding to is the creation of nodes without parents | 13:46 |
| Aram | niemeyer: I am aware of that race, don't have a good idea on how to solve it yet, have a few ideas though. at the moment you can create nodes without parents, but you can't query the non-existent parents for children. | 13:57 |
| Aram | that means you don't get stale data, but an error, which is probably better. | 13:57 |
| niemeyer | Aram: Both sound bad | 13:58 |
| niemeyer | Aram: and they're both related to how we store children information.. we shouldn't dive in onto a path before we understand how we're handling this | 13:58 |
| hazmat | rogpeppe, adding ml to our upgrade discussion, forgot to add it on the initial reply | 14:14 |
| rogpeppe | hazmat: oh good catch, i hadn't noticed that | 14:15 |
| hazmat | rogpeppe, apparently i fubar'd the second time two. third times the charm | 14:20 |
| TheMue | Hmm, could it bee that proposals for juju-core don't lead to a notification in IRC? | 14:21 |
| niemeyer | Aram: Any ideas around that? | 14:23 |
| Aram | niemeyer: yes, thinking, will email, probably better than IRC for explaining :). | 14:23 |
| niemeyer | Aram: Sounds good, although in some cases IRC works better | 14:23 |
| Aram | sure | 14:23 |
| mramm | So, I'd like to bring up something for the team to discuss | 15:02 |
| * fwereade_ listens | 15:03 | |
| * TheMue listens too | 15:03 | |
| mramm | currently we are making good progress | 15:03 |
| mramm | but it's hard to explain what that progress is or how far along we are to folks outside of the team | 15:03 |
| mramm | and I would say a little bit hard for the team itself to know what exactly needs to be done before 12.10 | 15:04 |
| mramm | I don't want to create any kind of heavyweight process, but I think it would be helpful to have a bit more status information | 15:05 |
| mramm | it was suggested to me that we use blueprints and a list of work items for each blueprint to track what needs doing | 15:05 |
| mramm | I think that if we keep it at that kind of high level, we should not have to spend much time on it | 15:05 |
| rogpeppe | niemeyer: ^ | 15:06 |
| niemeyer | mramm: This sounds like a nice topic for the mailing list | 15:06 |
| niemeyer | mramm: We have members of the team that are not here, and people are not necessarily listening right now | 15:06 |
| mramm | https://blueprints.launchpad.net/ubuntu/+spec/servercloud-q-arm-deployment would be an example (look at the work items) | 15:06 |
| mramm | niemeyer: yea | 15:07 |
| niemeyer | mramm: For example, aren't you supposing to be listening to that bad music waiting for a conference call with mark right now? :-) | 15:07 |
| niemeyer | robbiew: Is that meeting happening? | 15:07 |
| robbiew | niemeyer: man...I suspect not | 15:08 |
| robbiew | it is 11:08pm in Taipei | 15:08 |
| robbiew | this music is SO awesome though :/ | 15:08 |
| niemeyer | robbiew: I don't understand why we don't have just a short beep every once in a while or some such to notify about the fact the call is still up | 15:09 |
| niemeyer | robbiew: It's so obviously counter productive to make people wait with bad music with absurd audio quality | 15:09 |
| robbiew | 10min rule...I'm dropping | 15:10 |
| niemeyer | Ditto | 15:10 |
| robbiew | mramm: fyi ^^ | 15:12 |
| robbiew | we dropped | 15:12 |
| mramm | I got in in time to drop ;) | 15:12 |
| mramm | So, my plan is not to decide what to do here on IRC | 15:12 |
| mramm | but just to raise the issue so that you all have time to think about it | 15:12 |
| hazmat | the music is getting better ;-) | 15:13 |
| mramm | If we use work items on blueprints we can get in on the burn-down chart action here: http://status.ubuntu.com/ubuntu-quantal/group/topic-quantal-servercloud-arm.html | 15:13 |
| mramm | and have it rolled up to this: http://status.ubuntu.com/ubuntu-quantal/canonical-server.html | 15:14 |
| mramm | So, anyway, food for thought. I'll raise this on the list for wider discussion | 15:15 |
| TheMue | mramm: Success, thoughts are already running. ;) | 15:20 |
| niemeyer | Lunch time.. back later folks | 15:47 |
| TheMue | niemeyer: Enjoy. | 15:47 |
| * rogpeppe is off for the evening. see y'all tomorrow. | 18:04 | |
| mramm | have a good evening! | 18:04 |
| rogpeppe | mramm: will do, thanks! | 18:04 |
| niemeyer | rogpeppe: Cheers man | 18:13 |
| twobottux | aujuju: How do I gracefully shutdown a Juju Charm? <http://askubuntu.com/questions/149550/how-do-i-gracefully-shutdown-a-juju-charm> | 18:57 |
| twobottux | aujuju: How do I make a Juju Charm's revision match the Bazaar revision of its repo? <http://askubuntu.com/questions/149553/how-do-i-make-a-juju-charms-revision-match-the-bazaar-revision-of-its-repo> | 19:09 |
| robbiew | mramm: ping | 19:20 |
| mramm | robbiew: pong | 19:22 |
| robbiew | mramm: 1:1 time? (actually about 23min ago, but I assume my private msg was missed) | 19:23 |
| mramm | robbiew: sure | 19:24 |
| niemeyer | fwereade_: ping | 19:33 |
| fwereade_ | niemeyer, pong | 19:34 |
| niemeyer | fwereade_: Heya | 19:34 |
| niemeyer | fwereade_: Looking over the unit relation watching stuff | 19:34 |
| niemeyer | fwereade_: Was just pondering about the choice of merging both watchers onto one | 19:34 |
| fwereade_ | niemeyer, cool, hopefully not too crackful... | 19:34 |
| niemeyer | fwereade_: I don't have much of a feeling, but mostly wanted to learn from you why you felt this was a good direction | 19:34 |
| niemeyer | fwereade_: It's not | 19:35 |
| fwereade_ | niemeyer, it seemed natural, I think, because the validity of the settings node is contingent on the presence node; and so synchronising th ewatching by putting them together seemed sensible | 19:36 |
| fwereade_ | niemeyer, we get a single stream of events representing everything we care about for a single relation | 19:36 |
| fwereade_ | niemeyer, single *unit* relation | 19:37 |
| niemeyer | fwereade_: Well, kind of.. there are apparently a disjoint set of methods? | 19:38 |
| niemeyer | fwereade_: E.g. settingsChange | 19:38 |
| niemeyer | fwereade_: Ah, I see.. but it's routed into a single chan | 19:38 |
| fwereade_ | niemeyer, are you on https://codereview.appspot.com/6305082/ ? | 19:39 |
| niemeyer | fwereade_: Is your idea to expose this watcher as part of the API? | 19:39 |
| fwereade_ | niemeyer, I thought I deleted the earlier proposals, I'm pretty sure settingsChange is gone | 19:39 |
| fwereade_ | niemeyer, not this one, no; there will be a RelatedUnitsWatcher returned from UnitRelation.WatchRelated | 19:39 |
| niemeyer | fwereade_: There is a single patch set in that CL | 19:39 |
| niemeyer | fwereade_: (with settingsChange in) | 19:40 |
| fwereade_ | niemeyer, it originally depended on a prereq we decided to drop | 19:40 |
| niemeyer | fwereade_: Am I reviewing the wrong thing? | 19:40 |
| fwereade_ | niemeyer, I think you might be... are you sure you're on the one I linked? | 19:41 |
| fwereade_ | niemeyer, (did I paste the wrong one in themail..?) | 19:41 |
| niemeyer | fwereade_: Have you opened the link you pasted? :-) | 19:41 |
| fwereade_ | niemeyer, yes; it seems to me to have: | 19:42 |
| fwereade_ | +// unitRelationChange describes the state of a unit relation. | 19:42 |
| fwereade_ | +type unitRelationChange struct { | 19:42 |
| fwereade_ | + Present bool | 19:42 |
| fwereade_ | + Settings string | 19:42 |
| fwereade_ | +} | 19:42 |
| niemeyer | fwereade_: 501 func (w *unitRelationWatcher) settingsChanges() <-chan watcher.ContentChange { | 19:42 |
| fwereade_ | niemeyer, ah sorry, that errant "s" had me barking up completely the wrong tree | 19:43 |
| fwereade_ | niemeyer, an earlier version had a settingsChange type | 19:43 |
| fwereade_ | sorry :) | 19:43 |
| niemeyer | fwereade_: Ah, phew, ok | 19:43 |
| niemeyer | fwereade_: So this watcher is the underlying implementation of an aggregated channel that will dispatch on all units? | 19:47 |
| fwereade_ | niemeyer, yeah, there will be a RelatedUnitsWatcher that starts one of these for each candidate related unit | 19:48 |
| niemeyer | fwereade_: Fun | 19:48 |
| fwereade_ | niemeyer, it was a fun w/e actually, I have half a dozen unpushed branches labelled things like another-abortive-ruw-attempt | 19:49 |
| niemeyer | fwereade_: LOL | 19:49 |
| fwereade_ | niemeyer, but it's definitely an awful lot simpler than the first attempts | 19:50 |
| niemeyer | fwereade_: Yeah, looks quite reasonable | 19:50 |
| niemeyer | fwereade_: Missing some docs here and there, but nothing major | 19:50 |
| fwereade_ | niemeyer, cool, the followup is a little more complex, but I hope it won't be too bad -- I'm pretty sure it's an improvement on the twisted stuff which frankly made my brain hurt | 19:52 |
| niemeyer | fwereade_: I'm having a feeling that the watcher infra on the mongo+zk will have to be optimized pretty soon | 19:52 |
| * niemeyer looks at Aram | 19:52 | |
| fwereade_ | niemeyer, I'm vaguely hoping one of you will take one look and shear off another layer of complexity ;) | 19:52 |
| niemeyer | fwereade_: Oh, I had the feeling that the follow up would actually be simpler | 19:52 |
| fwereade_ | niemeyer, if it were just children changes I had to worry about it probably would be | 19:53 |
| niemeyer | fwereade_: Isn't that a sign that the joint watching may not be helping after all? | 19:53 |
| fwereade_ | niemeyer, a lot of the hassle in in accommodating the fact that a presence node can exist but not actually indicate presence | 19:53 |
| niemeyer | fwereade_: Why is that so? | 19:54 |
| fwereade_ | niemeyer, because that's how presence nodes are designed | 19:54 |
| niemeyer | fwereade_: Sure, but that's why we have the abstraction on top of them | 19:55 |
| niemeyer | fwereade_: We have a nice on/off answer from it, right? | 19:55 |
| fwereade_ | niemeyer, yes, and the unitRelationWatcher makes it nicer still | 19:56 |
| fwereade_ | niemeyer, but | 19:56 |
| fwereade_ | niemeyer, the RelatedUnitsWatcher also needs to deal with tedious stuff like keeping the key:name mapping going, plus a bit of bookkeeping for the watchers, then some way of tracking state... | 19:56 |
| fwereade_ | niemeyer, in terms of goroutines and channels it's pretty simple really | 19:57 |
| fwereade_ | niemeyer, the devil is as always in the details | 19:57 |
| niemeyer | fwereade_: Yeah | 19:57 |
| niemeyer | fwereade_: It's quite readable to be honest | 20:03 |
| fwereade_ | niemeyer, cool :) | 20:03 |
| niemeyer | Aram: Seriously, let's talk briefly about watches when you have a moment | 20:04 |
| niemeyer | Aram: We should have a single watching goroutine per process | 20:04 |
| niemeyer | Or we'll explode the server with a ridiculous number of threads | 20:04 |
| Aram | niemeyer: hmm... one goroutine per process, hmm. this complicates things a bit. | 20:09 |
| Aram | now is a good time as every to discuss this | 20:09 |
| Aram | ever | 20:09 |
| Aram | well | 20:11 |
| Aram | hmm | 20:11 |
| niemeyer | Aram: I thought it was late for you | 20:17 |
| niemeyer | Aram: We can discuss now if you still have the energy | 20:18 |
| Aram | it's late but I think it's fine :). | 20:18 |
| niemeyer | Aram: While reviewing fwereade_'s branch, it's apparent we'll have quite a few watches per process | 20:20 |
| niemeyer | Aram: If we have a model where each watch we attempt yields a new session with a new watch, we'll end up with a massive number of threads | 20:20 |
| niemeyer | Aram: All locked down waiting | 20:21 |
| Aram | niemeyer: in doozer you can do watch(path, ver), but from what I can see in zk you can only do watch(path), if that is the case, it's easy to use a single goroutine and zk session to read the oplog. if we also care about versions, it's more complex, still doable. | 20:21 |
| niemeyer | Aram: With zk we have watch(path) => ver, in a way | 20:22 |
| niemeyer | Aram: The watch will be a delta against ver | 20:23 |
| niemeyer | Aram: Or rather, a note stating that ver has changed | 20:23 |
| Aram | niemeyer: that is fine, the design seems simple. a single goroutine always loops reading from the oplog, at each step it checks to see if that operation has a path someone is interested in, if so, makes the notification. | 20:26 |
| niemeyer | Aram: Yeah.. I don't have real world experience with this, but I suspect it should be cheap, since it's a naturally ordered collection (no index, queries, or anything else really) | 20:26 |
| Aram | yes. | 20:27 |
| niemeyer | Aram: interestingly, I suspect this will make the design a lot simpler in some ways, since we can start the watch upfront, before any watch requests arrive | 20:28 |
| Aram | yes, that's the way it should be done. | 20:28 |
| Aram | niemeyer: btw, I know that it's this way because this is the way zk works, but is there a real reason why we care that create and set are different? (have to do create before set). | 20:30 |
| niemeyer | Hmmm | 20:31 |
| niemeyer | Aram: If one tries to Create a node, and there's one previously existent, a race was lost.. this is a pretty relevant fact for the distributed nature of the system | 20:32 |
| niemeyer | Aram: Without distinction, the algorithm would go on pretending all was good | 20:32 |
| Aram | not really, set should be set(path, data, ver), in effect compare-and-swap. you could specify a version for set, if there were already a node, you'd know. | 20:33 |
| niemeyer | Aram: Sure.. we can easily call Create as Set(version=-1).. then of course there's no distinction :) | 20:35 |
| niemeyer | Actually, not entirely.. Set(version=-1) exists.. it means change whatever the version | 20:36 |
| niemeyer | Create means change with non-existing-version | 20:36 |
| niemeyer | So, yeah, doesn't work | 20:36 |
| Aram | niemeyer: I know that Set(version=-1) changes whatever the version, this idea doesn't exist in doozer, you can't change without caring for the version, and there's no create, only set. | 20:38 |
| niemeyer | Aram: Cool, so the primitives are similar.. | 20:39 |
| davecheney | niemeyer: i was going to extract shortattempt so I can reuse it in the PA today | 22:12 |
| davecheney | so it will need a package, do you have any preferences ? | 22:12 |
| niemeyer | davecheney: It's a good question | 22:13 |
| niemeyer | davecheney: Good morning, btw | 22:13 |
| davecheney | and to you :P | 22:13 |
| davecheney | niemeyer: some suggestions are lp/juju-core/juju/util | 22:15 |
| davecheney | or even at the top level | 22:15 |
| davecheney | lp/juju-core/juju | 22:15 |
| davecheney | or in go style | 22:15 |
| davecheney | lp/juju-core/juju/jujuutil | 22:16 |
| davecheney | none are particularly good | 22:16 |
| niemeyer | davecheney: Indeed | 22:16 |
| niemeyer | davecheney: I'm always afraid of that kind of package because they end up as a bag of undefined things | 22:16 |
| davecheney | niemeyer: oh yeah, com/atlassian/confluence/bucket | 22:16 |
| davecheney | which was so named so people would feel ashamed of importing or using it | 22:17 |
| niemeyer | davecheney: :) | 22:17 |
| davecheney | which didn't work | 22:17 |
| davecheney | com/atlassian/confluence/bucket2 | 22:17 |
| davecheney | no kidding | 22:17 |
| niemeyer | davecheney: I suggest environs, for the moment | 22:17 |
| davecheney | niemeyer: as good as any | 22:17 |
| niemeyer | davecheney: It's not a great fit either, but we can look at it as a utility to be used in the implementation of environments, with aspirations of promotion onto a better home | 22:18 |
| davecheney | or I could just copy the bits I need into cmd/jujud and we defer the discussion until we have a better example ? | 22:18 |
| niemeyer | davecheney: Nah, the bad taste of a misplaced type is easy to fix.. code duplications tends to grow much uglier legs | 22:19 |
| davecheney | niemeyer: understood, i'll make it so | 22:20 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!