Browse Source

layer: optimize layerStore mountL

Goroutine stack analisys shown some lock contention
while doing massively (100 instances of `docker rm`)
parallel image removal, with many goroutines waiting
for the mountL mutex. Optimize it.

With this commit, the above operation is about 3x
faster, with no noticeable change to container
creation times (tested on aufs and overlay2).

kolyshkin@:
- squashed commits
- added description
- protected CreateRWLayer against name collisions by
temporary assiging nil to ls.mounts[name], and treating
nil as "non-existent" in all the other functions.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Xinfeng Liu 6 years ago
parent
commit
05250a4f00
2 changed files with 35 additions and 21 deletions
  1. 32 18
      layer/layer_store.go
  2. 3 3
      layer/migration.go

+ 32 - 18
layer/layer_store.go

@@ -189,7 +189,9 @@ func (ls *layerStore) loadLayer(layer ChainID) (*roLayer, error) {
 }
 
 func (ls *layerStore) loadMount(mount string) error {
-	if _, ok := ls.mounts[mount]; ok {
+	ls.mountL.Lock()
+	defer ls.mountL.Unlock()
+	if m := ls.mounts[mount]; m != nil {
 		return nil
 	}
 
@@ -477,7 +479,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
@@ -491,13 +493,21 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
 	}
 
 	ls.mountL.Lock()
-	defer ls.mountL.Unlock()
-	m, ok := ls.mounts[name]
-	if ok {
+	if _, ok := ls.mounts[name]; ok {
+		ls.mountL.Unlock()
 		return nil, ErrMountNameConflict
 	}
+	// Avoid name collision by temporary assigning nil
+	ls.mounts[name] = nil
+	ls.mountL.Unlock()
+	defer func() {
+		if err != nil {
+			ls.mountL.Lock()
+			delete(ls.mounts, name)
+			ls.mountL.Unlock()
+		}
+	}()
 
-	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,10 +548,10 @@ 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
@@ -549,9 +559,9 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
 
 func (ls *layerStore) GetRWLayer(id string) (RWLayer, 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 nil, ErrMountDoesNotExist
 	}
 
@@ -561,8 +571,8 @@ 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]
+	if mount == nil {
 		return "", ErrMountDoesNotExist
 	}
 	logrus.Debugf("GetMountID id: %s -> mountID: %s", id, mount.mountID)
@@ -572,9 +582,9 @@ func (ls *layerStore) GetMountID(id string) (string, error) {
 
 func (ls *layerStore) ReleaseRWLayer(l RWLayer) ([]Metadata, error) {
 	ls.mountL.Lock()
-	defer ls.mountL.Unlock()
-	m, ok := ls.mounts[l.Name()]
-	if !ok {
+	m := ls.mounts[l.Name()]
+	ls.mountL.Unlock()
+	if m == nil {
 		return []Metadata{}, nil
 	}
 
@@ -606,7 +616,9 @@ func (ls *layerStore) ReleaseRWLayer(l RWLayer) ([]Metadata, error) {
 		return nil, err
 	}
 
+	ls.mountL.Lock()
 	delete(ls.mounts, m.Name())
+	ls.mountL.Unlock()
 
 	ls.layerL.Lock()
 	defer ls.layerL.Unlock()
@@ -634,7 +646,9 @@ func (ls *layerStore) saveMount(mount *mountedLayer) error {
 		}
 	}
 
+	ls.mountL.Lock()
 	ls.mounts[mount.name] = mount
+	ls.mountL.Unlock()
 
 	return nil
 }

+ 3 - 3
layer/migration.go

@@ -18,9 +18,9 @@ import (
 // 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 {
+	m := ls.mounts[name]
+	ls.mountL.Unlock()
+	if m != nil {
 		if m.parent.chainID != parent {
 			return errors.New("name conflict, mismatched parent")
 		}