Browse code

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>
(cherry picked from commit 05250a4f0094e6802dd7d338d632ea632d0c7e34)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Xinfeng Liu authored on 2019/04/23 11:33:20
Showing 2 changed files
... ...
@@ -189,7 +189,9 @@ func (ls *layerStore) loadLayer(layer ChainID) (*roLayer, error) {
189 189
 }
190 190
 
191 191
 func (ls *layerStore) loadMount(mount string) error {
192
-	if _, ok := ls.mounts[mount]; ok {
192
+	ls.mountL.Lock()
193
+	defer ls.mountL.Unlock()
194
+	if m := ls.mounts[mount]; m != nil {
193 195
 		return nil
194 196
 	}
195 197
 
... ...
@@ -477,7 +479,7 @@ func (ls *layerStore) Release(l Layer) ([]Metadata, error) {
477 477
 	return ls.releaseLayer(layer)
478 478
 }
479 479
 
480
-func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWLayerOpts) (RWLayer, error) {
480
+func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWLayerOpts) (_ RWLayer, err error) {
481 481
 	var (
482 482
 		storageOpt map[string]string
483 483
 		initFunc   MountInit
... ...
@@ -491,13 +493,21 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
491 491
 	}
492 492
 
493 493
 	ls.mountL.Lock()
494
-	defer ls.mountL.Unlock()
495
-	m, ok := ls.mounts[name]
496
-	if ok {
494
+	if _, ok := ls.mounts[name]; ok {
495
+		ls.mountL.Unlock()
497 496
 		return nil, ErrMountNameConflict
498 497
 	}
498
+	// Avoid name collision by temporary assigning nil
499
+	ls.mounts[name] = nil
500
+	ls.mountL.Unlock()
501
+	defer func() {
502
+		if err != nil {
503
+			ls.mountL.Lock()
504
+			delete(ls.mounts, name)
505
+			ls.mountL.Unlock()
506
+		}
507
+	}()
499 508
 
500
-	var err error
501 509
 	var pid string
502 510
 	var p *roLayer
503 511
 	if string(parent) != "" {
... ...
@@ -517,7 +527,7 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
517 517
 		}()
518 518
 	}
519 519
 
520
-	m = &mountedLayer{
520
+	m := &mountedLayer{
521 521
 		name:       name,
522 522
 		parent:     p,
523 523
 		mountID:    ls.mountID(name),
... ...
@@ -528,7 +538,7 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
528 528
 	if initFunc != nil {
529 529
 		pid, err = ls.initMount(m.mountID, pid, mountLabel, initFunc, storageOpt)
530 530
 		if err != nil {
531
-			return nil, err
531
+			return
532 532
 		}
533 533
 		m.initID = pid
534 534
 	}
... ...
@@ -538,10 +548,10 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
538 538
 	}
539 539
 
540 540
 	if err = ls.driver.CreateReadWrite(m.mountID, pid, createOpts); err != nil {
541
-		return nil, err
541
+		return
542 542
 	}
543 543
 	if err = ls.saveMount(m); err != nil {
544
-		return nil, err
544
+		return
545 545
 	}
546 546
 
547 547
 	return m.getReference(), nil
... ...
@@ -549,9 +559,9 @@ func (ls *layerStore) CreateRWLayer(name string, parent ChainID, opts *CreateRWL
549 549
 
550 550
 func (ls *layerStore) GetRWLayer(id string) (RWLayer, error) {
551 551
 	ls.mountL.Lock()
552
-	defer ls.mountL.Unlock()
553
-	mount, ok := ls.mounts[id]
554
-	if !ok {
552
+	mount := ls.mounts[id]
553
+	ls.mountL.Unlock()
554
+	if mount == nil {
555 555
 		return nil, ErrMountDoesNotExist
556 556
 	}
557 557
 
... ...
@@ -561,8 +571,8 @@ func (ls *layerStore) GetRWLayer(id string) (RWLayer, error) {
561 561
 func (ls *layerStore) GetMountID(id string) (string, error) {
562 562
 	ls.mountL.Lock()
563 563
 	defer ls.mountL.Unlock()
564
-	mount, ok := ls.mounts[id]
565
-	if !ok {
564
+	mount := ls.mounts[id]
565
+	if mount == nil {
566 566
 		return "", ErrMountDoesNotExist
567 567
 	}
568 568
 	logrus.Debugf("GetMountID id: %s -> mountID: %s", id, mount.mountID)
... ...
@@ -572,9 +582,9 @@ func (ls *layerStore) GetMountID(id string) (string, error) {
572 572
 
573 573
 func (ls *layerStore) ReleaseRWLayer(l RWLayer) ([]Metadata, error) {
574 574
 	ls.mountL.Lock()
575
-	defer ls.mountL.Unlock()
576
-	m, ok := ls.mounts[l.Name()]
577
-	if !ok {
575
+	m := ls.mounts[l.Name()]
576
+	ls.mountL.Unlock()
577
+	if m == nil {
578 578
 		return []Metadata{}, nil
579 579
 	}
580 580
 
... ...
@@ -606,7 +616,9 @@ func (ls *layerStore) ReleaseRWLayer(l RWLayer) ([]Metadata, error) {
606 606
 		return nil, err
607 607
 	}
608 608
 
609
+	ls.mountL.Lock()
609 610
 	delete(ls.mounts, m.Name())
611
+	ls.mountL.Unlock()
610 612
 
611 613
 	ls.layerL.Lock()
612 614
 	defer ls.layerL.Unlock()
... ...
@@ -634,7 +646,9 @@ func (ls *layerStore) saveMount(mount *mountedLayer) error {
634 634
 		}
635 635
 	}
636 636
 
637
+	ls.mountL.Lock()
637 638
 	ls.mounts[mount.name] = mount
639
+	ls.mountL.Unlock()
638 640
 
639 641
 	return nil
640 642
 }
... ...
@@ -18,9 +18,9 @@ import (
18 18
 // after migration the layer may be retrieved by the given name.
19 19
 func (ls *layerStore) CreateRWLayerByGraphID(name, graphID string, parent ChainID) (err error) {
20 20
 	ls.mountL.Lock()
21
-	defer ls.mountL.Unlock()
22
-	m, ok := ls.mounts[name]
23
-	if ok {
21
+	m := ls.mounts[name]
22
+	ls.mountL.Unlock()
23
+	if m != nil {
24 24
 		if m.parent.chainID != parent {
25 25
 			return errors.New("name conflict, mismatched parent")
26 26
 		}