Merge pull request #247 from thaJeztah/19.03_aufs_backports

[19.03 backport] backport layer store optimizations

layer/layer_store.go

@@ -10,6 +10,7 @@ import (
 	"github.com/docker/distribution"
 	"github.com/docker/docker/daemon/graphdriver"
 	"github.com/docker/docker/pkg/idtools"
+	"github.com/docker/docker/pkg/locker"
 	"github.com/docker/docker/pkg/plugingetter"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/docker/pkg/system"
@@ -36,7 +37,11 @@ type layerStore struct {
 
 	mounts map[string]*mountedLayer
 	mountL sync.Mutex
-	os     string
+
+	// protect *RWLayer() methods from operating on the same name/id
+	locker *locker.Locker
+
+	os string
 }
 
 // StoreOptions are the options used to create a new Store instance
@@ -92,6 +97,7 @@ func newStoreFromGraphDriver(root string, driver graphdriver.Driver, os string)
 		driver:      driver,
 		layerMap:    map[ChainID]*roLayer{},
 		mounts:      map[string]*mountedLayer{},
+		locker:      locker.New(),
 		useTarSplit: !caps.ReproducesExactDiffs,
 		os:          os,
 	}
@@ -189,6 +195,8 @@ func (ls *layerStore) loadLayer(layer ChainID) (*roLayer, error) {
 }
 
 func (ls *layerStore) loadMount(mount string) error {
+	ls.mountL.Lock()
+	defer ls.mountL.Unlock()
 	if _, ok := ls.mounts[mount]; ok {
 		return nil
 	}
@@ -477,7 +485,7 @@ func (ls *layerStore) Release(l Layer) ([]Metadata, error) {
 	return ls.releaseLayer(layer)
 }
 
-func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWLayerOpts) (RWLayer, error) {
+func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWLayerOpts) (_ RWLayer, err error) {
 	var (
 		storageOpt map[string]string
 		initFunc   MountInit
@@ -490,14 +498,16 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
 		initFunc = opts.InitFunc
 	}
 
+	ls.locker.Lock(name)
+	defer ls.locker.Unlock(name)
+
 	ls.mountL.Lock()
-	defer ls.mountL.Unlock()
-	m, ok := ls.mounts[name]
+	_, ok := ls.mounts[name]
+	ls.mountL.Unlock()
 	if ok {
 		return nil, ErrMountNameConflict
 	}
 
-	var err error
 	var pid string
 	var p *roLayer
 	if string(parent) != "" {
@@ -517,7 +527,7 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
 		}()
 	}
 
