thumpertaking the kids out for lunch00:30
thumperback later00:30
=== thumper is now known as thumper-afk
=== thumper-afk is now known as thumper
thumperdavecheney: got a few minutes for a hangout?03:04
davecheneyi'll get my other pc03:05
thumperdavecheney: https://plus.google.com/hangouts/_/gqpui2frcqiffoyg6usnsp6dfaa?authuser=1&hl=en03:06
=== Ursinha is now known as Ursinha-afk
=== Ursinha-afk is now known as Ursinha
stokachusomeone mind taking a look at this json response http://paste.ubuntu.com/7778445/03:47
stokachuim passing it the ConfigYAML file with the service name03:47
stokachubut it isn't finding the settings for keystone for some reason03:48
stokachui parsed the yaml file to make sure there were no errors and it loads up with the keystone set as the key03:49
thumperstokachu: is the service named keystone?03:53
thumperstokachu: I've found that the config is based on the name of the service, not the charm name03:54
stokachuah hmm03:54
stokachulemme check something03:54
stokachuso i set the ServiceName to keystone03:54
thumperstokachu: what is it named in status?03:55
thumperstokachu: or, another way, what was your deploy command?03:55
stokachuim using ServiceDeploy api call03:56
stokachuso i pass in the params for ServiceName etc03:56
stokachuthumper, if i deploy without ConfigYAML set then it deploys the keystone service named 'keystone'03:57
thumpersorry, not sure03:58
stokachui cant find a test that uses the ConfigYAML parameters04:01
stokachui tried to mimic the api call to https://github.com/juju/juju/blob/master/state/apiserver/client/client_test.go#L80704:04
stokachuwhich i think is right04:04
thumperstokachu: let me look at something04:06
thumperstokachu: according to the code in cmd/juju/deploy.go, the yaml config expected in the ServiceDeploy api is the entire yaml file with a top level name matching the service name.04:10
stokachuso not the string?04:10
stokachufilename string i take it04:10
thumperyou need to read the file, and pass the yaml as string04:11
stokachuah well then thats probably why04:11
thumperwill be04:11
stokachushouldnt code so late04:11
* thumper was hacking at 1am the other night04:11
thumpergets tiring04:11
stokachuyea but im so close to get this working lol04:12
stokachudamnit what a stupid mistake04:14
stokachuthumper, thanks for the extra set of eyes04:15
thumpermy brain has stopped working04:20
thumpertime to leave04:20
waiganiugh, just realized my irc was not connected04:46
waiganidavecheney: here's the output of my script: http://pastebin.ubuntu.com/7778556/04:48
waiganidavecheney: it deals with comments, if the groups are in the wrong order, if there are too many groups. I *think* I don't have any false positives.04:50
* davecheney looks05:15
=== vladk|offline is now known as vladk
vladkdimitern: please, take a look http://github.com/juju/juju/pull/12106:51
dimiternvladk, looking06:57
dimiternvladk, reviewed07:39
vladkdimitern: thanks07:39
dimiternvladk, remind me again please, why do we need to care about parsing the interfaces files so thoroughly ?07:42
dimiternvladk, can't we get away with just what the net package gives us?07:42
dimitern(and running some commands like ifup etc.)07:42
dimiternalso, seems like fixing maas changes to interfaces files is not a job for juju07:43
dimitern(does it need to be a special case like that?)07:43
vladkdimitern: to bring up interface, we need to add some lines to configuration file, in the future we are going to support static IPs, so configuration lines may depend on network state07:51
dimiternvladk, well, my point is we can *write* to the interfaces files, but use the net package to *read* what's currently on and has address07:52
dimiternvladk, re static ips - let's think about that when we get there - it won't be until october that's for sure07:52
vladkdimitern: I asked people and found that most prefer to have configuration in separate files07:53
vladkadditionally it may be interfaces not managed by juju07:53
dimiternvladk, if it's a machine juju started, juju has total control over it and we don't have to care about what users change later07:53
dimiternvladk, so istm being less careful to preserve what's there and making sure we (over)write the config file for interfaces juju manages (all of them) should be fine07:55
dimiternTheMue, a small-ish review? https://github.com/juju/juju/pull/29308:40
TheMuedimitern: yep08:41
dimiterndamn! i've edited a card and then it disappeared!08:44
dimiternTheMue, thanks!08:45
TheMuedimitern: after first fears that the review process in GH gets worse I now feel more comfortable with it08:47
dimiternTheMue, it grows on you :)08:51
TheMuedimitern: ;)08:54
TheMuedimitern: you’ve got a review08:55
dimiternTheMue, tyvm08:55
TheMuedimitern: yw08:55
* TheMue continues with the flakey tests08:56
dimiternTheMue, need any help on these? For the "port 54321 already in use" it should be trivial not to hard code it, but use ":0" and then get the random port Listen used08:58
TheMuedimitern: I’m currently at the double interface event08:59
dimiternTheMue, ok09:00
TheMuedimitern: but I’ll keep your hint in mind, thanks09:00
dimiternTheMue, cheer09:00
dimiterns :)09:00
=== vladk is now known as vladk|offline
=== vladk|offline is now known as vladk
TheMuesystem just logged me out (w/ kiling all appls)09:05
dimiternTheMue, the PR linked to the flaky tests bug report is wrong btw09:05
TheMuedimitern: which PR?09:06
TheMuedimitern: I’ve not yet done one09:06
dimiternTheMue, the one linked is about the networker https://github.com/juju/juju/pull/207/09:06
dimiternTheMue, but the failing test is in state/machine_test.go, which is an earlier one - https://github.com/juju/juju/pull/169/09:08
TheMuedimitern: thx, will change it09:08
=== vladk is now known as vladk|offline
TheMuedimitern: could you pls take a look at state/machine_test.go line 1803 - 1808?09:15
dimiternTheMue, something's fishy, I've been looking at the machine interfaces watcher implementation09:16
TheMuedimitern: if two different interfaces are disabled, why we only await one change?09:16
dimiternTheMue, the only reason I can think of is if 2 changes are merged and reported as one, but the watcher has to be written like that09:17
TheMuedimitern: that would be in watcher.go 1758ff09:18
dimiternTheMue, there's a collect() func in state/watcher09:18
dimiternTheMue, which is supposed to collapse multiple changes into one, but the machine interfaces watcher is not using it, but it's being tested as it is09:19
TheMuedimitern: yes, looks like09:20
dimiternTheMue, what freaks me out is why it's not happening *all the time*09:21
TheMuedimitern: it’s a race, sometimes the second disabling event is fast enough received, sometimes not09:22
dimiternTheMue, that seems to be the issue - *all* notify watchers use collect() inside the loop09:22
TheMuedimitern: so I’ll change it do it so here too09:22
dimiternTheMue, can that be forced by introducing a delay before disabling the second iface in the test?09:23
TheMuedimitern: not yet tested09:23
dimiternTheMue, that way you can be sure your changes in the loop() work09:23
TheMuedimitern: good idea, will try that first to have a good testbed09:23
dimiternTheMue, thanks09:23
dimiternTheMue, also, while you're at it, can you please drop the notify flag from the loop? setting out to nil or to w.out is enough to signal a notification is needed09:24
TheMuedimitern: yip09:25
dimiternTheMue, ta09:26
TheMuedimitern: btw, why did you moved the flakey test card, there’s no PR yet ;)09:29
dimiternTheMue, did I? Wasn't intentional if it was me09:31
TheMuedimitern: changed it back, an will split it into machine interface and the other one09:31
dimiternTheMue, good idea09:32
=== vladk|offline is now known as vladk
vladkdimitern: you wrote many comments on exportable names. 90% of them are related to access to internals from test package, because test package has different name. How do you solve this problem?09:57
dimiternvladk, using export_test?09:58
=== urulama is now known as uru-afk
TheMuedimitern: standup10:46
dimiternTheMue, brt10:46
=== uru-afk is now known as urulama
=== vladk is now known as vladk|offline
=== vladk|offline is now known as vladk
=== vladk is now known as vladk|offline
=== vladk|offline is now known as vladk
=== psivaa is now known as psivaa-lunch
TheMuevladk: ping12:55
vladkTheMue: pong12:55
TheMuevladk: in TestWatchInterfaces() in line 1854, why do you expect no change here?12:56
TheMuevladk: state/machine_test.go12:57
vladkTheMue: because we have a watcher per machine, so it does not notify about interface changes on foreign machine12:59
TheMuevladk: ah, ic, thx12:59
perrito666natefinch: the suspense is killing us, what is the veredict?13:00
TheMuevladk: and found the error here in my code ;)13:00
TheMuevladk: yeah, that has been the needed info, now it works.13:03
TheMuevladk: you’re OCR? here’s PR https://github.com/juju/juju/pull/294 for one of the flakey tests (see lp:1339864)13:11
vladkTheMue: looking13:11
TheMuevladk: thanks13:11
perrito666more than 60 secs to run 1 test when using mgosuite... beautiful13:15
perrito666wwitzel3: dont get that near the computer, it will mess your eyes13:38
wwitzel3perrito666: :)13:44
=== Ursinha is now known as Ursinha-afk
=== Ursinha-afk is now known as Ursinha
=== psivaa-lunch is now known as psivaa
TheMuevladk: any questions regarding the PR?14:14
vladkTheMue: no, just a minute14:15
TheMuevladk: thx14:15
TheMuevladk: looking into the second issue during that time ;)14:15
mgzTheMue: I'm confused as to how that pr actually makes the test reliable14:18
TheMuemgz: it has been a race condition as it not used the collection of changes before. and when those have been with the wrong timng the watcher sent multiple events14:19
TheMuemgz: I’ve been able to reproduce the error locally by removing the error asserts in the test (to make it faster, funnily a Sleep doesn’t changed it)14:20
TheMuemgz: and after the change (dimiter gave me the hint with collect) it worked fine in all situations14:21
vladkTheMue: where did you find a race condition? We have a separate watcher for every machine with separate map of changes14:32
TheMuevladk: I didn’t found it, Andrew. See https://bugs.launchpad.net/juju-core/+bug/1339864.14:33
_mup_Bug #1339864: MachineSuite.TestWatchInterfaces intermittently fails <intermittent-failure> <test-failure> <juju-core:In Progress by themue> <https://launchpad.net/bugs/1339864>14:33
=== vladk is now known as vladk|offline
=== vladk|offline is now known as vladk
TheMuevladk: I simply tried to understand it, discussed with dimiter, found a way to have it almost every time here on my machine too, changed it and now it never happened again (ran the test multiple times in a loop)14:35
vladkTheMue: It would better to drop that test with two sequential changes, rather to implement collect. I agree that the test is not correct. But no race condition here. Even with the collect implemented that test may failed.14:37
TheMuevladk: why using a different pattern then in the other watchers?14:37
TheMuevladk: collect() hasn’t been implemented by me and is used in multiple watchers14:38
vladkTheMue: because all watchers already has different patterens14:38
vladkI'm not sure that this code has large practical meaning, because interfaces are not changed very often. We will get more complex code, but will not get any increase in performance.14:39
TheMuevladk: in details yes, not always in patterns14:39
vladkTheMue: I gave you LGTM14:40
TheMuevladk: thanks14:41
vladkTheMue: But I would drop that test with 2 events or changed it to one event.14:41
vladkTheMue: also there is a mistake in WatchInterfaces() comment14:41
TheMuevladk: Some minutes14:42
=== vladk is now known as vladk|offline
wallyworldmgz: https://github.com/juju/juju/pull/29514:44
axwwallyworld: https://github.com/juju/juju/pull/29615:36
=== vladk|offline is now known as vladk
katconiemeyer: you around?16:26
niemeyerkatco: Always!16:40
=== urulama is now known as uru-afk
mattywrogpeppe, ping?17:19
rogpeppemattyw: off now, sorry17:25
rogpeppeg'night all17:25
voidspaceniemeyer: ping17:32
voidspaceniemeyer: I'm looking at a juju bug, "i/o timeout" errors, which occur in various places https://bugs.launchpad.net/juju-core/+bug/130743417:33
_mup_Bug #1307434: talking to mongo can fail with "TCP i/o timeout" <cloud-installer> <landscape> <performance> <reliability> <juju-core:Triaged> <https://launchpad.net/bugs/1307434>17:33
voidspaceniemeyer: we *believe* this is due to the way we use a single session for *everything* in juju17:33
voidspaceniemeyer: please bear in mind that I know very little about mgo and our use of it, so I'm very happy for you to correct any misconceptions I may have....17:34
voidspaceniemeyer: state.State has a transaction runner (actually our own that wraps an mgo one)17:34
niemeyervoidspace: Heya17:34
=== ev_ is now known as ev
voidspaceniemeyer: o/ sorry for the wall of text :-)17:34
niemeyervoidspace: In a call, but can be with you in 20mins, at most17:34
voidspaceniemeyer: ok, sure - I'll finish describing the problem and if you get a chance to look at it and answer my question (I do have a question) then great17:35
voidspaceniemeyer: if not I can fire it off as an email (probably when I get back from Lexington)17:35
niemeyervoidspace: Sounds good17:35
voidspaceniemeyer: we would rather use the socket pooling that is built into mgo17:36
niemeyervoidspace: I mean, please feel free to continue describing the problem.. will read and respond right after the call17:36
voidspaceniemeyer: yep, thanks17:36
voidspaceniemeyer: the comment on the mgo transaction runner says17:37
voidspace/ Multiple transaction collections may exist in a single database, but17:37
voidspace/ all collections that are touched by operations in a given transaction17:37
voidspace/ collection must be handled exclusively by it.17:37
voidspaceniemeyer: this leads us to believe that we must only have a single transaction runner and that we can't just switch to using multiple sessions (as the transaction runner has a reference to the session it is created from)17:38
mgzfun fixes: https://github.com/juju/juju/pull/29817:40
voidspaceniemeyer: and for some reason, just refreshing the session every time we use it is causing auth errors (possibly related to the way we change passwords - but reopening the state after changing password doesn't fix the auth errors so there must be some other interactin here)17:40
voidspaceniemeyer: so, can you see a way for us to avoid having a long lived transaction runner / session - and do you think that it is indeed likely that the long lived session is the root cause of these errors we're seeing17:40
niemeyervoidspace: Okay, here17:50
voidspaceniemeyer: o/17:50
niemeyervoidspace: The reason why it's timing out is likely that you have a session sitting around unused for more than 10 minutes17:50
voidspaceniemeyer: yep, we use a single session for everything17:51
niemeyervoidspace: mgo currently does not ping sessions in use to keep them alive17:51
niemeyervoidspace: Using a single session is not strictly a problem by itself17:52
niemeyervoidspace: It may be a problem depending on what else is being done17:52
voidspaceniemeyer: ok17:52
voidspaceniemeyer: the transaction runner keeps a reference to the session, and we must have a single transaction runner (we believe)17:52
niemeyervoidspace: For example, sharing a session means sharing the underlying socket too, which means concurrent clients may have to wait for long running operations17:52
voidspaceniemeyer: right17:53
niemeyervoidspace: It also means that connectivity errors break both clients at once, and you cannot recover the session locally because it would hide the error from every other user17:53
voidspaceniemeyer: so we could use multiple sessions (copy / clone / refresh), but we also need to ensure we keep alive the one used by the transaction runner17:53
niemeyervoidspace: Okay, switching to the transaction runner issue17:54
niemeyervoidspace: You can run as many transaction runners with as many sessions as you want17:54
niemeyervoidspace: Creating a runner is very cheap, and has no kind of caching17:54
voidspaceniemeyer: what about that comment in the transaction runner code?17:55
niemeyervoidspace: That comment is about the transaction collection..17:55
niemeyervoidspace: not about sessions17:55
niemeyer(or runners)17:55
voidspaceah... indeed17:55
niemeyervoidspace: I would like to implement global session pinging, but I'm a bit concerned about the cost of doing so17:56
niemeyervoidspace: I will sit on this over the weekend to ponder how it might be implemented in a good way17:57
voidspaceniemeyer: I think I mentally substituted "runner" for "collection" when reading that comment - so it's not actually a concern17:57
niemeyervoidspace: I definitely don't want to have an extra goroutine for every single session, for instance17:57
voidspaceniemeyer: that would certainly help us, but we should probably fix our code anyway17:57
voidspaceniemeyer: thanks, that's very helpful - I think I have some things I can try now17:57
niemeyervoidspace: Right, I do think you likely have issues to solve there either way17:57
niemeyervoidspace: FWIW, I've shared that sentiment before in that same context (I/O timeouts)17:58
voidspacewe could have a session pinger ourselves, but that's probably not the best fix17:58
niemeyervoidspace: I pretty much always implement http request handling as Copy + defer Close, for instance17:58
niemeyervoidspace: Agreed.. it sounds like this is a symptom of a more fundamental design issue17:58
voidspaceI still have to work out why we get these auth errors, but that's my problem...17:59
niemeyervoidspace: Even with pinging, connections die all the time.. then what?17:59
mgzwallyworld_: https://bugs.launchpad.net/juju-core/+bug/134089318:33
_mup_Bug #1340893: juju bootstrap in an existing environment destroys the environment <canonical-is> <juju-core:New> <https://launchpad.net/bugs/1340893>18:33
bodie_super simple PR for testing Actions -- literally nothing added, just a testing charm location.  I'll need the godeps change in order to land Actions RunHook19:35
=== vladk is now known as vladk|offline
=== vladk|offline is now known as vladk
jcw4bodie_: LGTM, I think it's fine to merge19:52
=== vladk is now known as vladk|offline
bodie_hmm, my dependencies.tsv diff seems off20:00
jcw4how so?20:01
bodie_I seem to have a different entry for loggo even though I've synced with master20:02
mgzbodie_: see my latest mp20:02
jcw4bodie_: I think the launchpad.net/loggo dependency was wrong20:02
jcw4mgz: +120:02
mgzso, just remove that line20:03
bodie_there we are then https://github.com/juju/juju/pull/30120:10

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