Race in mgo Stats implementation

[Curtis Hovey]

As seen at
    http://reports.vapour.ws/releases/issue/578f7bf1749a567c7344833e

The is a race in github.com/juju/juju/featuretests

WARNING: DATA RACE
Read by goroutine 53:
  gopkg.in/mgo%2ev2.(*mongoSocket).kill()
      /home/ubuntu/juju-core_2.0-beta13/src/gopkg.in/mgo.v2/socket.go:329 +0x7e5
  gopkg.in/mgo%2ev2.(*mongoSocket).Close()
      /home/ubuntu/juju-core_2.0-beta13/src/gopkg.in/mgo.v2/socket.go:316 +0xd9
  gopkg.in/mgo%2ev2.(*mongoServer).Close()
      /home/ubuntu/juju-core_2.0-beta13/src/gopkg.in/mgo.v2/server.go:199 +0x35f
  gopkg.in/mgo%2ev2.(*mongoCluster).Release()
      /home/ubuntu/juju-core_2.0-beta13/src/gopkg.in/mgo.v2/cluster.go:101 +0x352
  gopkg.in/mgo%2ev2.(*mongoCluster).syncServersLoop()
      /home/ubuntu/juju-core_2.0-beta13/src/gopkg.in/mgo.v2/cluster.go:363 +0x370

Previous write by goroutine 132:
  gopkg.in/mgo%2ev2.ResetStats()
      /home/ubuntu/juju-core_2.0-beta13/src/gopkg.in/mgo.v2/stats.go:59 +0x17e
  github.com/juju/testing.(*MgoSuite).SetUpTest()
      /home/ubuntu/juju-core_2.0-beta13/src/github.com/juju/testing/mgo.go:618 +0x41
  github.com/juju/juju/juju/testing.(*JujuConnSuite).SetUpTest()
      /home/ubuntu/juju-core_2.0-beta13/src/github.com/juju/juju/juju/testing/conn.go:121 +0x4c
  github.com/juju/juju/featuretests.(*syslogSuite).SetUpTest()
      /home/ubuntu/juju-core_2.0-beta13/src/github.com/juju/juju/featuretests/syslog_test.go:109 +0x2b4
  runtime.call32()
      /usr/lib/go-1.6/src/runtime/asm_amd64.s:472 +0x3d
  reflect.Value.Call()
      /usr/lib/go-1.6/src/reflect/value.go:303 +0xcd
  gopkg.in/check%2ev1.(*suiteRunner).runFixture.func1()
      /home/ubuntu/juju-core_2.0-beta13/src/gopkg.in/check.v1/check.go:721 +0x1b8
  gopkg.in/check%2ev1.(*suiteRunner).forkCall.func1()
      /home/ubuntu/juju-core_2.0-beta13/src/gopkg.in/check.v1/check.go:666 +0x80

[Reed O'Brien (reedobrien)]

Looking into mgo. There appeared to be a data race in the way Stats are implemented. menn0 confirmed and suggested a fix. Without the fix I managed to reproduce the race twice in a couple hours of running.

Last night I left a script running to stress feature tests with a fix. It didn't trigger overnight.

Currently the patch doesn't change the outcome of tests in gopkg.in/mgo.v2. However, gopkg.in/mgo.v2-unstable is where new fixes would be applied so I created a branch there and am testing against that and running a script to stress feature tests with mgo.v2-unstable.

So a little bit of polish and I'll make a PR against mgo in github.

[Reed O'Brien (reedobrien)]

NOTE: mgo.v2-unstable has changed from using chrism's python supervisor to djb's supervisor. So in unstable you need to `apt-get install daemontools-run` in order to run the tests.

sudo apt-get install daemontools-run

make startdb; go test; make stopdb

Of course if you want to inspect the artifacts after the test run don't make stopdb as it cleans up said artifacts. WRT artifacts in unstable they are now in a _harness directory rather than a _testdb directory.

[Reed O'Brien (reedobrien)]

fix proposed https://github.com/go-mgo/mgo/pull/303

We'll see if it breaks the build...

[Reed O'Brien (reedobrien)]

If the fix is accepted we'll need to either wait until it lands in mgo.v2, pin against v2-unstable, vendor, or maintain a patch. I don't know our MO.

[Reed O'Brien (reedobrien)]

I decided it would be wise to run the mgo tests with the race flag. The change we made created a new race in mgo. This is now fixed, too. Same PR https://github.com/go-mgo/mgo/pull/303

[Reed O'Brien (reedobrien)]

Menno is reaching out to gustavo for another mgo issue and said he'll also bring this to his attention.

[Anastasia (anastasia-macmood)]

New mgo has been released - https://twitter.com/gniemeyer/status/760472159256993792.

We need to update our dependencies.

[Menno Finlay-Smits (menno.smits)]

The new mgo release doesn't appear to contain the fix for this issue. The PR hasn't been accepted and the stable branch doesn't have the fix.

[Menno Finlay-Smits (menno.smits)]

I've generalised the bug title as this issue can happen in almost any Juju test which uses MongoDB.

[Reed O'Brien (reedobrien)]

Upstream isn't moving, so this has been added to the patches directory and should be applied on CI test runs.