=== alexlist` is now known as alexlist [04:25] Bug #1576366 changed: juju 2 beta6: show-controller --format=json is broken

=== frankban|afk is now known as frankban === gnuoy` is now known as gnuoy [08:00] I believe reviewboard is not having a good day [08:00] this never got a rb link https://github.com/juju/juju/pull/5747 [08:28] Bug #1597354 changed: Juju 2.0 Resource Error - cannot add resource failed to write data: read tcp : i/o timeout [08:28] Bug #1598049 opened: TestLogRecordForwarded fails on non-ubuntu

[08:29] cherylj, ericsnow: ^ bug A [08:56] ericsnow, katco: also bug 1598063 (I have not assigned this one to milestone, we may be able to punt) [08:56] Bug #1598063: Data race in apiserver/observer package

[08:57] wait, I did, but maybe we punt [09:01] Bug #1598063 opened: Data race in apiserver/observer package

[09:39] mgz: https://github.com/juju/juju/pull/5750 [09:41] sinzui: this looks good to me ^ [09:42] ericsnow: [09:42] ok github.com/juju/juju/featuretests 138.465s [09:42] http://reports.vapour.ws/releases/4108/job/run-unit-tests-win2012-amd64/attempt/2566 [09:43] ericsnow: that's 54617e0a from wed [09:44] so, it passed (possibly with that junk in the logs, not shown), before your changes [09:44] yep [09:47] mgz: jam: yes this looks good. but consider that run-unit-tests always calls "go test -i ./..." now [09:48] sinzui: I missed that, but I see it now. [09:49] jam: so gig mgz...who add it to the script :) [09:51] jam: mgz: In the past, we varied the command line for testing. 2 or 3 of the variations are no longer used. I think the only case where we don't use the makefile is running with race [09:52] sinzui: yeah, we were looking at the gating job specifically [09:53] mgz: yeah that is one we flip-flop from makefile's test to --race [09:53] mgz: and the goal for xenial-amd64 is to use race next week [09:53] or today even [10:14] would anyone kindly review https://github.com/juju/juju/pull/5747 that lacks a reviewboard link for reasons escape my control? [10:15] axw: I addressed your comments for register, please re-check [10:22] perrito666: reviewed [10:22] axw: tx [10:22] did you really go through the checklist? [10:27] perrito666: did u keep checklist in mind while coding? :D [10:27] * axw looks shamefaced [10:27] we're not starting till next week :) [10:27] anastasiamac: evidently, I always do, even before it existed :p [10:30] thumper: as if it had head you https://twitter.com/4BringingFire/status/748265855398576128 [11:11] Bug #1598113 opened: resource-get should not download if not necessary

[11:24] cherylj: got a fail from featuretest, which I little suspected where testing this [11:25] :/ [11:28] perrito666: bug 1598049? [11:28] Bug #1598049: TestLogRecordForwarded fails on non-ubuntu

[11:28] or a new one? [11:30] mgz: a new one, I made a change in juju register and it would seem that the same functionality is being tested in the cmd tests and in feature tests === bradm_ is now known as bradm [11:53] Bug #1316223 changed: specifying juju deploy --networks=vlan:42 causes a panic

[11:53] Bug #1584805 changed: Timeout in github.com/juju/juju/apiserver/service on windows

[11:53] Bug #1595276 changed: TestDestroyControllerErrors failure with out of order errors

[11:53] Bug #1598118 opened: log-forwarder worker bounces endlessly when forwarding is not configured <2.0>

[12:02] Bug #1598127 opened: lxdbr0 spam in log file

[12:10] mgz: this is a flaky run right? http://juju-ci.vapour.ws:8080/job/github-merge-juju/8340/console [12:14] Bug #1598127 changed: lxdbr0 spam in log file

[12:23] Bug #1598127 opened: lxdbr0 spam in log file

[13:32] cherylj: here it is - bug 1598164 [13:32] Bug #1598164: [aws] adding a machine post-bootstrap on the controller model closes of api port in controller security group

[13:32] Bug #1598164 opened: [aws] adding a machine post-bootstrap on the controller model closes of api port in controller security group

[13:34] thanks, dimitern! [13:44] mgz: thanks, i'll pick up bug 1598063 shouldn't be a hard dx/fix [13:44] Bug #1598063: Data race in apiserver/observer package

[13:47] katco: thank you! [15:03] perrito666: bug 1598049 [15:03] Bug #1598049: TestLogRecordForwarded fails on non-ubuntu

[15:05] mgz: tx [15:19] very simple review for someone: http://reviews.vapour.ws/r/5201/ [15:20] mgz: perhaps you are interested [15:26] Bug #1598206 opened: lxc/lxd/shared/util_linux.go sys/types.h: No such file or directory