-	m = &mountedLayer{
+	m := &mountedLayer{
 		name:       name,
 		parent:     p,
 		mountID:    ls.mountID(name),
@@ -528,7 +538,7 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
 	if initFunc != nil {
 		pid, err = ls.initMount(m.mountID, pid, mountLabel, initFunc, storageOpt)
 		if err != nil {
-			return nil, err
+			return
 		}
 		m.initID = pid
 	}
@@ -538,20 +548,23 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
 	}
 
 	if err = ls.driver.CreateReadWrite(m.mountID, pid, createOpts); err != nil {
-		return nil, err
+		return
 	}
 	if err = ls.saveMount(m); err != nil {
-		return nil, err
+		return
 	}
 
 	return m.getReference(), nil
 }
 
 func (ls *layerStore) GetRWLayer(id string) (RWLayer, error) {
+	ls.locker.Lock(id)
+	defer ls.locker.Unlock(id)
+
 	ls.mountL.Lock()
-	defer ls.mountL.Unlock()
-	mount, ok := ls.mounts[id]
-	if !ok {
+	mount := ls.mounts[id]
+	ls.mountL.Unlock()
+	if mount == nil {
 		return nil, ErrMountDoesNotExist
 	}
 
@@ -560,9 +573,10 @@ func (ls *layerStore) GetRWLayer(id string) (RWLayer, error) {
 
 func (ls *layerStore) GetMountID(id string) (string, error) {
 	ls.mountL.Lock()
-	defer ls.mountL.Unlock()
-	mount, ok := ls.mounts[id]
-	if !ok {
+	mount := ls.mounts[id]
+	ls.mountL.Unlock()
+
+	if mount == nil {
 		return "", ErrMountDoesNotExist
 	}
 	logrus.Debugf("GetMountID id: %s -> mountID: %s", id, mount.mountID)
@@ -571,10 +585,14 @@ func (ls *layerStore) GetMountID(id string) (string, error) {
 }
 
 func (ls *layerStore) ReleaseRWLayer(l RWLayer) ([]Metadata, error) {
+	name := l.Name()
+	ls.locker.Lock(name)
+	defer ls.locker.Unlock(name)
+
 	ls.mountL.Lock()
-	defer ls.mountL.Unlock()
-	m, ok := ls.mounts[l.Name()]
-	if !ok {
+	m := ls.mounts[name]
+	ls.mountL.Unlock()
+	if m == nil {
 		return []Metadata{}, nil
 	}
 
@@ -606,7 +624,9 @@ func (ls *layerStore) ReleaseRWLayer(l RWLayer) ([]Metadata, error) {
 		return nil, err
 	}
 
-	delete(ls.mounts, m.Name())
+	ls.mountL.Lock()
+	delete(ls.mounts, name)
+	ls.mountL.Unlock()
 
 	ls.layerL.Lock()
 	defer ls.layerL.Unlock()
@@ -634,7 +654,9 @@ func (ls *layerStore) saveMount(mount *mountedLayer) error {
 		}
 	}
 
+	ls.mountL.Lock()
 	ls.mounts[mount.name] = mount
+	ls.mountL.Unlock()
 
 	return nil
 }

layer/migration.go

@@ -3,7 +3,6 @@ package layer // import "github.com/docker/docker/layer"
 import (
 	"compress/gzip"
 	"errors"
-	"fmt"
 	"io"
 	"os"
 
@@ -13,64 +12,6 @@ import (
 	"github.com/vbatts/tar-split/tar/storage"
 )
 
-// CreateRWLayerByGraphID creates a RWLayer in the layer store using
-// the provided name with the given graphID. To get the RWLayer
-// after migration the layer may be retrieved by the given name.
-func (ls *layerStore) CreateRWLayerByGraphID(name, graphID string, parent ChainID) (err error) {
-	ls.mountL.Lock()
-	defer ls.mountL.Unlock()
-	m, ok := ls.mounts[name]
-	if ok {
-		if m.parent.chainID != parent {
-			return errors.New("name conflict, mismatched parent")
-		}
-		if m.mountID != graphID {
-			return errors.New("mount already exists")
-		}
-
-		return nil
-	}
-
-	if !ls.driver.Exists(graphID) {
-		return fmt.Errorf("graph ID does not exist: %q", graphID)
-	}
-
-	var p *roLayer
-	if string(parent) != "" {
-		p = ls.get(parent)
-		if p == nil {
-			return ErrLayerDoesNotExist
-		}
-
-		// Release parent chain if error
-		defer func() {
-			if err != nil {
-				ls.layerL.Lock()
-				ls.releaseLayer(p)
-				ls.layerL.Unlock()
-			}
-		}()
-	}
-
-	// TODO: Ensure graphID has correct parent
-
-	m = &mountedLayer{
-		name:       name,
-		parent:     p,
-		mountID:    graphID,
-		layerStore: ls,
-		references: map[RWLayer]*referencedRWLayer{},
-	}
-
-	// Check for existing init layer
-	initID := fmt.Sprintf("%s-init", graphID)
-	if ls.driver.Exists(initID) {
-		m.initID = initID
-	}
-
-	return ls.saveMount(m)
-}
-
 func (ls *layerStore) ChecksumForGraphID(id, parent, oldTarDataPath, newTarDataPath string) (diffID DiffID, size int64, err error) {
 	defer func() {
 		if err != nil {

layer/migration_test.go

@@ -3,7 +3,6 @@ package layer // import "github.com/docker/docker/layer"
 import (
 	"bytes"
 	"compress/gzip"
-	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
@@ -12,7 +11,6 @@ import (
 	"testing"
 
 	"github.com/docker/docker/daemon/graphdriver"
-	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/vbatts/tar-split/tar/asm"
 	"github.com/vbatts/tar-split/tar/storage"
@@ -269,161 +267,3 @@ func TestLayerMigrationNoTarsplit(t *testing.T) {
 
 	assertMetadata(t, metadata, createMetadata(layer2a))
 }
-
-func TestMountMigration(t *testing.T) {
-	// TODO Windows: Figure out why this is failing (obvious - paths... needs porting)
-	if runtime.GOOS == "windows" {
-		t.Skip("Failing on Windows")
-	}
-	ls, _, cleanup := newTestStore(t)
-	defer cleanup()
-
-	baseFiles := []FileApplier{
-		newTestFile("/root/.bashrc", []byte("# Boring configuration"), 0644),
-		newTestFile("/etc/profile", []byte("# Base configuration"), 0644),
-	}
-	initFiles := []FileApplier{
-		newTestFile("/etc/hosts", []byte{}, 0644),
-		newTestFile("/etc/resolv.conf", []byte{}, 0644),
-	}
-	mountFiles := []FileApplier{
-		newTestFile("/etc/hosts", []byte("localhost 127.0.0.1"), 0644),
-		newTestFile("/root/.bashrc", []byte("# Updated configuration"), 0644),
-		newTestFile("/root/testfile1.txt", []byte("nothing valuable"), 0644),
-	}
-
-	initTar, err := tarFromFiles(initFiles...)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	mountTar, err := tarFromFiles(mountFiles...)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	graph := ls.(*layerStore).driver
-
-	layer1, err := createLayer(ls, "", initWithFiles(baseFiles...))
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	graphID1 := layer1.(*referencedCacheLayer).cacheID
-
-	containerID := stringid.GenerateRandomID()
-	containerInit := fmt.Sprintf("%s-init", containerID)
-
-	if err := graph.Create(containerInit, graphID1, nil); err != nil {
-		t.Fatal(err)
-	}
-	if _, err := graph.ApplyDiff(containerInit, graphID1, bytes.NewReader(initTar)); err != nil {
-		t.Fatal(err)
-	}
-
-	if err := graph.Create(containerID, containerInit, nil); err != nil {
-		t.Fatal(err)
-	}
-	if _, err := graph.ApplyDiff(containerID, containerInit, bytes.NewReader(mountTar)); err != nil {
-		t.Fatal(err)
-	}
-
-	if err := ls.(*layerStore).CreateRWLayerByGraphID("migration-mount", containerID, layer1.ChainID()); err != nil {
-		t.Fatal(err)
-	}
-
-	rwLayer1, err := ls.GetRWLayer("migration-mount")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if _, err := rwLayer1.Mount(""); err != nil {
-		t.Fatal(err)
-	}
-
-	changes, err := rwLayer1.Changes()
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if expected := 5; len(changes) != expected {
-		t.Logf("Changes %#v", changes)
-		t.Fatalf("Wrong number of changes %d, expected %d", len(changes), expected)
-	}
-
-	sortChanges(changes)
-
-	assertChange(t, changes[0], archive.Change{
-		Path: "/etc",
-		Kind: archive.ChangeModify,
-	})
-	assertChange(t, changes[1], archive.Change{
-		Path: "/etc/hosts",
-		Kind: archive.ChangeModify,
-	})
-	assertChange(t, changes[2], archive.Change{
-		Path: "/root",
-		Kind: archive.ChangeModify,
-	})
-	assertChange(t, changes[3], archive.Change{
-		Path: "/root/.bashrc",
-		Kind: archive.ChangeModify,
-	})
-	assertChange(t, changes[4], archive.Change{
-		Path: "/root/testfile1.txt",
-		Kind: archive.ChangeAdd,
-	})
-
-	if _, err := ls.CreateRWLayer("migration-mount", layer1.ChainID(), nil); err == nil {
-		t.Fatal("Expected error creating mount with same name")
-	} else if err != ErrMountNameConflict {
-		t.Fatal(err)
-	}
-
-	rwLayer2, err := ls.GetRWLayer("migration-mount")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if getMountLayer(rwLayer1) != getMountLayer(rwLayer2) {
-		t.Fatal("Expected same layer from get with same name as from migrate")
-	}
-
-	if _, err := rwLayer2.Mount(""); err != nil {
-		t.Fatal(err)
-	}
-
-	if _, err := rwLayer2.Mount(""); err != nil {
-		t.Fatal(err)
-	}
-
-	if metadata, err := ls.Release(layer1); err != nil {
-		t.Fatal(err)
-	} else if len(metadata) > 0 {
-		t.Fatalf("Expected no layers to be deleted, deleted %#v", metadata)
-	}
-
-	if err := rwLayer1.Unmount(); err != nil {
-		t.Fatal(err)
-	}
-
-	if _, err := ls.ReleaseRWLayer(rwLayer1); err != nil {
-		t.Fatal(err)
-	}
-
-	if err := rwLayer2.Unmount(); err != nil {
-		t.Fatal(err)
-	}
-	if err := rwLayer2.Unmount(); err != nil {
-		t.Fatal(err)
-	}
-	metadata, err := ls.ReleaseRWLayer(rwLayer2)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if len(metadata) == 0 {
-		t.Fatal("Expected base layer to be deleted when deleting mount")
-	}
-
-	assertMetadata(t, metadata, createMetadata(layer1))
-}

layer/mounted_layer.go

@@ -2,6 +2,7 @@ package layer // import "github.com/docker/docker/layer"
 
 import (
 	"io"
+	"sync"
 
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/containerfs"
@@ -15,6 +16,7 @@ type mountedLayer struct {
 	path       string
 	layerStore *layerStore
 
+	sync.Mutex
 	references map[RWLayer]*referencedRWLayer
 }
 
@@ -62,16 +64,24 @@ func (ml *mountedLayer) getReference() RWLayer {
 	ref := &referencedRWLayer{
 		mountedLayer: ml,
 	}
+	ml.Lock()
 	ml.references[ref] = ref
+	ml.Unlock()
 
 	return ref
 }
 
 func (ml *mountedLayer) hasReferences() bool {
-	return len(ml.references) > 0
+	ml.Lock()
+	ret := len(ml.references) > 0
+	ml.Unlock()
+
+	return ret
 }
 
 func (ml *mountedLayer) deleteReference(ref RWLayer) error {
+	ml.Lock()
+	defer ml.Unlock()
 	if _, ok := ml.references[ref]; !ok {
 		return ErrLayerNotRetained
 	}
@@ -81,7 +91,9 @@ func (ml *mountedLayer) deleteReference(ref RWLayer) error {
 
 func (ml *mountedLayer) retakeReference(r RWLayer) {
 	if ref, ok := r.(*referencedRWLayer); ok {
+		ml.Lock()
 		ml.references[ref] = ref
+		ml.Unlock()
 	}
 }