| TheMue | morning all | 08:57 |
|---|---|---|
| fwereade_ | heya TheMue | 09:02 |
| wrtp | fwereade_, TheMue: yo! | 09:02 |
| TheMue | fwereade_: moin | 09:02 |
| TheMue | wrtp: moin | 09:03 |
| TheMue | lunchtime | 11:43 |
| niemeyer | Morning all | 12:30 |
| TheMue | niemeyer: moin | 12:31 |
| niemeyer | TheMue: Heya | 12:33 |
| wrtp | niemeyer: hiya | 12:41 |
| hazmat | g'morning | 12:41 |
| fwereade_ | wtf, it's hailing | 12:51 |
| niemeyer | wrtp: Any chances of a few additional reviews? You've reviewed the tips but the pre-reqs are blocking them | 12:51 |
| fwereade_ | this is not why I moved to the med | 12:52 |
| wrtp | niemeyer: i thought i'd reviewed 'em all. | 12:54 |
| niemeyer | wrtp: Nope, there are still pending ones in the queue | 12:54 |
| wrtp | niemeyer: one mo, i'll try and remember where the queue page is again... :-) | 12:54 |
| niemeyer | wrtp: Meanwhile, I'll code a quick one | 12:55 |
| niemeyer | wrtp: TO disable stats so that performance checks do not affect them | 12:55 |
| wrtp | sometimes i want a forward link as well as a backward link, so i can easily find the next review to do... | 12:59 |
| wrtp | niemeyer: done, i think. let me know if i've missed any | 13:27 |
| niemeyer | wrtp: You have, the one I'm pushing *right now*! ;-) | 13:28 |
| wrtp | :-) | 13:28 |
| niemeyer | wrtp: Should be a breeze, though | 13:28 |
| niemeyer | wrtp: https://codereview.appspot.com/6061045 | 13:30 |
| wrtp | niemeyer: wouldn't it be better to have stats enabled by default? | 13:32 |
| wrtp | niemeyer: i.e. disablestats=1 rather than stats!=0 | 13:33 |
| niemeyer | wrtp: It is enabled by default.. isn't it? | 13:33 |
| wrtp | niemeyer: it needs the "stats" field in the form to be set, no? | 13:33 |
| niemeyer | wrtp: Nope | 13:33 |
| niemeyer | wrtp: It needs it to not be set to zero | 13:33 |
| niemeyer | wrtp: Note that tests continue passing | 13:34 |
| wrtp | doh | 13:34 |
| wrtp | niemeyer: LGTM | 13:35 |
| niemeyer | wrtp: Cheers! | 13:35 |
| wrtp | niemeyer: i still don't see why you can't just use a blank user name in the key BTW | 13:35 |
| niemeyer | wrtp: A blank string is a valid token | 13:36 |
| wrtp | rather than having those if statements strewn everywhere | 13:36 |
| wrtp | niemeyer: why's that a problem? | 13:36 |
| niemeyer | wrtp: It's a problem because there's no point in having to match it | 13:37 |
| niemeyer | wrtp: The data should be foo:bar, not foo:bar:: | 13:37 |
| niemeyer | wrtp: Just like the URL is cs:oneiric/hadoop, not cs:~/oneiric/hadoop | 13:37 |
| andrewsmedina | wrtp: hi | 13:39 |
| wrtp | andrewsmedina: hiya | 13:40 |
| andrewsmedina | wrtp: everything ok? | 13:40 |
| wrtp | andrewsmedina: good thanks | 13:40 |
| andrewsmedina | wrtp: I run the ec2 (go port) tests here in my machine, but they are failing | 13:42 |
| andrewsmedina | wrtp: I need to do something to run these tests? | 13:42 |
| wrtp | andrewsmedina: please run with -gocheck.vv and send me the output | 13:42 |
| wrtp | niemeyer: is "cs:~/oneiric/hadoop" valid? | 13:43 |
| niemeyer | wrtp: No, it's not.. that was the point | 13:43 |
| andrewsmedina | wrtp: http://paste.ubuntu.com/933966/ | 13:44 |
| andrewsmedina | wrtp: It's need set the AWS envs? | 13:44 |
| wrtp | andrewsmedina: you need to set up an amazon AWS account and set your $AWS_ variables appropriately | 13:45 |
| wrtp | niemeyer: how visible are the keys to users? | 13:45 |
| wrtp | niemeyer: i think that having all charm keys of the form series:name:user: even when user is blank should be ok, if it's not generally visible. | 13:46 |
| wrtp | niemeyer: it would remove quite a few lines of code, at any rate :-) | 13:46 |
| niemeyer | wrtp: It doesn't matter.. let's please not be lazy. | 13:46 |
| niemeyer | wrtp: The data is oneiric:hadoop.. not oneiric:hadoop:: | 13:47 |
| wrtp | niemeyer: i don't think it's lazy, i think it's appropriate canonicalisation | 13:47 |
| niemeyer | wrtp: You're trying to save a couple of lines of code.. I can put that in a function if it bothers you so much, but it shouldn't affect the desired data format that we want | 13:48 |
| niemeyer | wrtp: The proper data format is oneiric:hadoop, not oneiric:hadoop: | 13:48 |
| andrewsmedina | wrtp: this for local tests? | 13:50 |
| fwereade_ | niemeyer, re https://code.launchpad.net/%7Eniemeyer/juju/go-store-blitz-key/+merge/102230 ...I don't quite get what performance is being tested there, it doesn't seem like it'd hit anything very significant? | 13:50 |
| niemeyer | fwereade_: wrtp misunderstood the point of that key, and that's of course because I've been extremely vague about it | 13:51 |
| niemeyer | fwereade_: This is just a validation key to tell the service we own this site | 13:51 |
| fwereade_ | niemeyer, ha! ok, that suddenly makes lots more sense | 13:52 |
| wrtp | andrewsmedina: ah, they shouldn't be... that might be a bug. in the meantime, try setting the AWS_ variables to something random and see if it works | 13:52 |
| wrtp | niemeyer: ah! i thought it was used directly as part of a performance test... | 13:53 |
| niemeyer | wrtp: No, the goal is to test the real endpoints | 13:53 |
| wrtp | niemeyer: cool, i understand now. i wondered about the strange form of the URL. | 13:53 |
| niemeyer | wrtp: That's why we need stats=0 as well | 13:54 |
| wrtp | andrewsmedina: yes, it's a bug. will fix. but for the time being, just do export AWS_ACCESS_KEY_ID=x; export AWS_SECRET_ACCESS_KEY=x; before running the tests | 13:56 |
| fwereade_ | niemeyer, hmm, it seems like stats=0 is a choice between skewing the stats and skewing the performance data | 13:57 |
| fwereade_ | niemeyer, and while obviously we don't want to skew the stats, it doesn't feel quite right | 13:58 |
| niemeyer | fwereade_: I agree, obviously we don't want to skew the stats :-) | 13:59 |
| niemeyer | fwereade_: Hmm.. we can put the IncCounter run in the background, actually | 14:00 |
| niemeyer | fwereade_: Which means it won't affect the response timing either way | 14:00 |
| andrewsmedina | wrtp: ok! | 14:00 |
| fwereade_ | niemeyer, well, it's still load on the server, isn't it, even if it doesn't necessarily directly affect the response time of that query? I'd be happiest if we could switch it to perform a real write to a different place | 14:02 |
| fwereade_ | niemeyer, but that'll potentially have different characteristics too, so maybe I'm heading up the garden path here | 14:02 |
| wrtp | niemeyer: how about defining a function like this and using it throughout: http://paste.ubuntu.com/933992/ | 14:03 |
| niemeyer | <niemeyer> wrtp: You're trying to save a couple of lines of code.. I can put that in a function if it bothers you so much, (...) | 14:03 |
| niemeyer | wrtp: Yes :) | 14:03 |
| wrtp | niemeyer: it would make me happier, yeah. that "if curl.User" test is in quite a few places. | 14:04 |
| niemeyer | wrtp: Sure, will do it | 14:04 |
| niemeyer | fwereade_: That feels a bit over the top for the level of traffic we currently have :-) | 14:05 |
| fwereade_ | niemeyer, yeah, garden path :) | 14:05 |
| niemeyer | fwereade_: I'm most interested in knowing which ballpark we're in, and measuring that every once in a while to see how it's going | 14:06 |
| niemeyer | fwereade_: It's far from being any kind of issue that would require such care | 14:06 |
| fwereade_ | niemeyer, indeed | 14:06 |
| niemeyer | fwereade_: I appreciate your input though, for real | 14:06 |
| wrtp | niemeyer: i think i agree with fwereade_ that we should in general test the usual path (including stats gathering). but it's useful to be able to turn off stats too, so that we can see how much overhead they impose. | 14:06 |
| niemeyer | fwereade_: I will move these calls to the background, and put that with the change wrtp just requested | 14:07 |
| fwereade_ | niemeyer, perfect, thanks | 14:07 |
| wrtp | niemeyer: sounds good | 14:07 |
| niemeyer | fwereade_, wrtp: https://codereview.appspot.com/6063043 | 14:32 |
| wrtp | niemeyer: chunk mismatch :-( | 14:32 |
| wrtp | niemeyer: oh, it's working now | 14:32 |
| wrtp | oh no it's not | 14:32 |
| fwereade_ | niemeyer, LGTM | 14:33 |
| niemeyer | What the heck.. | 14:33 |
| fwereade_ | (I see no mismatches...) | 14:33 |
| niemeyer | Well.. wrtp: https://codereview.appspot.com/6063043/patch/1/3 | 14:33 |
| niemeyer | fwereade_: You're probably not looking at the side-by-side | 14:34 |
| wrtp | niemeyer: yeah, that works fine for me | 14:34 |
| fwereade_ | niemeyer, that is true | 14:35 |
| niemeyer | Hmm.. I'll have to add a trick to the test to stabilize it | 14:35 |
| wrtp | niemeyer: i'd implement use the same function in the test. | 14:35 |
| niemeyer | wrtp: I prefer to see it in the test | 14:36 |
| niemeyer | wrtp: Most of them are constants | 14:36 |
| wrtp | niemeyer: ok | 14:36 |
| wrtp | niemeyer: i'm slightly concerned about the number of background IncCounters there may end up being. depends on the traffic i guess. | 14:37 |
| niemeyer | wrtp: They're necessarily proportional to the number of background requests being handled | 14:38 |
| niemeyer | wrtp: If incrementing a counter is more expensive than handling the request, something is wrong | 14:38 |
| wrtp | niemeyer: ok, if you're sure about that. | 14:39 |
| niemeyer | wrtp: Well, it's more that I can't imagine how either of those statements could possibly be false.. :-) | 14:40 |
| wrtp | niemeyer: only if the IncCounters aren't completing in a timely way and something's hammering the server with a cheap request. | 14:42 |
| niemeyer | wrtp: That's addressed by the second point | 14:42 |
| wrtp | niemeyer: indeed. | 14:43 |
| niemeyer | Lunch time.. back in a bit to finish merging/applying reviews | 15:03 |
| * niemeyer waves | 16:07 | |
| * wrtp waves back. | 16:13 | |
| fwereade_ | gn all, happy evenings | 16:27 |
| niemeyer | fwereade_: Cheers! | 16:41 |
| niemeyer | wrtp: I think I forgot a couple of renames you requested, but still have them in mind. Will submit them at the end of the pipeline. | 16:41 |
| wrtp | niemeyer: i was just trying to go back and have a look at the reviews, but my gmail seems to be down. | 16:41 |
| wrtp | niemeyer: still down. let me know here of anything you'd like me to respond to... | 16:48 |
| niemeyer | wrtp: I think it's all good in general.. I've agreed to most things you've said | 16:48 |
| wrtp | niemeyer: could you paste a link to the CL please... | 16:48 |
| niemeyer | wrtp: I actually didn't miss the review points I mention above either.. they were in the caching branch, which is still not in | 16:49 |
| niemeyer | wrtp: Which one the 7-8 ones? :-) | 16:49 |
| wrtp | niemeyer: the more recent one (it has links to the previous ones) | 16:49 |
| wrtp | s/more/most/ | 16:49 |
| niemeyer | wrtp: https://code.launchpad.net/~niemeyer/juju/go-store-stats-bg/+merge/102316 | 16:50 |
| wrtp | i'm sure i sent a LGTM for that one, oh well | 16:51 |
| niemeyer | wrtp: You did on IRC | 16:52 |
| wrtp | ah, must've forgot to do it on codereview | 16:52 |
| wrtp | just the cache one to go | 16:53 |
| niemeyer | Oh, I think I missed what fwereade_ meant in one of his comments | 17:04 |
| niemeyer | TheMue: ping | 17:16 |
| TheMue | niemeyer: pong | 17:30 |
| niemeyer | TheMue: Heya | 17:30 |
| niemeyer | TheMue: Just wanted to see how the branch is coming | 17:31 |
| TheMue | niemeyer: hi, today i have a real splitted day | 17:31 |
| niemeyer | TheMue: Did you read my follow up comments yesterday? | 17:31 |
| TheMue | niemeyer: yes, and the changed ChangeWatcher is now in testing and will then go in | 17:31 |
| niemeyer | TheMue: Super.. did it feel good while doing the changes? | 17:32 |
| TheMue | niemeyer: it would have been better if the GetW() watch also accepts ZNONODE and later can be used for new created nodes | 17:33 |
| TheMue | niemeyer: but i think i found a way | 17:33 |
| niemeyer | TheMue: Cool | 17:34 |
| TheMue | niemeyer: it now also does a re-get if a first get runs on a ZNONODE, does an ExistsW() for the needed watch and stat is not nil (aka somebody has created the node exactly between the get and the exists) | 17:36 |
| niemeyer | TheMue: Cool | 17:36 |
| niemeyer | TheMue: Does it do a re-exists too, if the node disappears? :-) | 17:36 |
| TheMue | yip | 17:37 |
| TheMue | niemeyer: eh, yup ;) | 17:37 |
| niemeyer | TheMue: Hehe :) | 17:37 |
| niemeyer | TheMue: Super | 17:37 |
| TheMue | niemeyer: it's a loop waiting for the correct criterias | 17:37 |
| niemeyer | TheMue: Makes sense | 17:39 |
| niemeyer | TheMue: It's awesome to have that well abstracted away in that compact form | 17:39 |
| TheMue | niemeyer: test looks good | 17:40 |
| andrewsmedina | wrtp: I'm thinking in move cloudinit to environs package. Because it's used for all providers, ec2, local etc | 17:47 |
| wrtp | andrewsmedina: is it used for the local provider? | 17:48 |
| andrewsmedina | wrtp: somethings, like jujuOrigin and other things | 17:49 |
| wrtp | andrewsmedina: i was certainly thinking of moving into the environs package at some point, but i didn't think it was quite time yet | 17:49 |
| andrewsmedina | wrtp: I'm writing the local provider | 17:50 |
| wrtp | andrewsmedina: does the local provider actually execute the cloudinit init script? | 17:50 |
| wrtp | andrewsmedina: i was under the impression that the local environ would execute the agents directly. | 17:51 |
| wrtp | andrewsmedina: (but i'm fuzzy in this area :-]) | 17:51 |
| wrtp | andrewsmedina: i'm going to have to go very shortly BTW, sorry. | 17:54 |
| andrewsmedina | wrtp: ok, can I talk with you about it later? | 17:56 |
| wrtp | andrewsmedina: that would be good. any time tomorrow is fine. i'm online from about 0800UTC | 17:57 |
| * wrtp always finds crypto auth errors to be a pain to diagnose. | 17:58 | |
| andrewsmedina | wrtp: ok | 17:58 |
| wrtp | niemeyer: as you might guess, i'm actually testing the ssh tunnel stuff. not far now :-) | 17:58 |
| wrtp | "sshd: Failed publickey for rog from 127.0.0.1 port 45416 ssh2" ... but why has it failed? | 18:00 |
| wrtp | time to stop for the day. see y'all tomorrow. | 18:00 |
| wrtp | andrewsmedina: speak tomorrow, i hope | 18:00 |
| TheMue | niemeyer: So, changed watcher is in at https://codereview.appspot.com/6059044 | 18:00 |
| andrewsmedina | wrtp: ok | 18:01 |
| andrewsmedina | wrtp: thanks | 18:01 |
| niemeyer | TheMue: Within that loop, I believe the eventType is a red-herring | 18:07 |
| TheMue | niemeyer: could you please explain? | 18:08 |
| niemeyer | TheMue: Sure | 18:08 |
| niemeyer | TheMue: We used to consider a removal as an error, so we stopped immediately with the event type hint | 18:08 |
| niemeyer | TheMue: Now that's not an error anymore | 18:08 |
| niemeyer | TheMue: so we don't really care about what is the *last* event that we got. | 18:09 |
| niemeyer | TheMue: We care about our ability to get the state *now* | 18:09 |
| niemeyer | TheMue: So you can drop the usage of that detail entirely, I believe, and have a cleaner algorithm with the same semantics | 18:09 |
| TheMue | niemeyer: it's a change after a hint of rog. he had a problem with my first approach always trying a GetW() first. his idea has been to skip it if we enter update() with a last EVENT_DELETED | 18:11 |
| TheMue | niemeyer: i've done without it before and it worked fine, so i can easily remove it | 18:12 |
| TheMue | niemeyer: i'll do so and check it in again. or shall i wait until your review? | 18:13 |
| niemeyer | TheMue: Hmm | 18:15 |
| niemeyer | TheMue: So the idea is to optimize it so that we try to get the current state right first.. | 18:16 |
| niemeyer | TheMue: We can simplify it then.. | 18:18 |
| niemeyer | TheMue: By moving the "exists bool" into the parameter | 18:18 |
| niemeyer | TheMue: This should get rid of most of the extra logic handling the event type | 18:19 |
| niemeyer | TheMue: We also don't need a watch variable, btw.. we already have one in scope called nextWatch | 18:19 |
| TheMue | niemeyer: ok, think I've got your ideas, will try it | 18:23 |
| niemeyer | TheMue: As a trivial, comments at lines 126 and 133 are replicating the code below them exactly | 18:25 |
| niemeyer | TheMue: That's pretty much it as far as the code is concerned.. it looks nice | 18:26 |
| niemeyer | TheMue: Will check the tests once you propose again | 18:27 |
| niemeyer | Don't expect anything surprising there.. | 18:27 |
| TheMue | niemeyer: fine. and I just fetch an evening beer and can so continue now. *lol* | 18:31 |
| niemeyer | TheMue: Sounds like a good plan.. :) | 18:33 |
| TheMue | niemeyer: yeah | 18:33 |
| niemeyer | and I'm *almost* at the end of the pipeline | 18:34 |
| niemeyer | Or the graph, more precisely :) | 18:35 |
| TheMue | niemeyer: just put some code at the end of the pipeline *smile* | 18:46 |
| niemeyer | mthaddon: ping | 18:53 |
| TheMue | so, off for today | 19:22 |
| niemeyer | I'm taking a break.. | 20:38 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!