gary_posterif there's a review of https://codereview.appspot.com/8811043/ we could get it in under the wire00:14
gary_posterwhich would be nice :-)00:14
gary_posterand it is very small00:15
* davecheney looks00:16
davecheneygary_poster: this diff is dirty00:22
davecheneythere is some bzr .THIS crap in thre00:22
gary_posterdavecheney, I was assuming that it was a clean up00:22
gary_posterdavecheney, might a repush clean it up?00:23
gary_posterMakyo, ^^^ could you try a repush?00:23
gary_posterdavecheney, than kyou for looking00:23
davecheneyactually, this removes a mistaken checkin00:23
davecheneylet me publish my review, there is some more dirtyness00:23
gary_posterok thanks00:23
davecheneyhazmat: this was your fault!00:24
davecheneythumper: https://codereview.appspot.com/8811043/00:25
davecheneycan you give the gui guys a second LGTM00:25
davecheneygary_poster: or you could just commit it00:25
davecheneyif time is a factor00:25
gary_posterdavecheney, thanks much.  I'll check what is possible,00:25
* thumper looks00:25
* thumper off to the gym shorly for some sparring00:28
davecheneygary_poster: Makyo you've got two, make those changes and fire at will00:28
gary_posterawesome thanks very much davecheney and thumper00:28
=== thumper is now known as thumper-afk
davecheneydoes anyone have a link to last week's hangout agenda ?00:38
davecheneym_3: ping00:38
mgzokay, have the other bug down as well00:46
m_3davecheney: hey00:49
m_3mgz: we're stuc00:49
davecheneym_3: how's it going ?00:49
m_3mgz: I'm leaving it be atm00:49
m_3davecheney: seems hung up on something00:50
davecheneyhang on, changing hosts00:50
davecheneym_3: tell me where it hurts00:50
m_3davecheney: we brought up 200 earlier to checck on adding incremental nodes via nova cli00:51
mgzm_3: I saw it's not increasing, but didn't see anything in the log00:51
m_3davecheney: mgz played a bit00:51
m_3davecheney: then added another 200 (juju add-unit hadoop-slave -n200)00:51
mgzI have two of the three issues I've seen tracked down00:51
m_3davecheney: it added 15 more and hung00:51
* m_3 gotta relocate... getting kicked out of the expo area00:51
davecheneymgz: i've seen an issue where the PA will get into a loop if there are problems creating security groups00:52
m_3back on after while... I'll leave it up for y'all00:52
davecheneym_3: mgz: i suggest destroying this instance00:52
davecheneybzr pull to rev 116400:52
davecheneythen trying again00:52
davecheney2013/04/16 23:41:06 ERROR provisioning worker: failed to GET object provider-state from container goscale2-100:53
davecheneycaused by: https://region-a.geo-1.objects.hpcloudsvc.com/v1/17031369947864/goscale2-1/provider-state%!(EXTRA string=failed executing the request)00:53
davecheneyi think i have a fix for these formatting errors00:53
davecheneymgz: i'd like to destroy this envbironment and start again with a fresh build00:55
mgzokay, we could probably do with fixing a couple of the dumb issues too00:56
davecheneyrev 1164 cuts down on 66% of the log span00:56
mgzthat particular one is probably api flood issue again00:56
davecheneyi'll grap the all machines log00:56
davecheneythe blow away the environment00:57
davecheneyubuntu@juju-hpgoctrl2-machine-0:~$ scp .00:59
davecheneyall-machines.log                                                            20%  141MB   5.6MB/s   01:38 ETA00:59
davecheneythat is a great network hp00:59
mgzfiled bug 116977801:03
mgzdavecheney: compress first :)01:03
davecheneymgz: sure01:04
davecheneybut 5.4Mb/s is shit01:04
davecheneymy pandaboard can do better than that01:04
davecheneymgz: it's nice to know you can resume juju destroy-environment01:04
davecheneymgz: m_3 we have 7 stray nova instances after destroy-environemnt01:08
davecheneyactually only 201:08
davecheneynever mind01:08
mgzone was mine, that I killed, and one is the manage one?01:12
davecheneyfixed it01:12
davecheneywc -l was the wrong thing to tuse01:12
davecheneymgz: m_3 deploying 200 hadoop-slave units01:15
davecheney100% of the time is being consumed waiting on hp to respond to new instance requests01:17
m_3davecheney: hey01:57
m_3davecheney: how's it going?01:57
m_3davecheney: btw, you never want to use 'hadoop-slave' the charm... (long story)01:58
davecheneym_3: 2013/04/17 01:52:25 NOTICE worker/provisioner: started machine 200 as instance 150998501:58
m_3davecheney: make sure you're doing `juju deploy hadoop hadoop-slave -n200`01:58
davecheneym_3: sorry, i was looking in the history, that is all I could find01:58
davecheneynm, we can destroy the instance01:58
m_3always ran from bin/hadoop-stack01:58
davecheneyok, let me nuke this environment01:59
davecheney(it worked fine btw01:59
davecheneyrev 1164 is much less chatty01:59
m_3davecheney: did it get above 200?01:59
m_3davecheney: we have perms to go to 200001:59
davecheneyjust did -n20001:59
m_3were just having probs right near the 200 mark01:59
davecheneyi'll bootstrap a fresh env01:59
davecheneydo bin/hadoop-stack01:59
m_3I did a `-n1997` before01:59
davecheneythati'll take hours02:00
m_3did you get mgz's changes in?02:00
davecheneyno, but the logging change went in02:00
m_3yup... about 8hrs or so02:00
davecheneyso we can actually see what is going on02:00
m_3ah, cool02:00
m_3wanna do 300?02:00
m_3or shall we go big02:00
davecheney500 should see us throught to drinks02:00
davecheneyhow is ODS ?02:00
davecheneyis the marketing completely off tap02:00
davecheneyhookers and blow ?02:01
m_3it's pretty over the top02:01
m_3yeah, all hookers and blow02:01
m_3supposed to meet kapil in the lobby for dinner in a bit02:01
davecheneyactaully, card tables, laptops and tshirts02:01
m_3yeah :(02:02
m_3better than couch, popcorn, and underwear at least02:02
davecheneym_3: i don't need to go to a conference for that02:02
davecheneym_3: we're good to go02:04
davecheneyhit it with the old -n30002:04
davecheneyhmm, that isn't good02:05
davecheneylet me drive for a second02:05
davecheneyok, still piss farting around02:05
davecheneythe last bootstrap was much faster02:05
davecheneystill scroblling in apt02:06
davecheney     ├─cloud-init-cfg─┬─add-apt-reposit02:06
davecheney     │                └─sh───tee02:06
davecheneythat is slow02:06
davecheneythis instance may fail to bootstrap02:06
m_3might just killl it02:07
davecheneydo it02:07
davecheneyoh wait02:07
davecheneyyeah, that is really slow02:07
davecheneynuke it02:07
m_3one that one's up just run bin/hadoop-stack02:08
m_3I'll chcek in after dinner if that's cool with you?02:08
davecheneym_3: ok, is that configured for -n300 ?02:08
m_3maybe you can catch what's going wrong near the 200 mark02:08
davecheneytotolly cool02:08
davecheneyyou go have dinner with K02:08
m_3awesome thanks man... ttyl02:08
m_3oh, and let's archive the logs in ~/arch/<date>02:09
davecheneysure, i've been scp'ing them onto my machine02:09
davecheneywill fix02:09
m_3at least to keep some sort of record of what we're doing02:09
davecheneyactually, i'll put it in google drive02:09
m_3I'm still rsyncing the whole control node's /home/ubuntu offsite daily02:09
davecheneyprobably isn't a bad idea02:10
davecheneymake sure you use rsync -z02:10
m_3cool... /me food02:10
m_3thanks man02:10
davecheneym_3: also, status now shows relations02:12
davecheney2013/04/17 02:13:05 DEBUG environs/openstack: openstack user data; 2708 bytes02:13
davecheney2013/04/17 02:13:05 ERROR settings for entity with unrecognized key "r#0#requirer#hadoop-slave/6"02:13
davecheney2013/04/17 02:13:05 ERROR settings for entity with unrecognized key "r#1#requirer#hadoop-slave/6"02:13
davecheney2013/04/17 02:13:05 ERROR settings for entity with unrecognized key "r#0#requirer#hadoop-slave/5"02:13
davecheney2013/04/17 02:13:05 ERROR settings for entity with unrecognized key "r#1#requirer#hadoop-slave/5"02:13
davecheney2013/04/17 02:13:05 ERROR settings for entity with unrecognized key "r#0#requirer#hadoop-slave/6"02:13
davecheney2013/04/17 02:13:05 ERROR settings for entity with unrecognized key "r#1#requirer#hadoop-slave/6"02:13
davecheney2013/04/17 02:13:05 ERROR settings for entity with unrecognized key "r#0#requirer#hadoop-slave/5"02:14
davecheney2013/04/17 02:13:05 ERROR settings for entity with unrecognized key "r#1#requirer#hadoop-slave/5"02:14
davecheneyif this is an error, why does the provisioner not stop02:14
thumperdavecheney: that doesn't look good02:14
davecheney2013/04/17 02:12:29 INFO environs/openstack: starting machine 14 in "goscale2" running tools version "1.9.14-precise-amd64" from "https://region-a.geo-1.objects.hpcloudsvc.com/v1/17031369947864/goscale2-1/tools/juju-1.9.14-precise-amd64.tgz"02:15
davecheney2013/04/17 02:12:29 DEBUG environs/openstack: openstack user data; 2710 bytes02:15
davecheney2013/04/17 02:12:40 ERROR settings for entity with unrecognized key "r#0#requirer#hadoop-slave/1"02:15
davecheney2013/04/17 02:12:40 ERROR settings for entity with unrecognized key "r#1#requirer#hadoop-slave/1"02:15
davecheney2013/04/17 02:12:40 ERROR settings for entity with unrecognized key "r#0#provider#hadoop-master/0"02:15
davecheney2013/04/17 02:12:40 ERROR settings for entity with unrecognized key "r#1#provider#hadoop-master/0"02:15
davecheney2013/04/17 02:12:40 ERROR settings for entity with unrecognized key "r#0#requirer#hadoop-slave/1"02:15
davecheney2013/04/17 02:12:40 ERROR settings for entity with unrecognized key "r#1#requirer#hadoop-slave/1"02:15
davecheney2013/04/17 02:12:40 ERROR settings for entity with unrecognized key "r#0#provider#hadoop-master/0"02:15
davecheney2013/04/17 0202:15
davecheneystarts to screw up here02:15
davecheneycaused by: https://az-2.region-a.geo-1.compute.hpcloudsvc.com/v1.1/17031369947864/servers%!(EXTRA string=failed executing the request)02:17
davecheneyi really want to fix this stupid formatting errror02:17
davecheneyshit shit shit02:47
davecheneythe 1.9.14 release cannot seem to download any tools02:47
davecheneyhowever when I build the release from source02:47
davecheneyit can !02:47
davecheney2013/04/17 02:50:21 DEBUG checking tools 1.9.14-precise-amd6402:51
davecheney2013/04/17 02:50:21 INFO environs/openstack: starting machine 298 in "goscale2" running tools version "1.9.14-precise-amd64" from "https://region-a.geo-1.objects.hpcloudsvc.com/v1/17031369947864/goscale2-1/tools/juju-1.9.14-precise-amd64.tgz"02:51
davecheney2013/04/17 02:50:21 ERROR worker/provisioner: cannot start instance for machine "298": cannot find image satisfying constraints: failed to get list of flavours02:51
davecheneycaused by: https://az-2.region-a.geo-1.compute.hpcloudsvc.com/v1.1/17031369947864/flavors%!(EXTRA string=failed executing the request)02:51
davecheneycaused by: Get https://az-2.region-a.geo-1.compute.hpcloudsvc.com/v1.1/17031369947864/flavors: lookup az-2.region-a.geo-1.compute.hpcloudsvc.com: Temporary failure in name resolution02:51
davecheneyi'm having a lot of trouble validating the 1.9.14 release03:12
davecheneyis anyone in a position to install 1.9.14 from ppa:juju/devel03:13
davecheneyand try03:13
davecheney(don't forget to make sure you're calling /usr/bin/juju, not ~/bin/juju)03:13
fwereadehazmat, if you're there, I don't think we necessarily want to destroy machines03:33
fwereadehazmat, I suspect the deploy-ubuntu-to-prime, force-machine-to-deploy (anti-)pattern will be popularly used03:34
fwereadedavecheney, thumper: if you're there, can I get a trivial on https://codereview.appspot.com/8815043 please?03:54
davecheneyfwereade: i'll take a look03:57
davecheneyfwereade: i'm very concerned that 1.9.14 is stillborn03:57
davecheneyi'm thinking of proposing we move to daily point releases03:58
davecheneyso m_3 and I stop using the development chain03:58
fwereadedavecheney, I don't think it'll build, given this stuff03:58
fwereadedavecheney, it's an old test file that somehow crept in03:58
davecheneywut ?03:59
fwereadedavecheney, at a casual glance it looks like it slipped in from the force-machine branch04:00
davecheneythis is the second turd to fall out of that branch04:00
fwereadedavecheney, although it's not quite clear how04:00
davecheneywithout being rude, 4 people looked at that branch04:00
fwereadedavecheney, ouch :(04:00
davecheneyand nobody spotted the .THIS file there04:00
davecheneyfwereade: LGTM, please submit04:02
fwereadedavecheney, happening right now04:03
fwereadedavecheney, looks like that file slipped in somewhere between 1 and 5 days ago after the main round of reviewing04:03
davecheneyso that means the submitter did not run the tests before submitting04:04
davecheneydo we need to make another adding to .lbox.check ?04:04
fwereadeI never found out what happened to the bot proposal but that also seems to be stillborn04:05
fwereadein its absence, maybe we do04:05
thumperhi fwereade04:05
thumperwhat are you doing up?04:06
fwereadethumper, fell asleep early last night, back up now to merge a whole pile of things and do some reviews04:06
davecheneyfwereade: we can't have a bot til the tests pass reliably04:06
davecheneythat was my understanding04:06
thumperfwereade: oh, I have four reviews up?04:06
* thumper goes to drink the coffee04:06
fwereadedavecheney, if we can't have a bot without reliable tests but we can't commit reliably without a bot *something* needs to change04:08
davecheneyfwereade: i think we can at least do a test compile of the tests04:09
davecheneythat would ahve caught this error04:09
davecheneybut it does seem a bit futile04:09
davecheneywe're like the TSA04:09
davecheneyrunning after the horse04:09
fwereadedavecheney, oh! how do we compile tests without running them?04:09
fwereadedavecheney, actually, don't tell me, I might start doing it ;p04:10
davecheneyfwereade: i'm sure there is a way04:11
davecheney(cd /tmp ; go test -c $PKG) will do it04:11
fwereadedavecheney, emphatic +1 then04:12
davecheneyfwereade: http://paste.ubuntu.com/5714987/04:17
davecheneycan anyone else test the 1.9.14 release ?04:17
davecheneythe top invocation is from the pacakge04:18
davecheneythe bottom is the same source04:18
davecheney2013/04/17 02:50:17 INFO environs/openstack: starting machine 207 in "goscale2" running tools version "1.9.14-precise-amd64" from "https://region-a.geo-1.objects.hpcloudsvc.com/v1/17031369947864/goscale2-1/tools/juju-1.9.14-precise-amd64.tgz"04:23
davecheney2013/04/17 02:50:17 ERROR worker/provisioner: cannot start instance for machine "207": cannot find image satisfying constraints: failed to get list of flavours04:23
davecheneycaused by: https://az-2.region-a.geo-1.compute.hpcloudsvc.com/v1.1/17031369947864/flavors%!(EXTRA string=failed executing the request)04:23
davecheneycaused by: Get https://az-2.region-a.geo-1.compute.hpcloudsvc.com/v1.1/17031369947864/flavors: lookup az-2.region-a.geo-1.compute.hpcloudsvc.com: Temporary failure in name resolutio04:23
davecheneyafter 200 machines, we loose the ability to resolve the name of the hp endpoint04:23
fwereadedavecheney, hmm, I seem to be bootstrapping ok in us0east-1 with 1.9.14 from the package04:24
thumperfwereade: so is it like 6:30am for you now?04:24
fwereadethumper, yeah04:25
thumperfwereade: cool, not even mind-numbingly early04:25
thumperfwereade: don't suppose you have time for a quick hangout?04:25
fwereadeas predicted I fell asleep early and woke up early04:25
fwereadethumper, sure04:25
davecheneyfwereade: i cannot bootstrap into ap-southeast-1 or 2 with that release04:25
thumperfwereade: I'll start04:25
davecheneybut it works perfectly using the source04:25
* davecheney has had a gutfull of unreliable cloud providers04:31
davecheneyubuntu@juju-hpgoctrl2-machine-0:~$ juju destroy-environment04:31
davecheneyerror: failed to delete server with serverId: 151032304:31
davecheneycaused by: https://az-2.region-a.geo-1.compute.hpcloudsvc.com/v1.1/17031369947864/servers/1510323%!(EXTRA string=failed executing the request)04:31
davecheneycaused by: remote error: handshake failure04:31
fwereadedavecheney, ap-southeast-2 is... well, appearing to bootstrap anyway04:50
fwereadedavecheney, but acting a bit weird, I will let you know how I get on04:50
fwereadedavecheney, ok, the good news is that the bootstrap node comes up and works fine, the bad news is that the CLI will not acknowledge its existence04:56
davecheneyfwereade: -v ?04:57
davecheneydid it bootstrap the wrong tools04:57
fwereade2013/04/17 06:55:37 INFO environs/ec2: waiting for DNS name(s) of state server instances [i-79c80744]04:57
fwereade2013/04/17 06:55:42 ERROR command failed: no instances found04:57
fwereadeerror: no instances found04:57
fwereadedavecheney, the instance definitely exists and it's definitely got the right tools04:58
davecheneyfwereade: i;m getting these errors constantly during the load tests04:58
davecheney2013/04/17 04:57:50 ERROR settings for entity with unrecognized key "r#0#requirer#hadoop-slave/2"04:58
davecheney2013/04/17 04:57:50 ERROR settings for entity with unrecognized key "r#1#requirer#hadoop-slave/2"04:58
davecheney2013/04/17 04:57:50 ERROR settings for entity with unrecognized key "r#0#requirer#hadoop-slave/0"04:58
davecheney2013/04/17 04:57:50 ERROR settings for entity with unrecognized key "r#1#requirer#hadoop-slave/0"04:58
davecheney2013/04/17 04:57:50 ERROR settings for entity with unrecognized key "r#0#requirer#hadoop-slave/2"04:58
fwereadedavecheney, so that is good anyway04:58
davecheney2013/04/17 04:57:50 ERROR settings for entity with unrecognized key "r#1#requirer#hadoop-slave/2"04:58
davecheney2013/04/17 04:57:50 ERROR settings for entity with unrecognized key "r#0#requirer#hadoop-slave/0"04:58
davecheney2013/04/17 04:57:50 ERROR settings for entity with unrecognized key "r#1#requirer#hadoop-slave/0"04:58
fwereadedavecheney, ah ffs there is no way those should be logged04:59
fwereadedavecheney, they're not errors, it's just the allwatcher getting its knickers in a twist because it's watching things it doesn't know about05:00
fwereadedavecheney, btw, we seem to be waiting only a very few seconds for the DNS name (apse2 issues), which is surprising to me05:01
davecheneyfwereade: oh, so that is the api server05:10
fwereadedavecheney, yeah05:10
davecheneyi was wondering why provisioning didn't land in a screaming heap05:10
davecheneyfwereade: i'll log a bug05:10
fwereadedavecheney, they're not even really errors -- it's just logging and stopping processing for those docs05:11
fwereadedavecheney, cheers05:11
davecheneyi'll log two bugs05:11
davecheneycan I get a review on https://codereview.appspot.com/8818043/05:11
fwereadedavecheney, what happened to errors.Newf (if anything)?05:21
davecheneynothign, just people were not calling it correctly05:22
davecheneyURL was not a format specifier05:22
davecheneyit was arg 105:22
* fwereade sees now05:23
fwereadedavecheney, LGTM05:23
TheMuefwereade: seen my part 1? during sleep I found a by far simpler approach. will continue after breakfast.06:07
jam1afternoon wallyworld, how's it going today?06:21
fwereadeTheMue, great news, I'm afraid I fell asleep last night ;p06:21
wallyworldjam1: going well thanks, just sorting out a test isolation issue and i will send a mp for my work06:21
=== jam1 is now known as jam
TheMuefwereade: no wonder after your night before06:22
jamfwereade: second LGTM on https://codereview.appspot.com/8818043/06:28
jamsorry, I meant davecheney ^^06:29
jambigjools, jtv: Is the maas provider ready to be exposed in jujud/juju client?06:31
* jtv is not here06:32
bigjoolsjam: what is the effect of that?  Sorry, I have no context on it.06:33
jambigjools: if you have it enabled, people can configure it in ~/.juju/environments.yaml and try to bootstrap against a maas service.06:34
bigjoolsas far as I know it all works, so if there's something missing I don't know why it's needed06:34
jamIf it is ready for people to do so, then we should expose it.06:34
jam(which it has been)06:34
jamBut we also want to configure jujud to require it06:34
bigjoolsjam: we can already bootstrap with it, I don;t understand06:34
jamso we don't accidentally lose it later.06:34
jambigjools: I was just confirming that the code has gotten to the "working" state. I hadn't heard a specific update to where you guys were.06:35
davecheneyjam: bzr: ERROR: Cannot lock LockDir(chroot-72851216:///%2Bbranch/goose/.bzr/branch/lock): Transport operation not possible: readonly transport06:35
davecheneyI cannot submit that mp06:35
jamdavecheney: so I would say LGTM trivial on exposing it.06:35
bigjoolsjam: ok :)  yes it should all work06:35
davecheneyany ideas ?06:35
jamdavecheney: you can't use 'lbox submit' for goose. You mark the MP as approved (make sure it has a commit message), and then goose bot will run the tests and land it for you.06:35
jamI can do it if you need help.06:35
m_3davecheney: how's it going?06:36
jamdavecheney: so on here: https://code.launchpad.net/~dave-cheney/goose/001-fmt-error/+merge/15928706:36
davecheneym_3: hey06:36
jamthere is a "Set commit message" which you can just copy the description if you are happy with it.06:36
jamand then mark it Approved.06:36
davecheneysent you an email with the sitrep06:37
davecheneybasicallyu can't get above 228 machines06:37
davecheneyi susecpt we're running out of ipv4 allocations06:37
davecheneyand it is being reported in a strange way06:37
m_3actually, that's awesome... this is the shit we're doing this to flush  out06:37
davecheneythe way the dns address of the keystone endpoint stops resolving is both06:38
davecheneya. repeatable06:38
davecheneyb. fucked up06:38
* m_3 reading mail06:38
m_3davecheney: we can see our ip quotas... one sec06:39
davecheney228 is close enough to 254 that it makes me go hmmm06:40
davecheneym_3: ahh06:43
davecheney228 isn't near any of those numbers06:43
davecheneyjam: now what happens ?06:44
m_3davecheney: hmmmm.... thought that showed ip allocations too06:45
* m_3 rtfm-ing06:45
davecheneyjam: is the bot broken ?06:49
m_3davecheney: I'll dig through the acct setup with antonio tomorrow06:50
m_3davecheney: jorge and I are talking tomorrow, so won't get to this til later in my daytime06:51
jamdavecheney: usually it takes about 5 min for it to wake up and run the test suite06:51
jambut if it fails it *should* report a failure to Launchpad and unmark the proposal as pneding06:51
* davecheney thinks its dead06:52
m_3davecheney: thanks for the help... /me beddy-bye06:52
davecheneym_3: no worries mate06:52
jamdavecheney: from what I can tell 'canonistack' thinks the machine is running, but I'm getting 0 response from port 2206:53
davecheneyjam: i remember a discussion a while ago where you metnoined that you weren't able to contact th emachine06:56
davecheneybut it appeared to be doing stuff, so whateva06:56
wallyworldjam: i'm off to soccer, but here's a wip mp. dimiter was interested to see it. i can't land till his work goes in since i need to account for whatever changes he has made. i also need to land a small goose change07:14
wallyworldthe covering letter also need to be expanded, but i am late07:15
* TheMue is happy, the simpler approach works fine. now part two adding the subordinate unite information.07:30
rogpeppe2mornin' all07:54
TheMuerogpeppe2: morning07:56
=== TheRealMue is now known as TheMue
rogpeppe2in case my earlier email didn't get through, my phone line is down currently09:25
rogpeppe2i'm connected through my mobile currently, but i don't know how reliable it will be09:25
rogpeppe2trivial branch that fixes #1169825 ; review appreciated please: https://codereview.appspot.com/882104309:33
* TheMue does the subordinate dance ...09:59
rogpeppe2fwereade: i'm after a review of this fairly trivial branch, please. i think it's important to get in before the release. https://codereview.appspot.com/882304310:09
fwereaderogpeppe2, ok, I'll chat about s3 in a mo10:09
rogpeppe2fwereade: ok10:10
fwereaderogpeppe2, nice, LGTM10:11
rogpeppe2fwereade: trivial?10:12
TheMuefwereade: you've got a proposal :)10:12
fwereaderogpeppe2, yeah, I think so10:12
rogpeppe2fwereade: cool, thanks10:12
fwereadeTheMue, cheers10:12
* rogpeppe2 wonders how long it'll take to run lbox submit through the mobile data connection10:12
TheMuefwereade: it's https://codereview.appspot.com/8824043/10:13
=== rogpeppe2 is now known as rogpeppe
fwereadeTheMue, cheers10:13
fwereaderogpeppe, I am somewhat ambivalent about changing Storage.Put10:13
rogpeppefwereade: i think that's going to be a necessary thing if we ever want retries to work.10:14
fwereaderogpeppe, I would be much happier if it took a Reader and tried to ReadSeeker it10:15
rogpeppefwereade: i wondered about that, but i was very glad i didn't10:15
fwereaderogpeppe, oh yes?10:15
rogpeppefwereade: because the static type caught lots of places that i would not otherwise have found10:16
rogpeppefwereade: it's not that we can stream data without having it first either10:17
rogpeppefwereade: because we need to know the length10:17
fwereaderogpeppe, it is not unheard of for content lengths to be declared10:17
rogpeppefwereade: that is true, i suppose10:17
fwereaderogpeppe, if they are, then great,but even if not I don't really think we should force all clients to buffer everything10:18
rogpeppefwereade: but in all the cases we care about, we do want retries to work10:18
rogpeppefwereade: and they won't work on a straight io.Reader10:18
fwereaderogpeppe, maybe it's our own responsibility to buffer though10:19
rogpeppefwereade: i think so10:19
fwereaderogpeppe, ha, different "our" I suspect10:19
rogpeppefwereade: ah, maybe so :-)10:19
rogpeppefwereade: i really don't think that Put should do the buffering itself10:19
rogpeppefwereade: if that's what you were thinking of10:20
fwereaderogpeppe, yeah, I definitely see that side of it too10:20
fwereaderogpeppe, but I'm not sure it's a very strong objection -- convince me?10:20
rogpeppefwereade: in many of the places, we already have the data in a file10:21
fwereaderogpeppe, in which case, yay, easy ReadSeeker10:21
rogpeppefwereade: buffering in Put would force us to make another copy of that before doing the Put10:21
fwereaderogpeppe, I'm only suggesting we should buffer ourselves if we can't ReadSeeker it10:21
rogpeppefwereade: oh i see, you mean "if it doesn't implement ReadSeeker, then pull everything into a buffer" ?10:21
fwereaderogpeppe, more ideally a temp file10:22
rogpeppefwereade: i don't really see the point.10:22
rogpeppefwereade: in all the places we have, it's trivial to make a ReadSeeker10:22
rogpeppefwereade: and i much prefer to avoid dynamic type checking magic where reasonable10:22
fwereaderogpeppe, so that we can inject the reliability as close to the problematic bit as possible, and so we don't push a new restriction into the API that I'm not sure is justified10:23
rogpeppefwereade: you're saying we shouldn't force all clients to buffer everything, but we're going to buffer everything anyway, right?10:24
rogpeppefwereade: for the rare (currently non-existent) case where the client doesn't have a seeker, it's easy for them to make one.10:24
rogpeppefwereade: and know about the trade-offs involved10:25
rogpeppefwereade: as i said, having the static type caught lots of cases where i could trivially use a readseeker but didn't. if i hadn't made that change, we'd be creating a temp file for each of those.10:26
fwereaderogpeppe, or maybe we'd be making use of the length param to decide whether or not to create one ;)10:26
rogpeppefwereade: that seems to me like lots of heuristics and extra code where currently we don't need any10:27
rogpeppefwereade: if we find lots of places that want to use an io.Reader, we can easily make a func PutBuffered(s Storage, name string, r io.Reader, length int64) error10:28
fwereaderogpeppe, isn't that because you're just pushing the responsibility onto the client?10:28
rogpeppefwereade: so it's obvious to the caller that we're going to be creating a temporary file10:28
rogpeppefwereade: sure. but creating a temporary file is something i think it's worth the client be aware of10:28
rogpeppefwereade: and it means we don't have to write more code now10:29
fwereaderogpeppe, possible compromise: at the point where you'd seek, just error out if it's not a ReadSeeker?10:30
rogpeppefwereade: that's worse, i think10:31
fwereaderogpeppe, automatic reliability in the cases we already have, no external code changes, no magic buffering10:31
fwereaderogpeppe, I guess there's something I'm missing10:31
fwereaderogpeppe, (ok, maybe we should check first to know where to seek back to)10:31
rogpeppefwereade: except that i want automatic reliability in all cases, and it's not hard10:32
rogpeppefwereade: i don't really see why there's a problem with requiring ReadSeeker. it loses no generality10:32
rogpeppefwereade: yes, that is also an issue with just dynamically type converting to ReadSeeker10:33
rogpeppefwereade: we'd have to say "*if* this is a ReadSeeker, it must be positioned at the start"10:33
rogpeppefwereade: that's ok i guess, but i'd much prefer to just require a ReadSeeker10:34
fwereaderogpeppe, the issue is that it's a restrictive interface change to all the providers to implement a change that's ec2-specific10:35
fwereaderogpeppe, I feel like it's ec2 driving the interface, not the other way round10:36
rogpeppefwereade: it's not necessarily ec2-specific. it applies to any provider where the request might encounter a transient failure10:36
fwereaderogpeppe, it is in practice ec2-specific... and any other provider that implements Put-reliability will need to make similar internal changes to those in ec2 regardless10:38
rogpeppefwereade: i agree. but if you've just got an io.Reader, you *cannot* retry a request - so regardless of the provider implementation, they'll want some way of rewinding the stream.10:40
rogpeppefwereade: so we're going to force all providers that want to retry to implement their own temp-file buffering stuff10:40
rogpeppefwereade: i'd prefer to keep that cleverness outside of the individual providers10:40
fwereaderogpeppe, the tradeoff is between asking a limited number of providers to do so, and asking every client to do so10:41
fwereaderogpeppe, our perspective on this may skewed because we are focused on writing one client, and we have 3+ providers to think about10:42
rogpeppefwereade: if it's just a difference between s.Put(r, name, len) and environs.PutBuffered(r, name, len) then i don't see the issue10:42
rogpeppePutBuffered(s, name, r, len) of course10:42
rogpeppefwereade: then the temp-file buffering is implemented in a single place, once and for all10:43
rogpeppefwereade: (and currently we don't even need it, because there's not a single place that's really streaming)10:43
fwereaderogpeppe, the question I am trying to get clarity on is: purely at the level of the interface, why is it better to have a ReadSeeker than a Reader?10:43
rogpeppefwereade: because a Reader allows streaming only, and here we can't stream10:44
fwereaderogpeppe, we can stream, it's just not necessarily so reliable10:44
rogpeppefwereade: and that's a good thing?10:45
fwereaderogpeppe, considering the capabilities of the various providers, I think it is all we can reasonably guarantee10:46
rogpeppefwereade: we can guarantee to make available to a provider some data that they can retry on. i think that's a nice thing to do. it makes the provider's job easier.10:47
rogpeppefwereade: your solution is asking me to put more intelligence and code in the provider, where actually we want to move as much as possible *outside* the provider10:48
fwereaderogpeppe, *we* can, yes -- but I thought we weren't writing the interfaces just for our own convenience?10:48
fwereaderogpeppe, I don't know how you are going to put S3-specific buffering outside the provider10:48
rogpeppefwereade: as far as i'm concerned, it's the job of that interface to make it as easy as possible to write a provider10:49
fwereaderogpeppe, the provider should absolutely contain *provider-specific* intelligence and code10:49
rogpeppefwereade: for clients, we can easily write convenience functions that wrap it10:49
rogpeppefwereade: agreed. but buffering the reader is not provider-specific. i'm not sure what you mean by "S3-specific buffering".10:49
rogpeppefwereade: another possibility is to pass a function (func() io.Reader) that can be called multiple times to get the source.10:53
rogpeppefwereade: that would make it easier for a client to do streaming.10:54
fwereaderogpeppe, does every provider have to implement this retry stuff?10:54
fwereaderogpeppe, it seems it is a provider-specific enhancement10:54
rogpeppefwereade: it's a good question. i suspect it's not unlikely.10:54
rogpeppefwereade: the point is that any provider that *does* want to implement retry *must* implement the same buffering code.10:55
rogpeppefwereade: and that client code must work against any provider10:55
rogpeppefwereade: i suppose your argument could be: for providers that *don't* implement retry, we'll be doing more work than necessary.10:56
rogpeppefwereade: assuming we actually have some client code that wants to stream.10:57
fwereaderogpeppe, well, we already did, even if it was just a smattering of test code10:57
rogpeppefwereade: really?10:58
rogpeppefwereade: i thought the changes were just changing bytes.Buffer to bytes.Reader in the main10:58
rogpeppefwereade: no extra buffering required10:58
fwereaderogpeppe, my argument is really that *most* providers don't retry, and that even if we get to a point where most *do* I would rather have a new provider able to implement the interface to the full more easily10:58
rogpeppefwereade: huh? this doesn't make it harder in the slightest for a new provider10:59
fwereaderogpeppe, rather than make Put-retries a fundamental requirement for a provider, I would like them to be an optional enhancement10:59
rogpeppefwereade: they are10:59
rogpeppefwereade: a provider can totally ignore the fact that it's a ReadSeeker10:59
fwereaderogpeppe, if an Environ is always required to retry, ReadSeeker makes sense10:59
fwereaderogpeppe, Storage sorry10:59
rogpeppefwereade: i don't see any requirement - it makes it *possible* for a provider to retry, but by no means mandatory11:00
fwereaderogpeppe, a Reader in the interface implies potentially-unreliable streaming, a ReadSeeker tenacious persistence11:01
fwereaderogpeppe, if the lowest common denominator is unreliable streaming, that is what we should advertise11:01
rogpeppefwereade: i don't see any difference in reliability between a Reader and a ReadSeeker11:01
rogpeppefwereade: they both implement io.Reader11:01
rogpeppefwereade: a seeker has no more reliability guarantees11:02
fwereaderogpeppe, if any, or all, providers want to get smarter and buffer, then they can do so and everyone who uses them gets to see the benefits transparently11:02
fwereaderogpeppe, I'm talking about the reliability of the operation, not its data source11:02
rogpeppefwereade: but we can implement this behaviour once and for all providers11:02
rogpeppefwereade: another possibility would be to hoist the retry logic outside the providers, but i think it's too inherently provider-specific11:03
rogpeppefwereade: and even then, we'd need a stream we could rewind11:04
fwereaderogpeppe, how about EnsureSeeker(Reader) (ReadSeeker, error), and fall back to only trying once?11:05
rogpeppefwereade: who calls that?11:05
fwereaderogpeppe, any provider that wants to add retry logic11:06
fwereaderogpeppe, they just have to be prepared for it to fail and fall back to non-retrying11:06
rogpeppefwereade: what would EnsureSeeker do other than .(io.ReadSeeker) ?11:06
fwereaderogpeppe, one day, maybe, buffer to a file if that fails11:07
fwereaderogpeppe, I think that's a cleaner separation of concerns11:08
rogpeppefwereade: i'd find it preferable for a client to explicitly disable retrying (for instance by passing in a ReadSeeker where the Seek always fails)11:09
rogpeppefwereade: then a client has to make a positive action to *disable* reliability, rather than the other way around11:09
fwereaderogpeppe, except in the cases where it isn't reliable11:09
rogpeppefwereade: func ErrorReadSeeker(r io.Reader) io.ReadSeeker11:09
rogpeppefwereade: sure, some providers just aren't reliable. but when we use a reliable provider, we are assured of reliability11:10
rogpeppefwereade: rather than, some time in the future "oh, that's failing because i forgot to use a bytes.Reader rather than a bytes.Buffer"11:11
rogpeppefwereade: and this question is currently academic - we have a seekable reader in every single case, trivially.11:12
fwereadeTheMue, https://codereview.appspot.com/8824043/ reviewed11:12
TheMuefwereade: cheers11:12
fwereaderogpeppe, if not all providers are reliable, why would we imply they are?11:12
rogpeppefwereade: we are not. we are implying that a provider *may* be reliable.11:13
fwereaderogpeppe, and placing the burden for supporting reliability on the *client*11:13
rogpeppefwereade: ... which is trivial!11:14
fwereaderogpeppe, despite the fact that it may not actually be present11:14
rogpeppefwereade: there is no burden11:14
rogpeppefwereade: honestly - how does it actually burden the client?11:14
fwereaderogpeppe, you just suggested ErrorReadSeeker11:15
fwereaderogpeppe, which STM to be boilerplate you expect every client to use if they don't know the provenance of their Reader11:15
fwereaderogpeppe, which, honestly, I do not think they should have to care about11:16
rogpeppefwereade: yes - i'd like each client to be fully aware when they might not get reliability against an otherwise bulletproof provider11:16
rogpeppefwereade: but currently there are *no* clients that would use it11:17
fwereaderogpeppe, IIRC Put returns an error11:17
fwereaderogpeppe, under-promise, over-deliver ;)11:18
rogpeppefwereade: this isn't promising anything, really.11:19
rogpeppe1) this does not require any providers to do anything different11:19
rogpeppe2) this enables a provider to implement retry easily11:19
rogpeppe3) there is no significant impact on how easy it would be to write a client that uses the Storage.11:19
rogpeppei really can't see a down side11:19
fwereaderogpeppe, *every* provider needs to Read; *some* providers may be able to be smarter if they can also Seek11:21
rogpeppefwereade: sure.11:21
fwereaderogpeppe, this seems to be exactly why we can do `rs, ok := r.(io.ReadSeeker)`11:22
rogpeppefwereade: and *every* client needs to work with *every* provider11:22
rogpeppefwereade: so what the client provides will enable those *some* providers to be smarter.11:22
rogpeppefwereade: if there was a significant difficulty for clients to provide a Seeker, i'd agree with you.11:23
rogpeppefwereade: but really, there's not AFAICS.11:23
fwereaderogpeppe, whether or not a given Reader is also a Seeker is not something clients should have to think about11:23
rogpeppefwereade: it really is - a client should be aware that by using a reader that's not a seeker it is making the operations more unreliable on some providers.11:24
rogpeppefwereade: and we can let the compiler tell us that11:24
rogpeppefwereade: i think that's awesome11:24
* rogpeppe really doesn't like dynamic type checks where they're easily avoided.11:25
fwereaderogpeppe, you're proposing that introducing a magic implicit reliability switch will make people's lives *easier*?11:25
fwereaderogpeppe, the client should not get to affect that choice11:26
rogpeppefwereade: it's not magic. doing a dynamic type check under the hood to see if the client *might* have turned on that switch *is* magic.11:26
fwereaderogpeppe, how does it affect the client, except to cause fewer errors?11:27
rogpeppefwereade: fewer errors is a huge thing11:27
fwereaderogpeppe, yes11:27
fwereaderogpeppe, and a good thing11:28
fwereaderogpeppe, that the client doesn't have to think about11:28
rogpeppefwereade: i'm glad we agree on something :-|11:28
rogpeppefwereade: indeed - the io.ReadSeeker contract implies that they get reliability for free11:28
rogpeppefwereade: and is trivial for all current (and probably most future) clients to provide11:29
rogpeppefwereade: that's really the crux - if that's not true, then i think you're right.11:29
fwereaderogpeppe, saying "you have to provide a readseeker, and hence reliability, but it might not be used" is not better than saying "just give us a Reader -- if it's also a ReadSeeker, you might see some benefits on some providers"11:30
rogpeppefwereade: i think it is11:30
rogpeppefwereade: because it means that the compiler tells us where things might be unreliable.11:31
rogpeppefwereade: so we *know* that in all current cases, you *will* see benefits on the providers that can use it11:31
rogpeppefwereade: and when making this change, i was surprised by the places that it told me11:32
rogpeppefwereade: and they were all trivially changeable to make work11:32
dimiternjam: you around?11:32
rogpeppefwereade: i think that's a concrete benefit11:32
fwereaderogpeppe, you'll have to explain further11:33
rogpeppefwereade: so, when i changed the signature, i ran the build11:33
rogpeppefwereade: and it said, in various places "that argument does not implement io.ReadSeeker"11:33
rogpeppefwereade: and i went there11:33
fwereaderogpeppe, so, it doesn't get to be reliable on ec2, just like it doesn't everywhere11:34
rogpeppefwereade: and, lo! it was using bytes.Buffer not bytes.Reader11:34
wallyworlddimitern: https://code.launchpad.net/~wallyworld/juju-core/openstack-image-lookup/+merge/15930111:34
jamdimitern: I'm back11:34
rogpeppefwereade: i don't understand that11:34
fwereaderogpeppe, the interface should reflect the minimum set of required capabilities11:35
fwereaderogpeppe, if I've just got a Reader I *know* its potentially unrepeatable anyway11:35
fwereaderogpeppe, you're asking me to turn it into a ReadSeeker that you might not bother to use11:36
fwereaderogpeppe, how does it help?11:36
fwereaderogpeppe, OTOH transparently making use of reliability, or injecting it ourselves if it's really critical, frees the client to solve their own problem11:37
rogpeppefwereade: we require that code to run against every provider. we *know* that when we run with the ec2 provider that the ReadSeeker *will* be used (assuming an operation fails in a transient way)11:37
fwereaderogpeppe, why do all the clients for all the other providers have to care?11:37
rogpeppefwereade: there is no client "for another provider". a client is for *all providers*11:38
fwereaderogpeppe, you have been talking like the client knows its using ec211:38
rogpeppefwereade: we are indeed writing code that we know will be used against ec2 (and all the other providers too)11:39
fwereaderogpeppe, the *all providers* context should only make it more starkly clear that we are messing with a perfectly good interface so we can show off a special feature of a single provider11:39
rogpeppefwereade: on the contrary, i think that the ec2 requirement shows the inadequacy of the interface11:40
rogpeppefwereade: and i'm sure there will be other providers that will want to retry a Put too.11:42
rogpeppefwereade: by making the change now, we make it easy for them to do so11:42
fwereaderogpeppe, AFAICT the only "inadequate" thing about the interface is that it requires that you do about the most innocuous possible type check check11:45
fwereaderogpeppe, in order to add *provider-specific* functionality11:45
fwereaderogpeppe, ReadSeeker makes no sense unless reliability-vs-transient-errors is in the contract of the Storage type11:46
rogpeppefwereade: the inadequate thing is that it's not obvious to people using that interface that if the *happen* to pass a ReadSeeker in, that somehow things will magically get more reliable on some providers.11:46
fwereaderogpeppe, but you seemed to be saying that people would usually be using ReadSeekers anyway11:47
rogpeppefwereade: as far as i'm concerned, the contract of the Storage type is "do whatever you can to try and fulfil these requests"11:47
rogpeppefwereade: not really - i'm saying that people would usually be using some that could trivially *be* a ReadSeeker11:48
fwereaderogpeppe, so you're breaking the contracts of maas and openstack,because they don;t do that11:48
rogpeppefwereade: not at all11:48
rogpeppefwereade: perhaps they don't get transient errors11:48
fwereaderogpeppe, then they *definitely* don;t need ReadSeekers!11:48
rogpeppefwereade: that's fine. all they need is the reader bit then.11:49
rogpeppefwereade: there's no requirement to use the seeker part11:49
fwereaderogpeppe, then why would we require that it be supplied?11:49
rogpeppefwereade: ec2 won't use the seeker part unless something fails with a certain kind of error11:49
rogpeppefwereade: because thing client needs to supply a reader for *all* providers!11:49
fwereaderogpeppe, and it does11:50
fwereaderogpeppe, and ec2 would make use of it if you would just make use of the language feature designed explicitly for that purpose11:50
fwereaderogpeppe, type casts and types switches were not just put in for fun11:50
rogpeppefwereade: *and* if clients actually passed in a readseeker11:51
fwereaderogpeppe, we don;t get to control our inputs and probably nor do they11:51
fwereaderogpeppe, *they* then have to advertise ReadSeeker11:51
rogpeppefwereade: when would it ever *not* be appropriate for a client to pass in a ReadSeeker?11:51
fwereaderogpeppe, all the places you changed, for a start11:53
rogpeppefwereade: in that branch, there are various places where the changed code does "bytes.NewReader(buf.Bytes())". that code is totally non-obvious if the argument to Put is just a Reader. but it has significant runtime implications.11:53
fwereaderogpeppe, being?11:55
rogpeppefwereade: that when running against ec2 the operation will be less reliable if you don't11:56
rogpeppefwereade: essentially we want all our client code to work as well as possible against all providers. that means that the client code should work as well as possible against *at least* the ec2 provider. so *all* clients should provide the thing that makes ec2 work well - i.e. a ReadSeeker. given that, why not make the interface appropriate to that?11:57
* rogpeppe thinks it might be possible to phrase most of that in mathematical notation11:58
wallyworldfwereade: are you free sometime soon for a meeting with me and dimitern ?11:59
wallyworldsoon = next 30 minutes ?11:59
dimiternfwereade: about openstack constraints/images/flavors selection11:59
fwereadewallyworld, dimitern: sure, I can do now12:02
dimiternfwereade: ok, I'll start a g+12:02
dimiternhttps://plus.google.com/hangouts/_/edc5333a9131548ea93258b2c5c90f6a9ef4af15?authuser=0&hl=en << fwereade, wallyworld12:03
wallyworldtrying, not connecting12:04
dimiternwallyworld: william was having problems with sound12:06
jamwallyworld: as a side thing, when you know your holidays, try to put them on the juju team calendar. I think I did it correctly for you this time.12:13
wallyworldjam: i did add them12:13
wallyworldthe week before i left12:13
jamwallyworld: I mean the ones you just applied for national holidays in April and June12:14
wallyworldnp sorry12:14
jamapril 25th june 10th I think12:14
rogpeppefwereade: "Why aren't we doing this at the end of cmd.Main? We already get a log message on error."12:48
rogpeppefwereade: that's the way i tried to do it first12:48
rogpeppefwereade: but unforunately it breaks the jujuc log command12:48
rogpeppefwereade: we really don't want jujuc log to say "command finished" every time it runs.12:49
rogpeppejam: "12:52
rogpeppeplatforms allow renaming an empty dir over another empty directory.12:52
rogpeppei thought the whole point of using directories in fslock was that that wasn't the case12:52
rogpeppefwereade: if that *is* the case, then fslock is broken AFAICS12:53
* dimitern bbiab13:01
TheMuefwereade: the propose is in again13:06
* TheMue is at lunch13:06
fwereaderogpeppe, I'm not at fslock properly yet13:07
rogpeppefwereade: ok13:07
rogpeppefwereade: a review from you would be good before it goes in13:08
fwereaderogpeppe, re jujuc: hmm, I'm not so sure jujuc logging is entirely a misfeature13:08
fwereaderogpeppe, if anything I'd say "good point, add start logging to jujuc commands"13:09
fwereaderogpeppe, I would expect that in the eventual case that would be logged at a lower level than that specified for juju-log13:09
rogpeppefwereade: the problem is the jujuc *log* command. do we really want that to produce two lines of log for every line the user intended to log?13:10
rogpeppefwereade: i thought that the level was one of the arguments to juju-log13:10
rogpeppefwereade: well, one that we currently discard, i'm aware13:10
rogpeppefwereade: but given that we have those levels now, i think it probably should support that13:11
fwereaderogpeppe, it is one of the arguments to juju-log, and ofc it should support that13:11
fwereaderogpeppe, but that's the level at which the supplied message should be logged, and it's independent of the level at which we log command completion13:12
rogpeppefwereade: so if i do juju-log --level=debug foo, do i really want to see two lines at that level in the log?13:12
rogpeppefwereade: if you think that's ok, then i'll go with logging in cmd.Main. i didn't think it was, which was why i changed it.13:13
fwereaderogpeppe, yes, I think so, because if we unfuck juju-log we will also, I imagine, badge its output clearly such that we can filter that stuff easily13:13
fwereaderogpeppe, might be wrong13:13
rogpeppefwereade: i'm just imagining a scenario where the user is producing *lots* of output using juju-log. we just doubled the number of lines produced13:14
rogpeppefwereade: and log file size is actually a significant issue for us.13:14
fwereaderogpeppe, how much would we save if we dropped the logging in state.Open except (1) before the dial func and (2) on errors inside the dial func?13:16
fwereaderogpeppe, saving 240 lines/agent/hour will probably leave us some wiggle room:)13:17
rogpeppefwereade: i don't know. i have thrown away the last huge log file i acquired13:17
fwereaderogpeppe, not as much help as silencing the txn spam, I agree13:18
rogpeppefwereade: i just think it's a bit weird that a command whose sole purpose is to produce a line of log output actually produces two.13:18
rogpeppefwereade: well, probably more still actually, as the uniter probably logs when it gets a jujuc command excecution request13:19
rogpeppefwereade: i think the changes to logging in state.Open sound reasonable.13:20
mrammmorning all (well early my morning)13:20
rogpeppemramm: hiya13:20
fwereaderogpeppe, I think that the general "it's good to know what commands we run" principle *probably* beats the (otherwise clearly sane) juju-log consideration13:20
rogpeppemramm: i won't be able to join the kanban meeting today as my broadband connection is out13:21
rogpeppefwereade: i think i agree. but the "finished" message perhaps doesn't13:22
fwereaderogpeppe, sorry, the agent finished messages?13:22
mrammbummer that13:22
rogpeppefwereade: the "finished" message in cmd.Main13:22
rogpeppemramm: you never know, the engineers might turn up (chortle, chortle)13:23
fwereaderogpeppe, I don't see one13:24
fwereaderogpeppe, I see"command failed"13:24
rogpeppefwereade: that's the one you're suggesting13:24
rogpeppefwereade: "Why aren't we doing this at the end of cmd.Main?"13:24
fwereaderogpeppe, and I just suggested an alternative: in SuperCommand13:24
rogpeppefwereade: orly? i don't think i've seen that13:25
fwereaderogpeppe, or... apparently I didn't13:25
fwereaderogpeppe, the insanity peppers must be kicking in13:25
fwereaderogpeppe, or maybe I'll discover it in the wrong buffer somewhere in an hour13:25
* fwereade pokes himself with something sharp13:25
rogpeppefwereade: that happens to me too often13:26
rogpeppefwereade: why would doing it in SuperCommand help? doesn't jujuc use SuperCommand?13:26
fwereaderogpeppe, I *think* niemeyer talked me out of it, let me check13:26
rogpeppefwereade: you're right, i don't think it does13:27
rogpeppefwereade: still seems a bit iffy to me. we're logging in SuperCommand because use of SuperCommand *happens* to correspond exactly with the commands we want to log finished messages for.13:28
fwereaderogpeppe, feels a bit icky but maybe not too much so13:28
fwereaderogpeppe, because SuperCommand at least does know about logs13:28
fwereaderogpeppe, and in general I don't think we should expect either main.Main *or* cmd.Main to have logging configured13:28
rogpeppefwereade: and juju doesn't?13:28
fwereaderogpeppe, not much point logging if you don't have a target IMO13:29
rogpeppefwereade: true, but no harm either, no?13:29
fwereaderogpeppe, message that looks like it should be printed but actually isn't13:29
rogpeppefwereade: true of all log messages...13:29
fwereaderogpeppe, I think that , all other things being equal, it is better to write a given message when you know that logging setup has occurred, and to write important messages elsewhere if you can't be sure13:31
fwereaderogpeppe, but that is kinda a derail13:31
fwereaderogpeppe, how do you feel about dropping it from cmd.Main and logging success/failure in SuperCommand?13:31
fwereaderogpeppe, independent of reasons I might like it ;)13:31
rogpeppefwereade: that seems reasonable to me. i'd forgotten that logging was so tightly bound up with SuperCommand.13:32
fwereadeTheMue, https://codereview.appspot.com/8824043/ LGTM13:38
TheMuefwereade: yeah, just read it, thank.13:39
fwereadedimitern, rogpeppe, jam, anyone: a second look at that would be handy; I am not entirely happy about some of the code but it produces the correct output and that's the critical thing at this point13:39
TheMuefwereade: the deleting of the units of subordinate service is done this way in py too. the data is used before to add the unit info to the principal units.13:41
rogpeppefwereade, TheMue: looking13:42
fwereaderogpeppe, https://codereview.appspot.com/8821043/ LGTM trivial13:42
rogpeppefwereade: ta!13:43
* rogpeppe is trying hard to grok the code in processServices13:48
TheMuerogpeppe: maybe i should move those loops into two proprocessing funcs with a speaking name13:49
rogpeppeistm that it should be possible to get the type system working for us there, rather than fighting it in every line13:49
TheMuerogpeppe: it's only a mix of the collected data into the output13:49
TheMuerogpeppe: especially the lower loop is exactly how it is done today in py13:50
rogpeppeTheMue: the difficulty i have is that we're spending a lot of code dynamically type casting stuff that perhaps we can already know the types of.13:50
TheMuerogpeppe: but this map[string]interface{} indeed complicates it *sigh*13:50
TheMuerogpeppe: yes, i'm not happy about it too13:50
TheMuerogpeppe: but regarding the feature freeze i think we should change this afterwards13:51
rogpeppeTheMue: at the least it could do with some comments so we know what "post-processing the subordinates" is actually doing.13:52
TheMuerogpeppe: will add them13:52
=== wedgwood_away is now known as wedgwood
=== wedgwood is now known as Guest86505
rogpeppeTheMue: what do subFromMap and subToMap do?13:59
TheMuerogpeppe: will find better names. the subToMap had its name from collecting the service names for the output "subordinate-to". ;)14:00
rogpeppeTheMue: what's are the keys and what to the keys map to?14:01
rogpeppes/to the/do the/14:01
TheMuerogpeppe: they do hangout ;)14:01
rogpeppeTheMue: given that the data types are so dynamic, we really need more comments14:01
TheMuerogpeppe: i'll add comments and change the names14:01
rogpeppehmm, the link box says it's got a link, though the phone's still dead. will try broadband again.14:02
rogpeppe1fwereade: hmm, i'm not sure if it's working14:05
rogpeppe1very limited bandwidth!14:06
rogpeppe1dimitern: i saw you for a moment...14:06
=== wedgwood_ is now known as wedgwood
TheMuefwereade: is it ok to move the status in with your comments handled and improvements in readability and comments?14:34
fwereadeTheMue, has anyone else at least glanced at it?14:35
fwereadeTheMue, I care a lot more about output than about readability though :)14:35
TheMuefwereade: roger is looking, and i could as dimitern14:36
fwereadeTheMue, but yes, you have my LGTM, just get someone else's too :)14:37
TheMuefwereade: ok, will do14:37
rogpeppe1interesting, bandwidth tester indicates 1Mb download (not too bad) but 52Kb upload. not surprising that the hangout worked ok when i turned off my vid and microphone14:38
dimiternTheMue: you really hate merging conflicts it seems ;)14:39
TheMuedimitern: yeah, really, i do14:39
TheMuedimitern: and also i got my first lgtm ;)14:39
dimiternTheMue: no worries, it'll be easier for me as well once you land your stuff14:40
TheMuedimitern: great. had a chance to take a look at it?14:41
dimiternTheMue: just a brief glance14:42
TheMuedimitern: can i charm you a lgtm? ;)14:43
dimiternTheMue: in a bit, just looking through cards and bugs to make sure we're in sync14:44
TheMuedimitern: just wait, a new propose will fly in in a few moments14:52
dimiternTheMue: sure, np14:52
TheMuefwereade: ping14:54
fwereadeTheMue, pong14:55
TheMuefwereade: your comment in the status_test, line 842, how do you think it shall work?14:56
fwereadedimitern, rogpeppe1, TheMue: trivial if anyone wants it: https://codereview.appspot.com/876804514:56
TheMuefwereade: i'm lost with this comment.14:56
dimiternfwereade: LGTM, trivial14:56
TheMuefwereade: LGTM14:57
fwereadethanks guys14:57
TheMuefwereade: i thought i followed your first idea, but it seems that this has been wrong14:57
fwereadeTheMue, I mean that adding a relation is a prereq of adding a subordinate, and that won't (shouldn't ;)) work if you add a subordinate of service S to more than one unit of service T14:58
fwereadeTheMue, it's a good start14:58
fwereadeTheMue, but relateServices already exists and shouldn't be duplicated14:58
fwereadeTheMue, while adding the subordinate is an operation on an existing unit and an existing service14:59
TheMuefwereade: ah, ic14:59
fwereadepopping out for a mo, bbiab15:01
benjigary_poster/rick_h_: yeah, I'm looking at it15:13
gary_posterbenji, hi channel jumper15:13
TheMuedimitern: https://codereview.appspot.com/8824043/ is back in again15:26
dimiternTheMue: I'm on it15:26
dimiternTheMue: you got LGTM from me15:33
dimiternTheMue: please go ahead an land it, so I can continue on mine15:34
TheMuedimitern: thx, will do15:34
TheMueanyone experiencing the same bootstrap_test problems like me? i just merged the trunk and since then the test fails15:48
dimiternTheMue: what's you series?15:52
dimiternTheMue: i'm on quantal - jut got latest trunk and running tests now; haven't seen this issue before though15:53
fwereadeTheMue, if you're seeing https://bugs.launchpad.net/juju-core/+bug/1169826 then I need help tracking it down please :)15:56
TheMuefwereade: hmm, no, i don't get a panic15:57
dimiternI also didn't get a panic, but got this error in environs/openstack: http://paste.ubuntu.com/5716314/15:58
TheMuefwereade: it's in bootstrap_test line 10515:58
TheMuefwereade: c.Check(urls, HasLen, len(test.uploads))15:58
TheMuefwereade: if i get it right it's one url instead of the expected two15:59
fwereadeTheMue, expected/actual? which test?16:00
dimiternand i can see the cmd/juju tests almost doubled in running time the past 2 weeks16:00
TheMuefwereade: cmd/juju/bootstrap_test.go line 10516:01
fwereadedimitern, that's because we're actually testing stuff16:01
fwereadeTheMue, that's a table test, the line number tells me very little16:01
dimiternfwereade: that's good then :)16:01
fwereadedimitern, ha, I can repro yours (one of them anyway) if I edit /etc/lsb-release16:02
TheMuefwereade: does "test 10: --upload-tools always bumps build number" help you more?16:02
fwereadeTheMue, helpful, tyvm16:02
fwereadeTheMue, dimitern, sorry I have to take a break :/ back soon16:02
TheMuefwereade: as my CL doesn't cover this is it ok to still submit it?16:03
dimiternfwereade: that's awesome, I'll remember this trick to change current series16:03
dimiternTheMue: submitting onto a broken trunk is a no no16:03
hazmatods keynote livestream fwiw http://openstackportland2013livestream.eventbrite.com/16:04
TheMuedimitern: ok, so status has to wait16:05
dimiternhazmat: it says it's ended, but I can't see a link to the video..16:05
hazmatdimitern, by bad.. better link.. http://www.openstack.org/16:06
dimiternhazmat: cool, 10x!16:07
hazmatdimitern, mark's keynote is in +1hr 15m16:08
dimiternTheMue: just dump the whole error output + logs and paste it please16:08
=== BradCrittenden is now known as bac
dimiternhazmat: isn't it streaming live now? I can see it has started..16:11
hazmatdimitern, the keynotes have started, but there are some other ones scheduled first16:11
dimiternhazmat: ah, ok, cheers16:12
dimiternfwereade: I can reproduce the error I posed above consistently16:14
TheMuedimitern, fwereade: http://paste.ubuntu.com/5716425/16:42
dimiternTheMue: cheers16:46
* TheMue has to take a break16:59
=== TheMue_ is now known as TheMobileMue
=== deryck is now known as deryck[lunch]
dimiternmark's keynote is staring now17:23
rogpeppe1dimitern: link?17:50
dimiternrogpeppe1: http://www.openstack.org/home/Video/17:50
rogpeppe1dimitern: is it really not possible to find out a service's subordinate or principal without looking at its units?17:53
dimiternrogpeppe1: I don't think you can17:54
rogpeppe1dimitern: seems a bit weird. so if there are no units, there's no subordinate-principal relationship between two units?17:54
rogpeppe1s/two units?/two services?/17:54
dimiternrogpeppe1: you can take a look at the service's charm as well17:55
rogpeppe1dimitern: how does that help?17:55
rogpeppe1dimitern: presumably you actually need to look at the relations17:55
dimiternrogpeppe1: if the service is running a subordinate charm, then all it's units are/will be subordinates17:55
rogpeppe1dimitern: yes, but for status we need to find out a *service's* principal service, irrespective of units, no?17:56
dimiternrogpeppe1: well, from the list of relation a subordinate service is participating in then17:57
dimiternrogpeppe1: relations*17:57
rogpeppe1dimitern: yup. i think that if we see that there's a relation between two services and scope is container, then we know17:58
dimiternrogpeppe1: makes sense, yeah17:59
rogpeppe1niemeyer: is there any way of telling goyaml to ignore a field when marshalling? (unexported fields seem to cause an error)18:00
niemeyerrogpeppe1: unexported fields shouldn't cause errors18:01
niemeyerrogpeppe1: Example?18:01
rogpeppe1niemeyer: one mo18:01
rogpeppe1niemeyer: http://paste.ubuntu.com/5716622/18:03
rogpeppe1niemeyer: one mo, perhaps i'm running an old version of goyaml18:03
rogpeppe1niemeyer: when i run that code, i see "error: YAML error: reflect.Value.Interface: cannot return value obtained from unexported field or method"18:03
niemeyerrogpeppe1: Haven't made any changes about that18:03
niemeyerrogpeppe1: Will test18:03
niemeyerrogpeppe1: That's a bug18:03
niemeyerrogpeppe1: Will fix18:03
rogpeppe1niemeyer: ah, ok18:04
rogpeppe1niemeyer: there's another bug i just found too18:04
niemeyerSurprising to see that broken18:04
rogpeppe1niemeyer: i haven't looked into the source, but it seems like it might be hashing types by name somewhere18:05
niemeyerI'd have guessed someone would have reported by now18:05
niemeyerrogpeppe1: Can't tell what that means18:05
rogpeppe1niemeyer: because if you use two function-scoped types of the same name, goyaml gets confused18:05
niemeyerrogpeppe1: Sorry, I still have no idea about what you mean.. an example helps18:05
rogpeppe1niemeyer: ok, here's an example (slightly bigger than it could be, but it's the stage i got to when i realised i knew what the problem was:18:07
rogpeppe1niemeyer: http://paste.ubuntu.com/5716642/18:07
* rogpeppe1 has a very dodgy network connection currently18:08
rogpeppe1niemeyer: search for "noMethods" in that code18:08
niemeyerrogpeppe1: Ok?18:09
rogpeppe1niemeyer: sorry, i'm just finding the output18:12
rogpeppe1niemeyer: ok, this is a slightly smaller example: http://paste.ubuntu.com/5716665/ and this is its output: http://paste.ubuntu.com/5716667/18:16
rogpeppe1niemeyer: the fields showing under exposed-service/0 are from the serviceStatus type, not from the unitStatus type as they should be18:17
rogpeppe1niemeyer: for example "charm" is showing the field from slot 1 of the unitStatus struct18:18
niemeyerrogpeppe1: It looks correct given the code18:19
niemeyerfunc (s serviceStatus) GetYAML() (tag string, value interface{}) {18:20
niemeyertype noMethods serviceStatus18:20
niemeyerreturn "", noMethods(s)18:20
niemeyerrogpeppe1: serviceStatus *is* returning a serviceStatus18:20
rogpeppe1niemeyer: and unitStatus is returning a unitStatus18:20
niemeyerrogpeppe1: Yeah, and how will it ever get into a unitStatus?18:20
rogpeppe1niemeyer: but goyaml thinks that the unitStatus returned (well actually a noMethods value) is a serviceStatus18:21
niemeyerrogpeppe1: let me run the code here rather than guessing.. hold on18:22
niemeyerrogpeppe1: Yeah, there's a bg18:23
rogpeppe1niemeyer: indeed :-)18:24
niemeyerrogpeppe1: I actually improved that logic a long time ago in bson, and forgot to implement it in goyaml18:24
niemeyerrogpeppe1: Easy to fix, though18:24
rogpeppe1niemeyer: cool18:24
=== deryck[lunch] is now known as deryck
niemeyerrogpeppe1: Both fixed and pushed.18:49
rogpeppe1niemeyer: woah!18:50
rogpeppe1niemeyer: nice one!18:50
niemeyerrogpeppe1: I already had the context for the latter issue, and the former was a silly mistake18:51
rogpeppe1niemeyer: at some point, it would be nice for goyaml to support `goyaml:"-"` like json does18:52
=== TheMobileMue_ is now known as TheMobileMue
niemeyerrogpeppe1: Done and pushed. Next? :)18:55
* rogpeppe1 can't think of any more at the moment :-)18:56
niemeyerrogpeppe1: Cool, thanks for the reports18:57
rogpeppe1niemeyer: np. i was also surprised by the unexported field error, BTW.18:58
niemeyerrogpeppe1: I found a better naming for the flusher stuff, btw, which made me happier as well.18:58
niemeyerrogpeppe1: v2 is coming18:58
rogpeppe1niemeyer: great!18:58
rogpeppe1niemeyer: new name is?18:58
niemeyerrogpeppe1: type Task interface { Run; Kill }18:58
rogpeppe1niemeyer: perfect18:59
rogpeppe1niemeyer: that maps much better to how i came to understand it18:59
niemeyerrogpeppe1: Yeah, it maps better to the overall problem indeed.. and, funny enough, it was the previous name19:00
niemeyerrogpeppe1: But there were several refactorings after that which made things flow back and forth19:00
rogpeppe1niemeyer: i know how that goes...19:01
niemeyerrogpeppe1: The current design was finally sound, but the naming wasn't perfect anymore19:01
rogpeppe1niemeyer: overall it's a nice small package. i really like it.19:01
niemeyerrogpeppe1: Nice, I'm glad to hear it19:01
* TheRealMue goes to bed now, good night20:18
fwereadethumper, heyhey20:44
thumperhi fwereade20:44
fwereadethumper, I WIPped most of your fslock branches but there's only one serious question at the root of it, and then a feature we'd all like that will hopefully not hurt too much ("message" or similar on acquire)20:45
thumperfwereade: ok, what is the one serious question?20:45
fwereadethumper, the latter will enable units to reliably break their own locks, which I think is a Good Thing20:45
fwereadethumper, if some FSs can rename over empty dirs, shouldn't be we breating the held file before moving the dir into place?20:46
fwereadethumper, or does that break something I'm not seeing?20:46
thumperactually that makes a lot of sense, and yes, I'm +1 on that20:46
fwereadethumper, sweet20:47
mgzdumb go question, I have n *int, how do I do (n != nil ? *n : 0) idiomatically?20:49
mgzthis is to fill in a bunch of struct fields, so I really don't want a three line if block per one...20:49
fwereademgz, func valueOrZero(*int) int, I suspect20:52
fwereademgz, nothing neater springs to mind20:52
thumperfwereade: reading your review comment, how would you confirm that you didn't take a lock while someone else had it?  when running in a go routine, repeated?21:07
fwereadethumper, fist thought, may not be very smart, send a bunch of "acquired" and "released" thingies down a channel from n goroutines and check that you always get true/false/true/false/true/false21:09
fwereadethumper, can't think of anything obviously wrong with it though21:10
fwereadethumper, acq1/rel1/acq3/rel3/acq3/rel3/acq1/rel1 etc21:11
* thumper nods21:11
thumperI get the feeling that I should smash the pipes together, and have a single proposal...21:12
fwereadethumper, I'd be fine with that I think21:12
fwereadethumper, (oh, I would also really like tests for the uniter)21:12
thumperfwereade: yeah, read that, will look into it21:12
thumperfwereade: is there any way to nicely simulate a crash during hook execution?21:13
thumperfwereade: actually, we can trivially have a test that has a lock for that uniter21:13
fwereadethumper, not nicely -- I think best to create the lock dir really21:13
thumperfwereade: and we can check when we create one, it unlocks it21:13
fwereadethumper, exactly21:13
* thumper nods21:13
thumperfwereade: also, I don't like the message being the held file...21:14
thumperfwereade: feels wrong some how21:14
thumperfwereade: how do I check I own it? before unlocking21:15
thumperif we use message?21:15
thumperI'll update the code to use atomic writes for message21:15
fwereadethumper, message doesn't have to be the held file, you can create the two indpendently before the copy21:15
thumperand have an expected message on Lock21:15
thumperfwereade: jam was suggesting to use the message for held21:15
thumperjust saying I'm -1 on that21:16
fwereadethumper, yeah, I rather like the nonce in held21:16
fwereadethumper, there is I think a meaningful distinction between "did this identity create the lock" and "did this specific chunk of memory create the lock"21:17
fwereadethumper, so, yeah, I don't think it's necessary to conflate the two21:17
* thumper nods21:17
thumperfwereade: one problem with logging every time you can't get a lock21:18
thumperfwereade: when trying, is that retries are every second (by default)21:18
thumperfwereade: and hook execution could be a while21:18
fwereadethumper, ok, that's too many21:18
thumperin the order of minutes21:18
thumperfwereade: how about writing it the first time21:19
fwereadethumper, (not too many checks, just too many messages)21:19
fwereadethumper, first time is fine by me21:19
thumperfwereade: and each subsequent time the message changes?21:19
fwereadethumper, ah, yeah, nice21:19
thumpershould give an obvious, not too invasive trail21:19
fwereadethumper, and *maybe* one every 120 times though the loop or something?21:20
thumperfwereade: I'd rather use time than a loop counter21:20
thumpersynchronization is hard21:21
* thumper recalls his talk on "multithreading 101"21:21
fwereadethumper, follow your heart :)21:21
thumpergave that talk at a conference 7 years ago21:21
thumperI thought I knew heaps about it, in the end, I learned so much researching my talk it scared me21:22
fwereadeI know enough to be scared in general21:22
fwereadeI have learned a lot over the years but somehow the amount I know remains characterizable by "enough to be scared"21:22
thumperI'm busy downloading sabdfl's openstack keynote in the background21:23
thumperyoutube keeps killing it watching live21:23
fwereadeit's on youtube, I'm listening to it21:23
fwereadeah it's only fallen over once for me21:23
thumperI had a huge amount of fun writing some lock free algorithms21:23
rogpeppe1thumper: i might be tempted to use atomic.AddInt32 to check that two things aren't holding a lock at the same time21:23
thumperafter the third time it fell over, I decided o download it21:23
thumperrogpeppe1: can you explain more?21:24
thumperI don't quite get it21:24
thumperoh, so increment on lock21:24
thumperand decrement on unlock?21:24
rogpeppe1thumper: yup21:24
thumperand assert values21:24
rogpeppe1thumper: and after the increment, you look at the value returned21:24
rogpeppe1thumper: if >1 you've fucked up21:24
* thumper nods21:24
* fwereade expected rogpeppe1 to show up and explain how to do it better , cheers :)21:24
thumpersounds like a plan21:24
rogpeppe1thumper: also, i'd change the retry interval to zero when doing that test21:25
rogpeppe1thumper: all the better to stress with21:25
thumperrogpeppe1: ok, sounds good21:26
rogpeppe1thumper: what platform are we worried about not having O_EXCL, BTW?21:30
thumperrogpeppe1: I don't have an explicit reason, just went with what bzrlib had because I had talked to lifeless a lot about it, and it is convenient to add informational messages to21:30
rogpeppe1thumper: ok. messages would be just as convenient with a file though, i think, no?21:32
thumperI'm not familiar with O_EXCL21:32
thumperbut I trusted the giant who came before me :)21:32
rogpeppe1thumper: :-)21:32
rogpeppe1thumper: bzr probably needs to run on way more platforms than us, but i suppose it can't harm21:33
* thumper nods21:33
rogpeppe1thumper: i'm just slightly concerned about EBS network traffic though, when we could really do it all locally.21:34
thumperwhat EBS network traffc?21:34
rogpeppe1thumper: doesn't disk storage on amazon instances with EBS go across the network?21:34
thumperwell, we are storing in the agent datadir, where it puts tools21:35
thumperwe could do it anywhere21:35
thumper/var/run/juju would make sense to me21:35
rogpeppe1thumper: /tmp ?21:35
thumperI would assume some of these would be local21:35
thumperno, /tmp is fugly21:35
thumper/var/run was made for this reason21:36
* rogpeppe1 has never heard of /var/run21:36
fwereadethumper, btw, I forget, do you definitely create the temp dirs on the same filesystem for actual atomic renames?21:36
thumperfwereade: I was assuming that /tmp and the juju data dir were on the same filesystem...21:37
thumpercould create a temp filename in the lock dir to be sure21:38
fwereadethumper, +121:38
thumperbut I thought that was a bit messy21:38
fwereadethumper, subdir with invalid lock name?21:38
thumper.nonce or something21:38
thumperwhere nonce is the hex nonce21:38
* thumper will work something out21:39
* thumper goes to make a coffee and toast21:39
fwereaderogpeppe1, thumper: can I get a quick look at https://codereview.appspot.com/8804044/ please?22:27
* thumper looks22:27
fwereaderogpeppe1, thumper: I promise I was using quantal for some of my testig across the pipeline, but clearly not quite as much as I thought I had22:27
thumperfwereade: given, with trivial22:29
fwereaderogpeppe1, thumper: the only other quantal complaints I am aware of are (1) verified transient, fixed by completing pipeline and (2) cannot repro at all -- that's a *weird* error in UpgradeJujuSuite that I propse to ignore until I can squeeze more details out of themue22:29
fwereadethumper, tyvm22:30
rogpeppe1fwereade: looking22:30
rogpeppe1fwereade: PTAL  https://codereview.appspot.com/884204322:32
rogpeppe1thumper: would appreciate it if you could take a look too22:33
* thumper looks22:33
rogpeppe1fwereade: reviewed22:33
* rogpeppe1 has to go to bed very soon22:39
thumperrogpeppe1: +122:43
rogpeppe1thumper: thanks22:44
thumperrogpeppe1: personally I wouldn't have had map[string]map[string]interface{}, but instead had a few other structs instead of just reusing *state bits, but that's just me22:44
thumperand your approach is equally valid22:44
thumperso no review comment on that :)22:44
rogpeppe1thumper: that's about my maximum tolerance22:45
rogpeppe1thumper: thanks for forbearing :-)22:45
rogpeppe1thumper: actually, i *don't* have map[string]interface{} - i don't use interface{} at all and i'm really happy about that22:48
thumperactually you are right...22:48
thumperI typed from memory miss remembering22:48
rogpeppe1thumper: that was what really prompted this branch was the million occurrences of .(sometype) and thinking "i have no idea where this might panic or if it's valid"22:49
fwereaderogpeppe1, LGTM just trivials22:49
thumperfor me, I would have had the units map inside a struct with the service state22:49
thumperthat's all22:49
rogpeppe1fwereade: oh sorry, i've just submitted22:49
thumperthe rest looks really good22:49
fwereaderogpeppe1, meh, fix them tomorrow22:50
rogpeppe1fwereade: i saw your earlier LGTM22:50
fwereaderogpeppe1, huge win regardless22:50
fwereaderogpeppe1, yep, np at all22:50
rogpeppe1fwereade: i generally agree with your remarks. putting StatusError alongside other fields would be wonderful and mean that the json/goyaml custom marshallers could go22:53
fwereaderogpeppe1, no worries tonight though22:53
rogpeppe1fwereade: i just tried to be ultra-compatible because i didn't know what the constraints were22:53
fwereaderogpeppe1, +122:53
fwereaderogpeppe1, in general adding fields is fine22:53
fwereaderogpeppe1, existing stuff we're trying to be a bit more careful about22:54
rogpeppe1fwereade: right, it's that time of day. actually way past. and if it's that time for me, then i suspect it might be for you too :-)22:54
rogpeppe1thumper, fwereade: g'night22:54
fwereadeSLEEP IS FOR THE WEAK22:54
thumperrogpeppe1: night22:54
fwereaderogpeppe1, gn :)22:54
* thumper is writing the lock stress test22:54
davecheneym_3: ping23:04
mgzdavecheney: I can go find him if it's anything urgent23:07
thumperah poos23:08
davecheneymgz: nah, just wanted to talk about load testing stuffs23:11
* thumper facepalms23:15
wallyworld_thumper: how's the face?23:15
wallyworld_what exasperated you?23:16
thumper$ go test23:17
thumperthrow: all goroutines are asleep - deadlock!23:17
* thumper sighs c.Check(state, Equals, 1)23:24
thumper... obtained int32 = 123:24
thumper... expected int = 123:24
=== wedgwood is now known as wedgwood_away
thumperdavecheney: is there a nice way to get a go routine to release it's time segment?23:25
thumperdavecheney: I want to say, go run something else for a bit23:25
thumperdavecheney: will time.sleep(0) do that?23:25
* thumper recalls it does in other languages23:25
davecheneythat will do it23:26
* thumper goes to the manual23:26
davecheneygodoc runtime Goschel23:27
thumperthat's what I want, awesome23:27
davecheneygodoc runtime Gosched23:27
davecheneysleep will do the same thing23:27
davecheneythumper: sorry, maasive lag in australia23:28
thumperstress test now fails23:28
thumperit shouldn't23:28
* thumper goes to read some more23:28
davecheneyalso try with GOMAXPROCS=8 (your n cores) go test23:29
davecheneywill probably have a similar effect23:29
thumperheh, passes with max procs23:31
thumperfails without23:31
thumperfailed that time23:32
thumperdebugging this is going to be a PITA23:34
davecheneythumper: tried the race detector23:38
davecheneyif you have a branch, I can test it for you23:38
thumperwhat is the race detector?23:39
davecheneyit's a feature on go 1.123:39
davecheneyit is hte same thread santiser that is available in clag 3.2 / gcc 4.823:39
davecheneythumper: http://tip.golang.org/doc/articles/race_detector.html23:39
thumper davecheney: lp:~thumper/juju-core/fslock-mashup has a failing stress test in utils/fslock23:40
thumperpassed with 1 and 2 concurrent locks, failed with 323:40
thumperthat is why it currently says 323:41
thumperI want 10 in the end...23:41
davecheneythumper: two secs23:44
davecheneymgz: is mramm around ?23:48
mgznot near me currently, could either be at the booth or in a meeting23:49
davecheneymgz: nm23:50
davecheneythumper: sorry, got distracted by submitting my own branch23:57

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