[01:13] Baking another mgo release [01:56] lp is really giving me the shits today [01:56] lucky(~/src/launchpad.net/juju-core) % lbox propose [01:56] error: Failed to load data for member "dave-cheney": Get https://api.launchpad.net/devel/~dave-cheney: unexpected EOF [01:57] davecheney: Aw [01:57] davecheney: Hmm.. they might be doing a release [01:57] It used to be thursday-evening/your morning [01:57] nope, just needs persistence [01:57] Ouch :) [01:58] niemeyer: i have a suspicion that these EOF problems we see, are possibly related to the high concurency failures that others are finding in the http package [01:59] davecheney: Hmm.. perhaps [02:00] Okay, that's bed time for me [02:00] Hopefully will go through the full night today [02:00] davecheney: Have a good day man [02:01] you too mate [07:32] heyheyhey [07:40] morning [07:41] yo [07:41] TheMue: good news [07:41] we have watchers [07:41] machine,service,unit, and relation watchers [07:41] Aram: cheers, great! +1 [07:44] Aram: integration of life and moving it into the own file also go on [07:44] Aram: did you see the list of missing tests? [07:45] Aram, TheMue: mornin' [07:46] wrtp: morning, today as wrtp? === wrtp is now known as rogpeppe [07:46] :-) [07:49] :P [08:13] * TheMue watches the creeps on his arm and once again things that starting the day with listening loud to Porcupine Tree is a good idea [08:21] TheMue: yes, saw the list of missing tests, great. [08:21] rogpeppe: yo. [08:21] fwereade: hey [08:21] hey Aram, hey everyone [08:22] TheMue: there will be some pain merging our branches. [08:22] TheMue: I want to do it today as monday I won't be here. [08:22] taking a day off. [08:23] fwereade: heya [08:23] TheMue, heyhey [08:24] Aram: ok, my next step is pushing the unit life extension in and then continue with moving life into an own file with own tests as you've seen. got reviews here. [08:24] Aram: will your txn stuff go in today? [08:24] I hope. [08:25] but if it doesn't you can pull it and use it as -req [08:27] Aram: ok [08:52] doez bzr have something like: git merge --strategy=ours? [08:52] that makes the VCS acknowledge the merge, but doesn't do any change. [08:52] (git also has theirs instead of ours, btw). [08:53] bzr resolve --take-this [08:53] worked :). [09:03] error: Failed to update merge proposal log: EOF [09:04] die [09:30] Aram: i pulled your branches, how can i set them as prerequisite of my already existing life branch? [09:35] you need to create a new branch where you merge them. [09:35] well [09:36] now I'll push some approved branches to trunk [09:38] Aram: ic [09:38] Aram: hoped to have the txn stuff in for life.go [09:59] TheMue: I think it would be very counterproductive to be working on anything lifecycle related until the txn stuff is not in. I'm already fighting merging wars ATM :). [09:59] I'll do my best to have everything ready by the end of the day, so you can pick it up on monday. In fact it may be ready earlier in the day. [09:59] Perhaps you could look over state/presence in the meantime? I only glanced over that. [10:01] Aram: right now i handle the lifecycle review (w/o txn), mostly improving the tests for readbility [10:02] Aram: i've already outlined txn in lifecycle depending on your branch and the first look has been easy [10:02] TheMue: btw, with txn and watchers, tests already take 18 seconds :(. [10:02] Aram: but that'll be a second step after the merge w/o txn [10:02] Aram: hehe, as expected, but still fast [10:04] Aram: for presence we'll have to see how to do it, i'll take a look. which is your watcher branch? [10:05] lp:~aramh/juju-core/58-mstate-watchers-basic, but pull lp:~aramh/juju-core/59-mstate-bson-D-trivial because it's better [10:05] btw, I'm updating those as we speak so caveat emptor. [10:06] Aram: thx [10:07] Aram: interesting that you prefix them with a number, dave imho does so too while others are doing a go-mstate-… or go-worker-... [10:07] it's easier to see them and the relationship between them this way [10:14] Aram: watchPollFreq = 1000 * time.Millisecond, why not time.Second? [10:14] I change it all the time, and usually make it much less [10:17] Aram: ok, and I've seen that you impl stuff like State.WatchMachines() in watcher.go. in state the watchers are in watcher.go but their usage is in their type file, here state.go [10:18] I know, it was a conscious decision. [10:18] we talked about it with rogpeppe at lisbon. [10:20] i'm not happy with it, watcher.go reads very confusing due to the type jumping. every few lines a context change [10:21] i had been so happy about williams work to order state regarding the tests [10:22] I am not happy with the old way :). [10:22] so that stuff that belongs to each other is close [10:23] for me due to maintenance reasos it's hard to have code for one type so distributed [10:23] reasons [10:24] it may be ok just in the moment of development, because the developer knows where the stuff is [10:24] but it's hard for anyone who is new to the code [10:25] it's the oposite to me. if I want to look at the watchers, I want to see the function that creates them close to the watchers. [10:26] it's very annoying to me every time I modify something not to find the function that creates the thing that I modified right away so I can modify that too. [10:26] and if you want to look at the service you don't expect its code in 5 different files [10:26] I need to grep since I have no idea what file it's in [10:26] so you can turn it around depending from which directions you ome [10:27] oderwise we could have a file where all naming stuff is handled, one for all txn stuff, one for all resolved stuff, one for all ports stuff [10:27] and so on [10:28] my question is: are you interested in technological details or in the business object/entity [10:28] if I modify a thing, say a data model for a type, it's very likely that I need to also modify the thing that creates it. so I want the function that creates the watchers to be close with the watchers, the function that creates the service to be close with the service (e.g. in service.go, not state.go as it is now) etc.) [10:29] i've often seen that later you're interested in the entities, because the biusiness needs are changing [10:30] Aram: why isn't Service.AddUnit() in unit.go? [10:30] it should be [10:30] it isn't because I just emulated state [10:31] Aram: ok, so today it's inconsistent, what makes it even more difficult. history in state is in the py implementation. i would ask you to talk to niemeyer for a clear direction [10:32] Aram: i can live with both, so that we can harmonize it [10:32] yes, it's bad that it's inconsistent. [10:32] it should be consistent. [10:32] +1 [11:02] lunchtime [11:24] TheMue: you might want to pull again, significant changes, I merged your first branch that you pushed to trunk. I had to change ensureLife signature. [11:25] one new *State param plus another int return. [11:27] ok [11:32] Aram: what do you thing about a signature change to ensureLife(State, Collection, Id, Life, Descr) (rev, error)? [11:32] Aram: i think it would be more logical this way [11:33] TheMue: I'll do it right away in this branch. [11:33] cheers [11:39] TheMue: done, pull lp:~aramh/juju-core/60-mstate-ensureLife-sigchange [11:41] Aram: done too, great, tyvm [11:54] TheMue: I updated the test list document, as now we have all confignode tests and some watcher tests [11:55] Aram: great [12:14] Aram: how do i select something in mgo if i have the collection as string and the id? [12:14] collection name [12:15] DB("").C(colname).FindId(id) [12:15] session.DB("").C(colname).FindId(id) [12:16] tyvm [12:38] Aram, TheMue, rogpeppe: anyone free for a potentially bikesheddy conversation? [12:38] fwereade_: sure [12:38] fwereade_: go ahaed [12:38] rogpeppe, TheMue: ok, this is about state persistence for the uniter [12:39] fwereade_: by which you don't mean "State", presumably... :-) [12:39] rogpeppe, TheMue: sorry, local state; anyway for the purposes of this discussion, we care about hook state and charm state [12:39] fwereade_: ok [12:39] rogpeppe, charm state is Installing, Deployed, Upgrading, Conflicted [12:39] ic [12:39] fwereade_: ok [12:39] rogpeppe, hook state is Running, Commiting, Complete [12:40] committing :-) [12:40] rogpeppe, when we have installed the charm itself, we always want to run the install hook [12:40] rogpeppe, yes indeed :) [12:40] fwereade_: ok [12:41] rogpeppe, what I do at the moment is set the charm state to Installing, install the charm, and leave that state around until I've written Running for the followup hook [12:41] so far reasonable :) [12:41] rogpeppe, TheMue: before running the actual hook, I set charm status to Deployed [12:42] rogpeppe, and so the states overlap neatly and I can always come back up to roughly where I was is the process goes down [12:42] fwereade_: so 1st hook.Running, then charm.Deployed, then hook.Run() [12:42] fwereade_: ok [12:42] rogpeppe, TheMue: *but* it is a bit weird to be messing around setting charm states as part of the runHook method [12:42] TheMue, exactly [12:43] rogpeppe, TheMue: now, most of the time, the Mode funcs return the result of a func called hookStateMode [12:43] fwereade_: it kinda seems reasonable for the install hook to be doing that. [12:43] rogpeppe, well, indeed, it's not *bad* but it gave niemeyer pause [12:43] fwereade_: uh huh [12:44] rogpeppe, so anyway there is a conventional way we choose the next mode to go to [12:44] rogpeppe, I was wondering whether adding a Queued state to hook might be neater [12:44] fwereade_: what would Queued imply? [12:45] rogpeppe, "next time you call hookStateMode, it will run this hook for you" [12:45] fwereade_: ah, queued to run but not actually running? [12:45] rogpeppe, yeah [12:46] rogpeppe, TheMue: so charm.Installing -> hook.Queued -> charm.Deployed [12:46] fwereade_: so then it would be hook.Queued, charm.Deployed,, hook.Running [12:46] rogpeppe, yep [12:46] fwereade_: that seems reasonable to me. [12:46] fwereade_: would hooks *always* go through a Queued state? [12:47] rogpeppe, TheMue: the issue for me is that the transition to running hides away in hookStateMode, which is... well it is an obvious place to find it, I guess [12:47] rogpeppe, I don't think they have to -- but I can't offhand think of a reason they shouldn't [12:47] fwereade_: what's hookStateMode again? is it that func type? [12:47] rogpeppe, yeah, it does a few things [12:48] rogpeppe, it goes straight to ModeHookError if a hook is apparently Running [12:48] rogpeppe, if it's Committing, it recommits [12:48] rogpeppe, if it's Complete, the next mode is determined by what the hook was [12:48] fwereade_: isn't it better to think of it as an action that does whatever is appropriate and returns the next action? [12:49] fwereade_: and the func is selected based on the state? [12:49] rogpeppe, yeah, I think I've convinced myself, and I think I probably need a better name then hookStateMode [12:49] fwereade_: so then it's evident that *of course* that's where to find the next mode, because that's how all mode transitions work. [12:49] s/the next mode/the transition to running/ [12:50] fwereade_: i think hookStateMode is fine. [12:50] rogpeppe, if it's running hooks as well itself that doesn't feel quite right [12:50] TheMue, yes, the next mode func will be Starting if the last hook was Install, and Started in all other cases [12:51] fwereade_: perhaps "uniterMode" might be more appropriate [12:51] TheMue, (so far, anyway, unit lifecycle will change that a bit I think) [12:51] fwereade_: or just "mode" :-) [12:51] rogpeppe, ha, maybe [12:51] fwereade_: 'cos it actually represents the global state of the uniter, right? [12:52] rogpeppe, well, in a sense, yeah [12:52] fwereade_: to me transit sounds more natural, coming from a state, going to a state (maybe the same) [12:52] fwereade_: i saw that top level loop... that's all it does :-) [12:53] TheMue, that's a wrinkle, if we're returning to the same state we're doing it wrong [12:53] TheMue: i think "mode" represents the "mode of operation" between states quite well. [12:53] TheMue, no sense shutting down a bunch of watches only to restart them and handle the initial events all over again [12:53] fwereade_: we return to the same state when we crash :-) [12:53] TheMue, rogpeppe: and what this then means is that hookStateMode is not the *only* thing that causes a state transition -- it's just the usual thing to do so [12:54] fwereade_: sorry, i missed something. what else causes a state transition? [12:54] rogpeppe, if we're in ModeStarted, for example, we run the hook and directly return ModeHookError if it fails [12:54] rogpeppe, we don't want to leave ModeStarted [12:55] fwereade_: isn't it ModeStarted the thing that runs the hook? [12:55] s/it // [12:55] rogpeppe, in that case, yes [12:56] fwereade_: isn't ModeStarted a hookStateMode? [12:56] rogpeppe, sorry, no [12:56] fwereade_: ah, why not? [12:56] rogpeppe, hookStateMode is a single func that returns a mode func, based on hook state [12:56] fwereade_: seems odd that it's not, but i'm probably missing something [12:56] ah! [12:57] i thought that each mode returned the next mode to transition to. [12:57] rogpeppe, yes it does [12:57] rogpeppe, sorry, I am clearly communicating something really badly [12:57] * rogpeppe goes to the source [12:58] rogpeppe, as it happens, though, hookStateMode itself *does* have the right signature to be itself a mode [12:58] fwereade_: where's hookStateMode defined and used? [12:59] rogpeppe, modes.go, 2nd func, used throughout [12:59] rogpeppe, sorry it's still called nextMode in that proposal [12:59] fwereade_: ah! [13:00] * rogpeppe finds the if ... { return } elses distracting [13:01] fwereade_: i think nextMode is perhaps a better name. or hookNextMode. [13:02] rogpeppe, I'm thinking the signature is telling me it should be ModeSomething [13:03] fwereade_: perhaps "deduceNextMode" [13:03] :-) [13:03] rogpeppe, I'm starting to actually feel fond of ModeTransition [13:03] fwereade_: another way of slicing it would be that if a mode returns nil, the main loop would call nextMode [13:04] fwereade_: 'cos it seems to me that the modes are only returning nextMode because they don't really know what's going to happen next. [13:04] rogpeppe, I experimented with that, felt a bit off in practice [13:05] rogpeppe, well, not really, they're returning it because while the logic is simple in the various individual places it makes it easier to think about if, where possible, we use the same logic whether we're recovering from a crash or just switching normally [13:05] Aram: see the "new" life_test.go at https://codereview.appspot.com/6481045/diff/9001/mstate/life_test.go, that's why i needed the generic selection [13:06] fwereade_: how is recovering from a crash different? [13:09] hey niemeyer. [13:09] niemeyer: we have watches. [13:10] machine, unit, service, and relation watches. [13:10] Heya! [13:10] niemeyer: yo! [13:10] Aram: Wow [13:11] rogpeppe: Heya [13:11] niemeyer: hi [13:11] Aram: I've been thinking about how to implement them, but I guess you've already moved forward with something [13:11] Aram: What's the plan there? [13:11] TheMue: Heya [13:12] niemeyer: just the lisbon plan, but using transactions in a way to avoid races. [13:12] Aram: What does that actually mean? :-) [13:13] TheMue: Do you have that list of watches we need? [13:13] documents have a rev field, watchers remember last rev and poll perdiodically, rev field is incremented in transactions along with a global rev field. [13:13] niemeyer: you've got it in your mail, as well as the missing tests [13:14] Aram: Okay, can we have a quick call to save ourselves time? [13:14] Aram: I mean, just for more bandwidth [13:14] niemeyer: sure [13:14] niemeyer: what we have to look for is the presence watcher [13:14] Aram: So we can sync up [13:14] TheMue: Can you join us too? [13:14] niemeyer: yep [13:14] TheMue: Yeah, that's an interesting case.. but let's nail down the others first [13:15] (since we're already on them) [13:15] * Aram is ready anytime [13:15] Starting [13:30] niemeyer: how is that field called, txn_rev? [13:31] fwereade_: "Not really so keen on "pending", that implies to me that it hasn't started." [13:31] fwereade_: Funny enough, that's exactly the case.. :-) [13:31] fwereade_: When we save that hook state, the hook hasn't started [13:31] niemeyer, well, it is the case at the time we write ie [13:32] s/ie/it/ [13:32] Aram: txn-revno [13:32] thanks [13:32] Aram: np [13:32] niemeyer, what do yu think of running/committing/complete? [13:32] fwereade_: "running" has the same issue of "started" [13:33] niemeyer, yeah, and I can convince myself if means completion-pending, as it were [13:33] fwereade_: It's awkward to find a hook in a running state to mean it's in an error state and in fact not running [13:33] niemeyer, agreed [13:34] niemeyer, now, about setting charm state in runHook -- I'm just adding a Queued hook state so that changeCharm can set that before marking the charm as Deployed before it returns [13:34] niemeyer, seems quite neat; sound sane? [13:35] fwereade_: Definitely, cheers [13:35] niemeyer, so: queued, pending, comitting, complete? [13:35] * fwereade_ just cannot spell that word today :/ [13:36] fwereade_: +1 :) [13:36] niemeyer, cheers [13:53] niemeyer, just reproposed https://codereview.appspot.com/6482053 [13:58] fwereade_: Super, thanks [14:02] Aram: Hmmm.. I thought we had agreed to sort out the "path" issue before integrating the confignode stuff? [14:03] niemeyer: is renaming to key fine? [14:03] Aram: LOL.. no :) [14:04] Aram: I'm still happy to push these branches forward for the moment, but this is not correct.. we should fix it next week [14:04] Aram: environConfigKey = "environ" [14:04] return readConfigNode(s, environConfigKey) [14:04] config, err = readConfigNode(s.st, s.Name()) [14:04] Aram: Guess what happens if we have a service named "environ" [14:05] yeah, that's not great. [14:07] Aram: For the moment, I suggest namespacing stuff with slashes [14:08] Aram: "unit/" [14:08] Aram: "service/" [14:08] Aram: Well, hmm [14:08] Aram: That's not right [14:08] Aram: We don't need config nodes for unit or service.. we already have their own nodes [14:08] Aram: Where do we need config nodes? [14:08] Aram: EnvironConfig.. Relation.. [14:09] niemeyer: no idea, state has it: http://go.pkgdoc.org/launchpad.net/juju-core/state#Service.Config [14:09] so I did the same [14:09] ditto for units [14:09] Aram: Ah, service config, yes.. as in the config machinery [14:10] Aram: Hmm.. units don't [14:10] Aram: The Service.Config stuff is what we hook into for the "juju set" and "config-get" commands [14:11] they don't seem to have now, they did when I implemeted them though. [14:11] Aram: Which means it's indeed generic [14:11] somebody deleted them in the meantime :). [14:11] Aram: (as in, we have no control over which keys exist) [14:11] Aram: I don't think that's true [14:11] Aram: Units don't have a configuration of their own in that fashion [14:11] Aram: The unit configuration is the determined by service configuration [14:12] Aram: That's always been the case [14:14] niemeyer: ah, yes. but my branch doesn't have confignodes for units though either [14:14] only for service and environment config [14:14] Aram: I know.. I was exploring the field with you so we know what we're going to do, rather than blaming or anything [14:14] ah, yes [14:15] Aram: So, we need namespacing.. how do we call that collection again? /me looks [14:15] cfgnodes [14:17] Aram: Hmmm.. we don't have nodes in Mongo.. we have documents.. Can we call the collection "settings"? I think that's how we've been referring to the concept generically speaking [14:17] settings it is [14:17] Aram: So.. we need them for.. the service config.. for relations, both scoped and global, ... [14:18] Aram: Anything else? [14:18] environment [14:18] Aram: +1 [14:18] Aram: We should also rename ConfigNode to Settings.. but let's wait until we delete the current state package so we don't have to worry about it [14:18] yes [14:19] Aram: Awesome, looks like that's all [14:20] Aram: So, we have a couple of choices.. the first one is splitting collections, the other is namespacing [14:20] Aram: I think we have to namespace either way, because we can't determine the number of relation scopes ahead of time [14:20] namespacing sounds better [14:20] Aram: So that's probably the better choice for everything [14:20] Aram: Cool, +1 [14:22] * TheMue likes the Settings idea and namespaces, so +1 [14:23] Aram: "e", "s/", "r/" [14:23] sounds fine [14:24] Aram: No point in using a large prefix and forcing a huge number of comparisons on indexes and whatnot with common prefixes [14:25] niemeyer: as our model is relative fixed it's ok. only the second s or r would break it. [14:27] * TheMue so far finds nothing that doesn't fit into the rest of 22 letters [14:36] Aram: Review sent [14:37] thanks [14:37] Aram: LGTM with these in, actually [14:40] niemeyer: txn will never insert phantom documents in the "real" collection, right? [14:48] Aram: That's rihgt [14:49] Aram: The only way the real collection and documents is ever changed is in the "txn-revno" and "txn-queue" fields of real documents [14:49] Aram: I very much didn't want to deal with crack in the real content [14:50] Aram: The "There's one race" aspect mentioned in the post is a side effect of that [14:57] * Aram tries to understand why DeepEquals fails since the debug print of what we got from the watcher and what we expect is the same. [15:01] niemeyer, http://paste.ubuntu.com/1164568/ [15:03] Aram: int vs. int64 is a common issue [15:04] fwereade_: wtf [15:04] fwereade_: "unrevisioned executability"!? [15:05] niemeyer, yeah, I know :/ [15:12] niemeyer: I mistakenly assumed I could use a type newDoc struct { Doc; something } else in a query, and it would fill the fields of the embedded document, but I was wrong. [15:13] Aram: You're half-wrong only, though [15:13] Aram: ",inline" [15:13] hah, thanks! [15:13] Aram: np [15:22] niemeyer: dumping the database, I see that these two documents have the same txn-revno: http://paste.ubuntu.com/1164608/ [15:22] how can that be? [15:22] they were added in different Run() calls. [15:22] Aram: revno is per document [15:23] :( [15:24] Aram: What's the issue? [15:24] Aram: The "txn-" prefix is there for namespacing since the package is injecting that into alien documents [15:25] Aram: Rather than meaning "the revision number of the transaction executed" [15:25] well, if it's per document I don't see how I can use it in watchers. perhaps I'm missing something. [15:25] Aram: Why? [15:25] Aram: Can you run me through the procedure you had in mind? [15:26] the watcher remembers the last used revno, and when it polls it does a query to see documents with a revno greater then it remembered. [15:27] since new documents start with a fresh revno, it won't find them this way. [15:28] did you had in mind a different procedure? [15:29] Aram: Hmm [15:29] Aram: No, I just hadn't considered the possibility of doing that kind of grouping query [15:29] Aram: The idea was the same, but individually per-docuemnt [15:30] Aram: But, let me ponder for a moment. You may be onto something [15:30] Aram: I think all the logic in the txn package would work equally well if it used max(revno)+1, which seems to solve the issue [15:31] Aram: Let me go to the drawing board for a second [15:36] Aram: Nope, doesn't work [15:37] indeed. [15:37] Aram: Obviously, transactions that affect independent documents can't guarantee to have a monotonically increasing revno [15:37] Aram: So, back to the question: what was the logic you were implementing? [15:38] Aram: I'm still curious because I think I'm missing something [15:38] Aram: If we have 100k documents, we're likely only interested in a selection of them.. why do we care about a global revno? [15:39] how do we detect new documents? [15:41] Aram: I guess it depends on the scenario.. [15:41] Aram: Let's pick one to evaluate [15:41] machines [15:41] Aram: Cool [15:42] Aram: Thinking about the best approach, just a sec [15:45] Aram: Okay, so here are a few constraints: [15:46] Aram: 1) We don't want to scan everything regularly [15:47] Aram: 2) We don't want a gigantic document with all the ids [15:49] Aram: I think we can solve both of them by having a control document that is touched every time the selection of machines changes [15:49] niemeyer: sorry, I have to go. I have to pack and I have been here for 12 hours already. [15:49] so this will have to wait for next week [15:49] Aram: Of course, that's not a problem [15:49] Aram: Have fun, and thanks for the hard work [15:49] thanks [15:49] cheer, and have a nice weekend [15:50] Aram: Thanks [16:00] Lunch here [16:02] fwereade_: ping [16:02] rogpeppe, pong [16:02] fwereade_: should there be an entry in the State for the bootstrap Machine? [16:02] rogpeppe, yes, I think so [16:02] fwereade_: currently i don't think there is, though i may be missing something [16:03] rogpeppe, hmm, I thought initzk was going to do that, maybe it doesn't yet [16:03] fwereade_: hmm, maybe it does, let me check [16:03] rogpeppe, that's what the --instance-id param is for [16:05] fwereade_: oh yes, so it does. thanks. [16:05] rogpeppe, cool [16:34] niemeyer, hmm, when you get back... if the charm dir's a repo, can you think offhand of any reason *not* to version everything and commit it at the end of each hook? [16:35] rogpeppe, TheMue, ^^ [16:36] fwereade_: and do a revert when starting? [16:37] rogpeppe, offhand, that would maybe seem to make sense [16:37] * fwereade_ thinks through [16:37] fwereade_: it seems to me there might be some advantage in using the same kind of mechanism for storing charm state as for storing hook state [16:37] rogpeppe, the mechanisms seem pretty similar to me [16:37] fwereade_: that way we're not reliant on the VCS except for upgrades [16:37]