From 085a86e47549a97a3a6e493058f27d650753cbf7 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Fri, 12 Feb 2016 10:20:16 -0500 Subject: [PATCH] Fix some issues with concurrency in aufs. Adds a benchmark to measure performance under concurrent actions. Signed-off-by: Brian Goff Upstream-commit: 55c91f2ab9bcd48cfa248a4e842bb78257c14134 Component: engine --- .../engine/daemon/graphdriver/aufs/aufs.go | 31 ++++---- .../daemon/graphdriver/aufs/aufs_test.go | 71 ++++++++++++++++++- 2 files changed, 84 insertions(+), 18 deletions(-) diff --git a/components/engine/daemon/graphdriver/aufs/aufs.go b/components/engine/daemon/graphdriver/aufs/aufs.go index 51054fa6ef..e03576aa8a 100644 --- a/components/engine/daemon/graphdriver/aufs/aufs.go +++ b/components/engine/daemon/graphdriver/aufs/aufs.go @@ -227,7 +227,9 @@ func (a *Driver) Create(id, parent, mountLabel string) error { } } } + a.Lock() a.active[id] = &data{} + a.Unlock() return nil } @@ -285,20 +287,17 @@ func (a *Driver) Remove(id string) error { if err := os.Remove(path.Join(a.rootPath(), "layers", id)); err != nil && !os.IsNotExist(err) { return err } + if m != nil { + a.Lock() + delete(a.active, id) + a.Unlock() + } return nil } // Get returns the rootfs path for the id. // This will mount the dir at it's given path func (a *Driver) Get(id, mountLabel string) (string, error) { - ids, err := getParentIds(a.rootPath(), id) - if err != nil { - if !os.IsNotExist(err) { - return "", err - } - ids = []string{} - } - // Protect the a.active from concurrent access a.Lock() defer a.Unlock() @@ -309,13 +308,18 @@ func (a *Driver) Get(id, mountLabel string) (string, error) { a.active[id] = m } + parents, err := a.getParentLayerPaths(id) + if err != nil && !os.IsNotExist(err) { + return "", err + } + // If a dir does not have a parent ( no layers )do not try to mount // just return the diff path to the data m.path = path.Join(a.rootPath(), "diff", id) - if len(ids) > 0 { + if len(parents) > 0 { m.path = path.Join(a.rootPath(), "mnt", id) if m.referenceCount == 0 { - if err := a.mount(id, m, mountLabel); err != nil { + if err := a.mount(id, m, mountLabel, parents); err != nil { return "", err } } @@ -426,7 +430,7 @@ func (a *Driver) getParentLayerPaths(id string) ([]string, error) { return layers, nil } -func (a *Driver) mount(id string, m *data, mountLabel string) error { +func (a *Driver) mount(id string, m *data, mountLabel string, layers []string) error { // If the id is mounted or we get an error return if mounted, err := a.mounted(m); err != nil || mounted { return err @@ -437,11 +441,6 @@ func (a *Driver) mount(id string, m *data, mountLabel string) error { rw = path.Join(a.rootPath(), "diff", id) ) - layers, err := a.getParentLayerPaths(id) - if err != nil { - return err - } - if err := a.aufsMount(layers, rw, target, mountLabel); err != nil { return fmt.Errorf("error creating aufs mount to %s: %v", target, err) } diff --git a/components/engine/daemon/graphdriver/aufs/aufs_test.go b/components/engine/daemon/graphdriver/aufs/aufs_test.go index 761b5b6872..0f6d59d054 100644 --- a/components/engine/daemon/graphdriver/aufs/aufs_test.go +++ b/components/engine/daemon/graphdriver/aufs/aufs_test.go @@ -9,11 +9,13 @@ import ( "io/ioutil" "os" "path" + "sync" "testing" "github.com/docker/docker/daemon/graphdriver" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/reexec" + "github.com/docker/docker/pkg/stringid" ) var ( @@ -25,7 +27,7 @@ func init() { reexec.Init() } -func testInit(dir string, t *testing.T) graphdriver.Driver { +func testInit(dir string, t testing.TB) graphdriver.Driver { d, err := Init(dir, nil, nil, nil) if err != nil { if err == graphdriver.ErrNotSupported { @@ -37,7 +39,7 @@ func testInit(dir string, t *testing.T) graphdriver.Driver { return d } -func newDriver(t *testing.T) *Driver { +func newDriver(t testing.TB) *Driver { if err := os.MkdirAll(tmp, 0755); err != nil { t.Fatal(err) } @@ -732,3 +734,68 @@ func TestMountMoreThan42LayersMatchingPathLength(t *testing.T) { zeroes += "0" } } + +func BenchmarkConcurrentAccess(b *testing.B) { + b.StopTimer() + b.ResetTimer() + + d := newDriver(b) + defer os.RemoveAll(tmp) + defer d.Cleanup() + + numConcurent := 256 + // create a bunch of ids + var ids []string + for i := 0; i < numConcurent; i++ { + ids = append(ids, stringid.GenerateNonCryptoID()) + } + + if err := d.Create(ids[0], "", ""); err != nil { + b.Fatal(err) + } + + if err := d.Create(ids[1], ids[0], ""); err != nil { + b.Fatal(err) + } + + parent := ids[1] + ids = append(ids[2:]) + + chErr := make(chan error, numConcurent) + var outerGroup sync.WaitGroup + outerGroup.Add(len(ids)) + b.StartTimer() + + // here's the actual bench + for _, id := range ids { + go func(id string) { + defer outerGroup.Done() + if err := d.Create(id, parent, ""); err != nil { + b.Logf("Create %s failed", id) + chErr <- err + return + } + var innerGroup sync.WaitGroup + for i := 0; i < b.N; i++ { + innerGroup.Add(1) + go func() { + d.Get(id, "") + d.Put(id) + innerGroup.Done() + }() + } + innerGroup.Wait() + d.Remove(id) + }(id) + } + + outerGroup.Wait() + b.StopTimer() + close(chErr) + for err := range chErr { + if err != nil { + b.Log(err) + b.Fail() + } + } +}