[15:28] katco: ta [16:22] wallyworld: ping [16:23] katco: hey, how'd the house go? [16:23] wallyworld: fine, boring [16:23] boring is good [16:23] wallyworld: yep :) [16:23] wallyworld: re. your comment on my review. are you referring to the observer multiplexer? [16:24] wallyworld: e.g. this? https://github.com/juju/juju/blob/master/apiserver/observer/observer.go#L102-L115 [16:24] i.e. rather [16:24] katco: https://github.com/juju/juju/blob/master/apiserver/observer/observer.go#L109 [16:25] the use of a go routine inside the loop [16:25] wallyworld: i don't think that's what's causing the issue [16:25] wallyworld: the race is between a call to ServerRequest and ServerReply [16:25] the race output seemd to implicate that aspect; i was going by advice from williamn [16:26] ie the race output specically talks about calls from inside those go eoutines [16:26] wallyworld: that loop will just call a single observer method on multiple observers concurrently. [16:26] wallyworld: yeah, but not at the top of the call-stack... [16:27] wallyworld: Previous write by goroutine 56: [16:27] github.com/juju/juju/apiserver/observer.(*RequestNotifier).ServerRequest() [16:27] wallyworld: Read by goroutine 245: [16:27] github.com/juju/juju/apiserver/observer.(*RequestNotifier).ServerReply() [16:27] wallyworld: the issue is that there are two calls coming into the same observer concurrently... i could remove the multiplexer entirely and this race would still occur [16:28] ok, my brain hasn't yet delved into the full detail, so i can't confirm mentally one way or the other [16:29] but we didn;t see this race before right [16:29] wallyworld: this observer had mutexes before [16:29] the observer stuff seems to have introduced it [16:30] where were the mutexes? why were they removed? [16:30] did their removal intorduce the race? [16:30] wallyworld: https://github.com/juju/juju/blob/bbc4a902fe44ee6effdd5e0216b3e0b8216643ef/apiserver/apiserver.go#L248 [16:30] wallyworld: because of what i said in the PR... i incorrectly assumed that requests/replies would happen synchronously [16:31] wallyworld: the rpc server does not guarantee that. it has nothing to do with the multiplexer [16:32] wallyworld: here's where ServerReply is kicked off on a new goroutine: https://github.com/juju/juju/blob/master/rpc/server.go#L465 [16:32] katco: i'm slow today (or always) - so the above mutex on line 248 avoided races before this new work? [16:32] wallyworld: apparently so [16:32] so why does this new work introduce the recaes then? is that mutex removed? [16:33] the observer stuff should not need extra locking? [16:33] wallyworld: yes, because the rpc server will call ServerRequest, and then spawn another goroutine and call ServerReply [16:34] wallyworld: if it's not in the observer, we need to touch rpc.Conn to lock there [16:34] wallyworld: i.e. synchronize the reply to the request so it's synchronous [16:36] wallyworld: i can tell you don't believe me; i'll just inject the RequestNotifier and show that the problem doesn't reside in the multiplexer [16:40] wallyworld: yarp [16:44] sinzui: 2 things, 1) how will I know when I have my new mongo 2) how did you find a mongo for windows with ssl? [16:45] perrito666: we rarely update the db. the choice to switch to 2.6 was driven by the case it is supported. [16:45] perrito666: When we want to test only with 3.2, we will put 3.2 on the host [17:18] wallyworld: ok, justification posted to the review === frankban is now known as frankban|afk [17:32] mgz: https://github.com/juju/juju/pull/5759 [18:18] Bug # changed: 1575940, 1588403, 1594415, 1596967, 1597372, 1597516, 1597519 [18:42] katco: hey, sorry, have been smashed trying to get log forwarding working using the actual code for the build [18:43] wallyworld: no worries at all [18:45] katco: one comment given to me in additon was the use of a RWMutex instead of just a sync.Mutex - the added complexity is not justified [18:46] wallyworld: i'm sorry, i don't understand how that's more complex? [18:48] Bug #1598272 opened: LogStreamIntSuite.TestFullRequest sometimes fails

[18:56] katco: it's cognitive overhead - when do i use read vs write lock, as opposed to just lock [18:57] Bug #1585825 changed: Takes too long to download a resource from a controller to unit

[18:57] wallyworld: shouldn't we be thinking about that? [18:58] katco: not prematurely [18:58] not unless it has been shown to be an issue [18:58] wallyworld: it's our RPC mechanism, we want to limit our critical section as much as possible [19:09] Bug #1585825 opened: Takes too long to download a resource from a controller to unit

[19:18] Bug #1585825 changed: Takes too long to download a resource from a controller to unit

[20:03] Bug # opened: 1598286, 1598289, 1598290, 1598291 === mup_ is now known as mup [20:24] Bug # opened: 1598286, 1598289, 1598290, 1598291, 1598292, 1598293 [20:30] Bug #1598292 changed: log forwarding subject to clock skew [20:30] Bug #1598293 changed: log forwarding feature does not use updated config [20:36] Bug #1598292 opened: log forwarding subject to clock skew [20:36] Bug #1598293 opened: log forwarding feature does not use updated config === urulama|___ is now known as urulama|____ [23:46] Bug #1598319 opened: Openstack Provider - No way to use multiple images