| === _mup__ is now known as _mup_ | ||
| === zz_mwhudson is now known as mwhudson | ||
| fwereade | waigani___, heyhey | 22:12 |
|---|---|---|
| waigani_____ | fwereade: hello :) | 22:12 |
| fwereade | waigani___, how's it going? | 22:13 |
| waigani_____ | getting there | 22:13 |
| fwereade | waigani___, sorry I haven't really said hi properly since you joined :( | 22:13 |
| waigani_____ | a LOT of code to get my head around! | 22:13 |
| fwereade | waigani___, ha, yes indeed | 22:13 |
| waigani_____ | no problem, totally understandable | 22:13 |
| waigani_____ | ditto, sorry for not saying hi :) | 22:13 |
| waigani_____ | so, I'm getting racy hey? | 22:14 |
| fwereade | waigani___, I'm kinda going to bed shortly but I just wanted to chat briefly about the set environ config stuff | 22:14 |
| waigani_____ | sure | 22:14 |
| waigani____ | fwereade: hangout or irc? | 22:17 |
| fwereade | waigani___, the issue is mainly that I don't really want it to be possible to set an invalid environ config, and I am not convinced that it's possible to guarantee the sanity of a given change without (1) validating the change from the previous version --which is fine, you do it-- and (2) asserting in state that the previous version, from which the change is known to be valid, still applies | 22:17 |
| fwereade | waigani____, I think I can irc it, I will be off to bed in a mo | 22:17 |
| waigani____ | ok | 22:18 |
| fwereade | waigani____, does that make sense at first reading or shall I expand a little? ;) | 22:18 |
| waigani____ | right, so the real issue is the last one | 22:19 |
| waigani____ | how do you ensure the oldconfig is itslef valid | 22:19 |
| fwereade | waigani____, if you can guarantee every change is valid, I think yu can guarantee that the previous version was | 22:19 |
| fwereade | waigani____, because you cannot bootstrap but with a valid config | 22:20 |
| waigani____ | can we? | 22:20 |
| waigani____ | right | 22:20 |
| waigani____ | in which case the current method works then? | 22:20 |
| waigani____ | we can rely on an oldconfig | 22:21 |
| waigani____ | oohhhh | 22:21 |
| waigani____ | lol | 22:21 |
| waigani____ | i see | 22:21 |
| waigani____ | THAT is why you don't want to allow setconfig without validating | 22:21 |
| fwereade | waigani____, the current method is (at least in theory) crack, because the *actual* change we end up making is pretty random -- see state/state.go:264 | 22:22 |
| waigani____ | so this is the race condition you mentioned? | 22:23 |
| fwereade | waigani____, in *practice* it's usually ok, but that's because we don't get many concurrent updates to environ config | 22:24 |
| fwereade | waigani____, yeah, I think so | 22:24 |
| fwereade | waigani____, I have a tendency to froth about a wide range of possible race conditions | 22:24 |
| waigani____ | heh | 22:24 |
| fwereade | waigani____, ah, but, yes, that's the one | 22:25 |
| fwereade | waigani____, so what's interesting is that axw is doing some work that involves using an Environ inside state | 22:25 |
| waigani____ | what does axw mean when he says The settings changes are actually applied as a delta to what's on disk | 22:25 |
| waigani____ | "applied as a delta"? | 22:26 |
| fwereade | waigani____, there's a Settings type | 22:26 |
| fwereade | waigani____, it was written in python for zookeeper and ported straight without much consideration for whether it still makes sense | 22:26 |
| fwereade | waigani____, hint: it doesn't | 22:26 |
| waigani____ | lol, okay... | 22:26 |
| fwereade | waigani____, I am struggling to remember the *exact* details | 22:27 |
| fwereade | waigani____, but it is itself fundamentally racy | 22:27 |
| waigani____ | don't worry, I'll follow up | 22:27 |
| fwereade | waigani____, it reads existing state, calculates delta, applies delta | 22:27 |
| waigani____ | now I know the direction to look in | 22:27 |
| fwereade | waigani____, pretty sure it doesn't assert the base state still holds | 22:28 |
| waigani____ | now state == mongodb? | 22:28 |
| fwereade | waigani____, yeah, take a look at that for a bit of context | 22:28 |
| fwereade | waigani____, yep | 22:28 |
| waigani____ | i.e. config stored in mdb | 22:28 |
| fwereade | waigani____, but have a word with axw, because *he* is doing some work that involves using an Environ inside state to check sanity of AddMachine ops by checking with the provider | 22:29 |
| fwereade | waigani____, and *you* are doing something that involves checking sanity of applying changes to Environ (configs) | 22:29 |
| waigani____ | will do. | 22:30 |
| waigani____ | would you like to see the ApplyAndValidate method I've refactored out, added back into SetEnvironConfig? | 22:30 |
| waigani____ | Such that you cannot set an environ config without validating it? | 22:31 |
| fwereade | waigani____, I think that is at least part of it, but be suspicious of what actually happens when you try to use that settings type | 22:31 |
| waigani____ | fwereade: be suspicious of SetEnvironConfig? | 22:32 |
| fwereade | waigani____, it may be that you're best off using entirely different code that just happens to use the same data format | 22:32 |
| fwereade | waigani____, more of state.Settings | 22:32 |
| fwereade | waigani____, ...or, heh, maybe converting it to use a new data format | 22:33 |
| fwereade | waigani____, I'm also pretty sure that the fields are dumped straight into the mongo document | 22:33 |
| waigani____ | fwereade: just saw your comment: TODO(fwereade) state.Settings is itself really problematic in just about every use case | 22:33 |
| fwereade | waigani____, and by sheer conincidence magic fields like txn-revno happen not to collide with actual env settings | 22:33 |
| fwereade | waigani____, I think what I'm saying is that you have stumbled into a rabbit hole | 22:34 |
| fwereade | waigani____, do not feel obliged to fix everything | 22:34 |
| waigani____ | I was just about to say the same! | 22:34 |
| fwereade | waigani____, but be careful to understand what you're fixing and what you're not, and please document it clearly | 22:35 |
| fwereade | waigani____, take heart: you're unlikely to make that specific code *worse* | 22:35 |
| waigani____ | obviously my understanding is cursory, so I'll follow up with axw and look into state.Settings | 22:35 |
| fwereade | waigani____, and I'll be happy if it just gets a little bit better | 22:35 |
| fwereade | waigani____, awesome, thanks | 22:36 |
| waigani____ | are there any other hits for me? | 22:36 |
| waigani____ | bits of code I should look at? | 22:36 |
| fwereade | waigani____, for this particular stuff it's quite localised | 22:36 |
| fwereade | waigani____, there are a couple of other Settings clients | 22:37 |
| fwereade | waigani____, one day we'll get rid of them all | 22:37 |
| waigani____ | okay | 22:37 |
| fwereade | waigani____, but it's the env config that really upsets me | 22:37 |
| fwereade | waigani____, because it's the most potentially horrible one ;) | 22:38 |
| fwereade | waigani____, anyway I should get some sleep | 22:38 |
| waigani____ | just before you go .... :D | 22:38 |
| fwereade | waigani____, I might be a little late tomorrow, but dimitern is staying here, so if you see him and want to talk to me tell him to come shout at me :) | 22:38 |
| fwereade | waigani____, I'm still here :) | 22:39 |
| waigani____ | could you give me one example of the race conditon? | 22:39 |
| waigani____ | two clients trying to set the config at the same time? Is that right? | 22:39 |
| fwereade | waigani____, yeah | 22:39 |
| waigani____ | clients = machine agent, juju cli, ... | 22:40 |
| fwereade | waigani____, ah! | 22:40 |
| fwereade | waigani____, there *is* a replaceSettingsOp func | 22:40 |
| fwereade | waigani____, that I think is not racy | 22:40 |
| waigani____ | oh | 22:40 |
| fwereade | waigani____, it may need to be retried | 22:40 |
| fwereade | waigani____, but if you use it the txn will abort | 22:40 |
| waigani____ | okay, lots to learn ... I'll let you sleep | 22:41 |
| fwereade | waigani____, so, *if* you can construct a known-valid config, you can use replaceSettingsOp to be sure it actually lands in state | 22:41 |
| waigani____ | thanks for taking the time to walk me through the rabbit whole, a little ;) | 22:41 |
| fwereade | waigani____, but to construct a known-valid one inside state, you'll need to talk to axw | 22:41 |
| waigani____ | *hole | 22:41 |
| fwereade | waigani____, because | 22:41 |
| fwereade | (you're sitting down, right) | 22:41 |
| waigani____ | hehe yep | 22:42 |
| fwereade | waigani____, just because it's a valid config.Config | 22:42 |
| fwereade | waigani____, it is *not* necessarily a valid config for any given provider | 22:42 |
| waigani____ | ugh | 22:42 |
| fwereade | waigani____, let alone a valid change for that specific environ | 22:42 |
| waigani____ | what? | 22:42 |
| waigani____ | I get that it might not be valid for other providers | 22:43 |
| fwereade | waigani____, simplest case: some fields are immutable, or should be | 22:43 |
| fwereade | waigani____, change an env's name from foo to bar, bad things will happenb | 22:43 |
| fwereade | waigani____, constructing an env, and setting the new config, *should* catch all those cases | 22:43 |
| waigani____ | so that should be caught as invalid by the environ provider validator? | 22:43 |
| waigani____ | right | 22:44 |
| fwereade | waigani____, *but* the environs package uses the state package, so you can't directly reference an Environ as such from code inside state | 22:44 |
| fwereade | waigani____, axw is dealing with *exactly* the same problem, creating environs inside state to validate machine addition | 22:44 |
| fwereade | waigani____, so it would be good if your solutions were roughly aligned :) | 22:45 |
| fwereade | waigani____, was that a bit more helpful? | 22:45 |
| waigani____ | yes, you've set me in the right direction | 22:46 |
| fwereade | (ok, it's not *exactly* the same problem, but you can surely talk productively) | 22:46 |
| waigani____ | I'll go annoy axw now ;) | 22:46 |
| fwereade | waigani____, cool, cheers | 22:46 |
| waigani____ | thanks again, sleep well | 22:47 |
| * fwereade disappears | 22:47 | |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!