Jelajahi Sumber

Fix some issues with concurrency in aufs.
Adds a benchmark to measure performance under concurrent actions.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Brian Goff 9 tahun lalu
induk
melakukan
55c91f2ab9
2 mengubah file dengan 84 tambahan dan 18 penghapusan
  1. 15 16
      daemon/graphdriver/aufs/aufs.go
  2. 69 2
      daemon/graphdriver/aufs/aufs_test.go

+ 15 - 16
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.active[id] = &data{}
+	a.Unlock()
 	return nil
 	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) {
 	if err := os.Remove(path.Join(a.rootPath(), "layers", id)); err != nil && !os.IsNotExist(err) {
 		return err
 		return err
 	}
 	}
+	if m != nil {
+		a.Lock()
+		delete(a.active, id)
+		a.Unlock()
+	}
 	return nil
 	return nil
 }
 }
 
 
 // Get returns the rootfs path for the id.
 // Get returns the rootfs path for the id.
 // This will mount the dir at it's given path
 // This will mount the dir at it's given path
 func (a *Driver) Get(id, mountLabel string) (string, error) {
 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
 	// Protect the a.active from concurrent access
 	a.Lock()
 	a.Lock()
 	defer a.Unlock()
 	defer a.Unlock()
@@ -309,13 +308,18 @@ func (a *Driver) Get(id, mountLabel string) (string, error) {
 		a.active[id] = m
 		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
 	// If a dir does not have a parent ( no layers )do not try to mount
 	// just return the diff path to the data
 	// just return the diff path to the data
 	m.path = path.Join(a.rootPath(), "diff", id)
 	m.path = path.Join(a.rootPath(), "diff", id)
-	if len(ids) > 0 {
+	if len(parents) > 0 {
 		m.path = path.Join(a.rootPath(), "mnt", id)
 		m.path = path.Join(a.rootPath(), "mnt", id)
 		if m.referenceCount == 0 {
 		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
 				return "", err
 			}
 			}
 		}
 		}
@@ -426,7 +430,7 @@ func (a *Driver) getParentLayerPaths(id string) ([]string, error) {
 	return layers, nil
 	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 the id is mounted or we get an error return
 	if mounted, err := a.mounted(m); err != nil || mounted {
 	if mounted, err := a.mounted(m); err != nil || mounted {
 		return err
 		return err
@@ -437,11 +441,6 @@ func (a *Driver) mount(id string, m *data, mountLabel string) error {
 		rw     = path.Join(a.rootPath(), "diff", id)
 		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 {
 	if err := a.aufsMount(layers, rw, target, mountLabel); err != nil {
 		return fmt.Errorf("error creating aufs mount to %s: %v", target, err)
 		return fmt.Errorf("error creating aufs mount to %s: %v", target, err)
 	}
 	}

+ 69 - 2
daemon/graphdriver/aufs/aufs_test.go

@@ -9,11 +9,13 @@ import (
 	"io/ioutil"
 	"io/ioutil"
 	"os"
 	"os"
 	"path"
 	"path"
+	"sync"
 	"testing"
 	"testing"
 
 
 	"github.com/docker/docker/daemon/graphdriver"
 	"github.com/docker/docker/daemon/graphdriver"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/reexec"
 	"github.com/docker/docker/pkg/reexec"
+	"github.com/docker/docker/pkg/stringid"
 )
 )
 
 
 var (
 var (
@@ -25,7 +27,7 @@ func init() {
 	reexec.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)
 	d, err := Init(dir, nil, nil, nil)
 	if err != nil {
 	if err != nil {
 		if err == graphdriver.ErrNotSupported {
 		if err == graphdriver.ErrNotSupported {
@@ -37,7 +39,7 @@ func testInit(dir string, t *testing.T) graphdriver.Driver {
 	return d
 	return d
 }
 }
 
 
-func newDriver(t *testing.T) *Driver {
+func newDriver(t testing.TB) *Driver {
 	if err := os.MkdirAll(tmp, 0755); err != nil {
 	if err := os.MkdirAll(tmp, 0755); err != nil {
 		t.Fatal(err)
 		t.Fatal(err)
 	}
 	}
@@ -732,3 +734,68 @@ func TestMountMoreThan42LayersMatchingPathLength(t *testing.T) {
 		zeroes += "0"
 		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()
+		}
+	}
+}