From 1128fc1add66a849c12d2045aed39605e673abc6 Mon Sep 17 00:00:00 2001
From: Aaron Lehmann <aaron.lehmann@docker.com>
Date: Thu, 29 Jun 2017 18:56:22 -0700
Subject: [PATCH 1/3] Store container names in memdb

Currently, names are maintained by a separate system called "registrar".
This means there is no way to atomically snapshot the state of
containers and the names associated with them.

We can add this atomicity and simplify the code by storing name
associations in the memdb. This removes the need for pkg/registrar, and
makes snapshots a lot less expensive because they no longer need to copy
all the names. This change also avoids some problematic behavior from
pkg/registrar where it returns slices which may be modified later on.

Note that while this change makes the *snapshotting* atomic, it doesn't
yet do anything to make sure containers are named at the same time that
they are added to the database. We can do that by adding a transactional
interface, either as a followup, or as part of this PR.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
---
 container/view.go               | 223 +++++++++++++++++++++++++++++---
 container/view_test.go          |  57 +++++++-
 daemon/container.go             |   2 +-
 daemon/daemon.go                |   7 +-
 daemon/daemon_test.go           |  12 +-
 daemon/delete.go                |   3 +-
 daemon/list.go                  |   4 +-
 daemon/names.go                 |  17 ++-
 daemon/rename.go                |   8 +-
 pkg/registrar/registrar.go      | 130 -------------------
 pkg/registrar/registrar_test.go | 119 -----------------
 11 files changed, 285 insertions(+), 297 deletions(-)
 delete mode 100644 pkg/registrar/registrar.go
 delete mode 100644 pkg/registrar/registrar_test.go

diff --git a/container/view.go b/container/view.go
index f605b4f483..449cade149 100644
--- a/container/view.go
+++ b/container/view.go
@@ -1,6 +1,7 @@
 package container
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 	"time"
@@ -8,14 +9,23 @@ import (
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/network"
-	"github.com/docker/docker/pkg/registrar"
 	"github.com/docker/go-connections/nat"
 	"github.com/hashicorp/go-memdb"
 )
 
 const (
-	memdbTable   = "containers"
-	memdbIDIndex = "id"
+	memdbContainersTable = "containers"
+	memdbNamesTable      = "names"
+
+	memdbIDIndex          = "id"
+	memdbContainerIDIndex = "containerid"
+)
+
+var (
+	// ErrNameReserved is an error which is returned when a name is requested to be reserved that already is reserved
+	ErrNameReserved = errors.New("name is reserved")
+	// ErrNameNotReserved is an error which is returned when trying to find a name that is not reserved
+	ErrNameNotReserved = errors.New("name is not reserved")
 )
 
 // Snapshot is a read only view for Containers. It holds all information necessary to serve container queries in a
@@ -41,23 +51,37 @@ type Snapshot struct {
 	}
 }
 
+// nameAssociation associates a container id with a name.
+type nameAssociation struct {
+	// name is the name to associate. Note that name is the primary key
+	// ("id" in memdb).
+	name        string
+	containerID string
+}
+
 // ViewDB provides an in-memory transactional (ACID) container Store
 type ViewDB interface {
-	Snapshot(nameIndex *registrar.Registrar) View
+	Snapshot() View
 	Save(*Container) error
 	Delete(*Container) error
+
+	ReserveName(name, containerID string) error
+	ReleaseName(name string)
 }
 
 // View can be used by readers to avoid locking
 type View interface {
 	All() ([]Snapshot, error)
 	Get(id string) (*Snapshot, error)
+
+	GetID(name string) (string, error)
+	GetAllNames() map[string][]string
 }
 
 var schema = &memdb.DBSchema{
 	Tables: map[string]*memdb.TableSchema{
-		memdbTable: {
-			Name: memdbTable,
+		memdbContainersTable: {
+			Name: memdbContainersTable,
 			Indexes: map[string]*memdb.IndexSchema{
 				memdbIDIndex: {
 					Name:    memdbIDIndex,
@@ -66,6 +90,21 @@ var schema = &memdb.DBSchema{
 				},
 			},
 		},
+		memdbNamesTable: {
+			Name: memdbNamesTable,
+			Indexes: map[string]*memdb.IndexSchema{
+				// Used for names, because "id" is the primary key in memdb.
+				memdbIDIndex: {
+					Name:    memdbIDIndex,
+					Unique:  true,
+					Indexer: &namesByNameIndexer{},
+				},
+				memdbContainerIDIndex: {
+					Name:    memdbContainerIDIndex,
+					Indexer: &namesByContainerIDIndexer{},
+				},
+			},
+		},
 	},
 }
 
@@ -94,10 +133,9 @@ func NewViewDB() (ViewDB, error) {
 }
 
 // Snapshot provides a consistent read-only View of the database
-func (db *memDB) Snapshot(index *registrar.Registrar) View {
+func (db *memDB) Snapshot() View {
 	return &memdbView{
-		txn:       db.store.Txn(false),
-		nameIndex: index.GetAll(),
+		txn: db.store.Txn(false),
 	}
 }
 
@@ -106,25 +144,75 @@ func (db *memDB) Snapshot(index *registrar.Registrar) View {
 func (db *memDB) Save(c *Container) error {
 	txn := db.store.Txn(true)
 	defer txn.Commit()
-	return txn.Insert(memdbTable, c)
+	return txn.Insert(memdbContainersTable, c)
 }
 
 // Delete removes an item by ID
 func (db *memDB) Delete(c *Container) error {
 	txn := db.store.Txn(true)
 	defer txn.Commit()
-	return txn.Delete(memdbTable, NewBaseContainer(c.ID, c.Root))
+
+	// Delete any names referencing this container's ID.
+	iter, err := txn.Get(memdbNamesTable, memdbContainerIDIndex, c.ID)
+	if err != nil {
+		return err
+	}
+
+	var names []string
+	for {
+		item := iter.Next()
+		if item == nil {
+			break
+		}
+		names = append(names, item.(nameAssociation).name)
+	}
+
+	for _, name := range names {
+		txn.Delete(memdbNamesTable, nameAssociation{name: name})
+	}
+
+	return txn.Delete(memdbContainersTable, NewBaseContainer(c.ID, c.Root))
+}
+
+// ReserveName registers a container ID to a name
+// ReserveName is idempotent
+// Attempting to reserve a container ID to a name that already exists results in an `ErrNameReserved`
+// A name reservation is globally unique
+func (db *memDB) ReserveName(name, containerID string) error {
+	txn := db.store.Txn(true)
+	defer txn.Commit()
+
+	s, err := txn.First(memdbNamesTable, memdbIDIndex, name)
+	if err != nil {
+		return err
+	}
+	if s != nil {
+		if s.(nameAssociation).containerID != containerID {
+			return ErrNameReserved
+		}
+		return nil
+	}
+
+	txn.Insert(memdbNamesTable, nameAssociation{name: name, containerID: containerID})
+	return nil
+}
+
+// ReleaseName releases the reserved name
+// Once released, a name can be reserved again
+func (db *memDB) ReleaseName(name string) {
+	txn := db.store.Txn(true)
+	txn.Delete(memdbNamesTable, nameAssociation{name: name})
+	txn.Commit()
 }
 
 type memdbView struct {
-	txn       *memdb.Txn
-	nameIndex map[string][]string
+	txn *memdb.Txn
 }
 
 // All returns a all items in this snapshot. Returned objects must never be modified.
 func (v *memdbView) All() ([]Snapshot, error) {
 	var all []Snapshot
-	iter, err := v.txn.Get(memdbTable, memdbIDIndex)
+	iter, err := v.txn.Get(memdbContainersTable, memdbIDIndex)
 	if err != nil {
 		return nil, err
 	}
@@ -141,7 +229,7 @@ func (v *memdbView) All() ([]Snapshot, error) {
 
 // Get returns an item by id. Returned objects must never be modified.
 func (v *memdbView) Get(id string) (*Snapshot, error) {
-	s, err := v.txn.First(memdbTable, memdbIDIndex, id)
+	s, err := v.txn.First(memdbContainersTable, memdbIDIndex, id)
 	if err != nil {
 		return nil, err
 	}
@@ -151,13 +239,64 @@ func (v *memdbView) Get(id string) (*Snapshot, error) {
 	return v.transform(s.(*Container)), nil
 }
 
+// getNames lists all the reserved names for the given container ID.
+func (v *memdbView) getNames(containerID string) []string {
+	iter, err := v.txn.Get(memdbNamesTable, memdbContainerIDIndex, containerID)
+	if err != nil {
+		return nil
+	}
+
+	var names []string
+	for {
+		item := iter.Next()
+		if item == nil {
+			break
+		}
+		names = append(names, item.(nameAssociation).name)
+	}
+
+	return names
+}
+
+// GetID returns the container ID that the passed in name is reserved to.
+func (v *memdbView) GetID(name string) (string, error) {
+	s, err := v.txn.First(memdbNamesTable, memdbIDIndex, name)
+	if err != nil {
+		return "", err
+	}
+	if s == nil {
+		return "", ErrNameNotReserved
+	}
+	return s.(nameAssociation).containerID, nil
+}
+
+// GetAllNames returns all registered names.
+func (v *memdbView) GetAllNames() map[string][]string {
+	iter, err := v.txn.Get(memdbNamesTable, memdbContainerIDIndex)
+	if err != nil {
+		return nil
+	}
+
+	out := make(map[string][]string)
+	for {
+		item := iter.Next()
+		if item == nil {
+			break
+		}
+		assoc := item.(nameAssociation)
+		out[assoc.containerID] = append(out[assoc.containerID], assoc.name)
+	}
+
+	return out
+}
+
 // transform maps a (deep) copied Container object to what queries need.
 // A lock on the Container is not held because these are immutable deep copies.
 func (v *memdbView) transform(container *Container) *Snapshot {
 	snapshot := &Snapshot{
 		Container: types.Container{
 			ID:      container.ID,
-			Names:   v.nameIndex[container.ID],
+			Names:   v.getNames(container.ID),
 			ImageID: container.ImageID.String(),
 			Ports:   []types.Port{},
 			Mounts:  container.GetMountPoints(),
@@ -300,3 +439,55 @@ func (e *containerByIDIndexer) FromArgs(args ...interface{}) ([]byte, error) {
 	arg += "\x00"
 	return []byte(arg), nil
 }
+
+// namesByNameIndexer is used to index container name associations by name.
+type namesByNameIndexer struct{}
+
+func (e *namesByNameIndexer) FromObject(obj interface{}) (bool, []byte, error) {
+	n, ok := obj.(nameAssociation)
+	if !ok {
+		return false, nil, fmt.Errorf(`%T does not have type "nameAssociation"`, obj)
+	}
+
+	// Add the null character as a terminator
+	return true, []byte(n.name + "\x00"), nil
+}
+
+func (e *namesByNameIndexer) FromArgs(args ...interface{}) ([]byte, error) {
+	if len(args) != 1 {
+		return nil, fmt.Errorf("must provide only a single argument")
+	}
+	arg, ok := args[0].(string)
+	if !ok {
+		return nil, fmt.Errorf("argument must be a string: %#v", args[0])
+	}
+	// Add the null character as a terminator
+	arg += "\x00"
+	return []byte(arg), nil
+}
+
+// namesByContainerIDIndexer is used to index container names by container ID.
+type namesByContainerIDIndexer struct{}
+
+func (e *namesByContainerIDIndexer) FromObject(obj interface{}) (bool, []byte, error) {
+	n, ok := obj.(nameAssociation)
+	if !ok {
+		return false, nil, fmt.Errorf(`%T does not have type "nameAssocation"`, obj)
+	}
+
+	// Add the null character as a terminator
+	return true, []byte(n.containerID + "\x00"), nil
+}
+
+func (e *namesByContainerIDIndexer) FromArgs(args ...interface{}) ([]byte, error) {
+	if len(args) != 1 {
+		return nil, fmt.Errorf("must provide only a single argument")
+	}
+	arg, ok := args[0].(string)
+	if !ok {
+		return nil, fmt.Errorf("argument must be a string: %#v", args[0])
+	}
+	// Add the null character as a terminator
+	arg += "\x00"
+	return []byte(arg), nil
+}
diff --git a/container/view_test.go b/container/view_test.go
index 9b872998bd..2e81998ca4 100644
--- a/container/view_test.go
+++ b/container/view_test.go
@@ -7,8 +7,8 @@ import (
 	"testing"
 
 	containertypes "github.com/docker/docker/api/types/container"
-	"github.com/docker/docker/pkg/registrar"
 	"github.com/pborman/uuid"
+	"github.com/stretchr/testify/assert"
 )
 
 var root string
@@ -54,7 +54,6 @@ func TestViewSaveDelete(t *testing.T) {
 func TestViewAll(t *testing.T) {
 	var (
 		db, _ = NewViewDB()
-		names = registrar.NewRegistrar()
 		one   = newContainer(t)
 		two   = newContainer(t)
 	)
@@ -67,7 +66,7 @@ func TestViewAll(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	all, err := db.Snapshot(names).All()
+	all, err := db.Snapshot().All()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -89,14 +88,13 @@ func TestViewAll(t *testing.T) {
 func TestViewGet(t *testing.T) {
 	var (
 		db, _ = NewViewDB()
-		names = registrar.NewRegistrar()
 		one   = newContainer(t)
 	)
 	one.ImageID = "some-image-123"
 	if err := one.CheckpointTo(db); err != nil {
 		t.Fatal(err)
 	}
-	s, err := db.Snapshot(names).Get(one.ID)
+	s, err := db.Snapshot().Get(one.ID)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -104,3 +102,52 @@ func TestViewGet(t *testing.T) {
 		t.Fatalf("expected ImageID=some-image-123. Got: %v", s)
 	}
 }
+
+func TestNames(t *testing.T) {
+	db, err := NewViewDB()
+	if err != nil {
+		t.Fatal(err)
+	}
+	assert.NoError(t, db.ReserveName("name1", "containerid1"))
+	assert.NoError(t, db.ReserveName("name1", "containerid1")) // idempotent
+	assert.NoError(t, db.ReserveName("name2", "containerid2"))
+	assert.EqualError(t, db.ReserveName("name2", "containerid3"), ErrNameReserved.Error())
+
+	// Releasing a name allows the name to point to something else later.
+	db.ReleaseName("name2")
+	assert.NoError(t, db.ReserveName("name2", "containerid3"))
+
+	view := db.Snapshot()
+
+	id, err := view.GetID("name1")
+	assert.NoError(t, err)
+	assert.Equal(t, "containerid1", id)
+
+	id, err = view.GetID("name2")
+	assert.NoError(t, err)
+	assert.Equal(t, "containerid3", id)
+
+	_, err = view.GetID("notreserved")
+	assert.EqualError(t, err, ErrNameNotReserved.Error())
+
+	// Releasing and re-reserving a name doesn't affect the snapshot.
+	db.ReleaseName("name2")
+	assert.NoError(t, db.ReserveName("name2", "containerid4"))
+
+	id, err = view.GetID("name1")
+	assert.NoError(t, err)
+	assert.Equal(t, "containerid1", id)
+
+	id, err = view.GetID("name2")
+	assert.NoError(t, err)
+	assert.Equal(t, "containerid3", id)
+
+	// GetAllNames
+	assert.Equal(t, map[string][]string{"containerid1": {"name1"}, "containerid3": {"name2"}}, view.GetAllNames())
+
+	assert.NoError(t, db.ReserveName("name3", "containerid1"))
+	assert.NoError(t, db.ReserveName("name4", "containerid1"))
+
+	view = db.Snapshot()
+	assert.Equal(t, map[string][]string{"containerid1": {"name1", "name3", "name4"}, "containerid4": {"name2"}}, view.GetAllNames())
+}
diff --git a/daemon/container.go b/daemon/container.go
index 149df0dec6..4c015b70de 100644
--- a/daemon/container.go
+++ b/daemon/container.go
@@ -168,7 +168,7 @@ func (daemon *Daemon) GetByName(name string) (*container.Container, error) {
 	if name[0] != '/' {
 		fullName = "/" + name
 	}
-	id, err := daemon.nameIndex.Get(fullName)
+	id, err := daemon.containersReplica.Snapshot().GetID(fullName)
 	if err != nil {
 		return nil, fmt.Errorf("Could not find entity for %s", name)
 	}
diff --git a/daemon/daemon.go b/daemon/daemon.go
index 8359ef31ca..93d871d6df 100644
--- a/daemon/daemon.go
+++ b/daemon/daemon.go
@@ -42,7 +42,6 @@ import (
 	"github.com/docker/docker/migrate/v1"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/plugingetter"
-	"github.com/docker/docker/pkg/registrar"
 	"github.com/docker/docker/pkg/sysinfo"
 	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/pkg/truncindex"
@@ -104,7 +103,6 @@ type Daemon struct {
 	stores                map[string]daemonStore // By container target platform
 	PluginStore           *plugin.Store          // todo: remove
 	pluginManager         *plugin.Manager
-	nameIndex             *registrar.Registrar
 	linkIndex             *linkIndex
 	containerd            libcontainerd.Client
 	containerdRemote      libcontainerd.Remote
@@ -448,8 +446,8 @@ func (daemon *Daemon) parents(c *container.Container) map[string]*container.Cont
 
 func (daemon *Daemon) registerLink(parent, child *container.Container, alias string) error {
 	fullName := path.Join(parent.Name, alias)
-	if err := daemon.nameIndex.Reserve(fullName, child.ID); err != nil {
-		if err == registrar.ErrNameReserved {
+	if err := daemon.containersReplica.ReserveName(fullName, child.ID); err != nil {
+		if err == container.ErrNameReserved {
 			logrus.Warnf("error registering link for %s, to %s, as alias %s, ignoring: %v", parent.ID, child.ID, alias, err)
 			return nil
 		}
@@ -780,7 +778,6 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe
 	d.seccompEnabled = sysInfo.Seccomp
 	d.apparmorEnabled = sysInfo.AppArmor
 
-	d.nameIndex = registrar.NewRegistrar()
 	d.linkIndex = newLinkIndex()
 	d.containerdRemote = containerdRemote
 
diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go
index 6f07d0d1ee..13d1059c1c 100644
--- a/daemon/daemon_test.go
+++ b/daemon/daemon_test.go
@@ -12,7 +12,6 @@ import (
 	"github.com/docker/docker/container"
 	_ "github.com/docker/docker/pkg/discovery/memory"
 	"github.com/docker/docker/pkg/idtools"
-	"github.com/docker/docker/pkg/registrar"
 	"github.com/docker/docker/pkg/truncindex"
 	"github.com/docker/docker/volume"
 	volumedrivers "github.com/docker/docker/volume/drivers"
@@ -65,10 +64,15 @@ func TestGetContainer(t *testing.T) {
 	index.Add(c4.ID)
 	index.Add(c5.ID)
 
+	containersReplica, err := container.NewViewDB()
+	if err != nil {
+		t.Fatalf("could not create ViewDB: %v", err)
+	}
+
 	daemon := &Daemon{
-		containers: store,
-		idIndex:    index,
-		nameIndex:  registrar.NewRegistrar(),
+		containers:        store,
+		containersReplica: containersReplica,
+		idIndex:           index,
 	}
 
 	daemon.reserveName(c1.ID, c1.Name)
diff --git a/daemon/delete.go b/daemon/delete.go
index 2d3cd0f90f..c57a89654b 100644
--- a/daemon/delete.go
+++ b/daemon/delete.go
@@ -60,7 +60,7 @@ func (daemon *Daemon) rmLink(container *container.Container, name string) error
 	}
 
 	parent = strings.TrimSuffix(parent, "/")
-	pe, err := daemon.nameIndex.Get(parent)
+	pe, err := daemon.containersReplica.Snapshot().GetID(parent)
 	if err != nil {
 		return fmt.Errorf("Cannot get parent %s for name %s", parent, name)
 	}
@@ -128,7 +128,6 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo
 		return errors.Wrapf(err, "unable to remove filesystem for %s", container.ID)
 	}
 
-	daemon.nameIndex.Delete(container.ID)
 	daemon.linkIndex.delete(container)
 	selinuxFreeLxcContexts(container.ProcessLabel)
 	daemon.idIndex.Delete(container.ID)
diff --git a/daemon/list.go b/daemon/list.go
index b854be7549..6889c55889 100644
--- a/daemon/list.go
+++ b/daemon/list.go
@@ -182,7 +182,7 @@ func (daemon *Daemon) filterByNameIDMatches(view container.View, ctx *listContex
 // reduceContainers parses the user's filtering options and generates the list of containers to return based on a reducer.
 func (daemon *Daemon) reduceContainers(config *types.ContainerListOptions, reducer containerReducer) ([]*types.Container, error) {
 	var (
-		view       = daemon.containersReplica.Snapshot(daemon.nameIndex)
+		view       = daemon.containersReplica.Snapshot()
 		containers = []*types.Container{}
 	)
 
@@ -361,7 +361,7 @@ func (daemon *Daemon) foldFilter(view container.View, config *types.ContainerLis
 		publish:              publishFilter,
 		expose:               exposeFilter,
 		ContainerListOptions: config,
-		names:                daemon.nameIndex.GetAll(),
+		names:                view.GetAllNames(),
 	}, nil
 }
 func portOp(key string, filter map[nat.Port]bool) func(value string) error {
diff --git a/daemon/names.go b/daemon/names.go
index ec6ac2924f..7cdabeba9f 100644
--- a/daemon/names.go
+++ b/daemon/names.go
@@ -8,7 +8,6 @@ import (
 	"github.com/docker/docker/api"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/pkg/namesgenerator"
-	"github.com/docker/docker/pkg/registrar"
 	"github.com/docker/docker/pkg/stringid"
 )
 
@@ -31,7 +30,7 @@ func (daemon *Daemon) registerName(container *container.Container) error {
 		}
 		container.Name = name
 	}
-	return daemon.nameIndex.Reserve(container.Name, container.ID)
+	return daemon.containersReplica.ReserveName(container.Name, container.ID)
 }
 
 func (daemon *Daemon) generateIDAndName(name string) (string, string, error) {
@@ -62,9 +61,9 @@ func (daemon *Daemon) reserveName(id, name string) (string, error) {
 		name = "/" + name
 	}
 
-	if err := daemon.nameIndex.Reserve(name, id); err != nil {
-		if err == registrar.ErrNameReserved {
-			id, err := daemon.nameIndex.Get(name)
+	if err := daemon.containersReplica.ReserveName(name, id); err != nil {
+		if err == container.ErrNameReserved {
+			id, err := daemon.containersReplica.Snapshot().GetID(name)
 			if err != nil {
 				logrus.Errorf("got unexpected error while looking up reserved name: %v", err)
 				return "", err
@@ -77,7 +76,7 @@ func (daemon *Daemon) reserveName(id, name string) (string, error) {
 }
 
 func (daemon *Daemon) releaseName(name string) {
-	daemon.nameIndex.Release(name)
+	daemon.containersReplica.ReleaseName(name)
 }
 
 func (daemon *Daemon) generateNewName(id string) (string, error) {
@@ -88,8 +87,8 @@ func (daemon *Daemon) generateNewName(id string) (string, error) {
 			name = "/" + name
 		}
 
-		if err := daemon.nameIndex.Reserve(name, id); err != nil {
-			if err == registrar.ErrNameReserved {
+		if err := daemon.containersReplica.ReserveName(name, id); err != nil {
+			if err == container.ErrNameReserved {
 				continue
 			}
 			return "", err
@@ -98,7 +97,7 @@ func (daemon *Daemon) generateNewName(id string) (string, error) {
 	}
 
 	name = "/" + stringid.TruncateID(id)
-	if err := daemon.nameIndex.Reserve(name, id); err != nil {
+	if err := daemon.containersReplica.ReserveName(name, id); err != nil {
 		return "", err
 	}
 	return name, nil
diff --git a/daemon/rename.go b/daemon/rename.go
index 2a8d0b22c7..686fbd3b99 100644
--- a/daemon/rename.go
+++ b/daemon/rename.go
@@ -55,7 +55,7 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error {
 	}
 
 	for k, v := range links {
-		daemon.nameIndex.Reserve(newName+k, v.ID)
+		daemon.containersReplica.ReserveName(newName+k, v.ID)
 		daemon.linkIndex.link(container, v, newName+k)
 	}
 
@@ -68,10 +68,10 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error {
 			container.NetworkSettings.IsAnonymousEndpoint = oldIsAnonymousEndpoint
 			daemon.reserveName(container.ID, oldName)
 			for k, v := range links {
-				daemon.nameIndex.Reserve(oldName+k, v.ID)
+				daemon.containersReplica.ReserveName(oldName+k, v.ID)
 				daemon.linkIndex.link(container, v, oldName+k)
 				daemon.linkIndex.unlink(newName+k, v, container)
-				daemon.nameIndex.Release(newName + k)
+				daemon.containersReplica.ReleaseName(newName + k)
 			}
 			daemon.releaseName(newName)
 		}
@@ -79,7 +79,7 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error {
 
 	for k, v := range links {
 		daemon.linkIndex.unlink(oldName+k, v, container)
-		daemon.nameIndex.Release(oldName + k)
+		daemon.containersReplica.ReleaseName(oldName + k)
 	}
 	daemon.releaseName(oldName)
 	if err = container.CheckpointTo(daemon.containersReplica); err != nil {
diff --git a/pkg/registrar/registrar.go b/pkg/registrar/registrar.go
deleted file mode 100644
index df12db7eeb..0000000000
--- a/pkg/registrar/registrar.go
+++ /dev/null
@@ -1,130 +0,0 @@
-// Package registrar provides name registration. It reserves a name to a given key.
-package registrar
-
-import (
-	"errors"
-	"sync"
-)
-
-var (
-	// ErrNameReserved is an error which is returned when a name is requested to be reserved that already is reserved
-	ErrNameReserved = errors.New("name is reserved")
-	// ErrNameNotReserved is an error which is returned when trying to find a name that is not reserved
-	ErrNameNotReserved = errors.New("name is not reserved")
-	// ErrNoSuchKey is returned when trying to find the names for a key which is not known
-	ErrNoSuchKey = errors.New("provided key does not exist")
-)
-
-// Registrar stores indexes a list of keys and their registered names as well as indexes names and the key that they are registered to
-// Names must be unique.
-// Registrar is safe for concurrent access.
-type Registrar struct {
-	idx   map[string][]string
-	names map[string]string
-	mu    sync.Mutex
-}
-
-// NewRegistrar creates a new Registrar with the an empty index
-func NewRegistrar() *Registrar {
-	return &Registrar{
-		idx:   make(map[string][]string),
-		names: make(map[string]string),
-	}
-}
-
-// Reserve registers a key to a name
-// Reserve is idempotent
-// Attempting to reserve a key to a name that already exists results in an `ErrNameReserved`
-// A name reservation is globally unique
-func (r *Registrar) Reserve(name, key string) error {
-	r.mu.Lock()
-	defer r.mu.Unlock()
-
-	if k, exists := r.names[name]; exists {
-		if k != key {
-			return ErrNameReserved
-		}
-		return nil
-	}
-
-	r.idx[key] = append(r.idx[key], name)
-	r.names[name] = key
-	return nil
-}
-
-// Release releases the reserved name
-// Once released, a name can be reserved again
-func (r *Registrar) Release(name string) {
-	r.mu.Lock()
-	defer r.mu.Unlock()
-
-	key, exists := r.names[name]
-	if !exists {
-		return
-	}
-
-	for i, n := range r.idx[key] {
-		if n != name {
-			continue
-		}
-		r.idx[key] = append(r.idx[key][:i], r.idx[key][i+1:]...)
-		break
-	}
-
-	delete(r.names, name)
-
-	if len(r.idx[key]) == 0 {
-		delete(r.idx, key)
-	}
-}
-
-// Delete removes all reservations for the passed in key.
-// All names reserved to this key are released.
-func (r *Registrar) Delete(key string) {
-	r.mu.Lock()
-	for _, name := range r.idx[key] {
-		delete(r.names, name)
-	}
-	delete(r.idx, key)
-	r.mu.Unlock()
-}
-
-// GetNames lists all the reserved names for the given key
-func (r *Registrar) GetNames(key string) ([]string, error) {
-	r.mu.Lock()
-	defer r.mu.Unlock()
-
-	names, exists := r.idx[key]
-	if !exists {
-		return nil, ErrNoSuchKey
-	}
-
-	ls := make([]string, 0, len(names))
-	ls = append(ls, names...)
-	return ls, nil
-}
-
-// Get returns the key that the passed in name is reserved to
-func (r *Registrar) Get(name string) (string, error) {
-	r.mu.Lock()
-	key, exists := r.names[name]
-	r.mu.Unlock()
-
-	if !exists {
-		return "", ErrNameNotReserved
-	}
-	return key, nil
-}
-
-// GetAll returns all registered names
-func (r *Registrar) GetAll() map[string][]string {
-	out := make(map[string][]string)
-
-	r.mu.Lock()
-	// copy index into out
-	for id, names := range r.idx {
-		out[id] = names
-	}
-	r.mu.Unlock()
-	return out
-}
diff --git a/pkg/registrar/registrar_test.go b/pkg/registrar/registrar_test.go
deleted file mode 100644
index 70f8084b30..0000000000
--- a/pkg/registrar/registrar_test.go
+++ /dev/null
@@ -1,119 +0,0 @@
-package registrar
-
-import (
-	"reflect"
-	"testing"
-)
-
-func TestReserve(t *testing.T) {
-	r := NewRegistrar()
-
-	obj := "test1"
-	if err := r.Reserve("test", obj); err != nil {
-		t.Fatal(err)
-	}
-
-	if err := r.Reserve("test", obj); err != nil {
-		t.Fatal(err)
-	}
-
-	obj2 := "test2"
-	err := r.Reserve("test", obj2)
-	if err == nil {
-		t.Fatalf("expected error when reserving an already reserved name to another object")
-	}
-	if err != ErrNameReserved {
-		t.Fatal("expected `ErrNameReserved` error when attempting to reserve an already reserved name")
-	}
-}
-
-func TestRelease(t *testing.T) {
-	r := NewRegistrar()
-	obj := "testing"
-
-	if err := r.Reserve("test", obj); err != nil {
-		t.Fatal(err)
-	}
-	r.Release("test")
-	r.Release("test") // Ensure there is no panic here
-
-	if err := r.Reserve("test", obj); err != nil {
-		t.Fatal(err)
-	}
-}
-
-func TestGetNames(t *testing.T) {
-	r := NewRegistrar()
-	obj := "testing"
-	names := []string{"test1", "test2"}
-
-	for _, name := range names {
-		if err := r.Reserve(name, obj); err != nil {
-			t.Fatal(err)
-		}
-	}
-	r.Reserve("test3", "other")
-
-	names2, err := r.GetNames(obj)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if !reflect.DeepEqual(names, names2) {
-		t.Fatalf("Expected: %v, Got: %v", names, names2)
-	}
-}
-
-func TestDelete(t *testing.T) {
-	r := NewRegistrar()
-	obj := "testing"
-	names := []string{"test1", "test2"}
-	for _, name := range names {
-		if err := r.Reserve(name, obj); err != nil {
-			t.Fatal(err)
-		}
-	}
-
-	r.Reserve("test3", "other")
-	r.Delete(obj)
-
-	_, err := r.GetNames(obj)
-	if err == nil {
-		t.Fatal("expected error getting names for deleted key")
-	}
-
-	if err != ErrNoSuchKey {
-		t.Fatal("expected `ErrNoSuchKey`")
-	}
-}
-
-func TestGet(t *testing.T) {
-	r := NewRegistrar()
-	obj := "testing"
-	name := "test"
-
-	_, err := r.Get(name)
-	if err == nil {
-		t.Fatal("expected error when key does not exist")
-	}
-	if err != ErrNameNotReserved {
-		t.Fatal(err)
-	}
-
-	if err := r.Reserve(name, obj); err != nil {
-		t.Fatal(err)
-	}
-
-	if _, err = r.Get(name); err != nil {
-		t.Fatal(err)
-	}
-
-	r.Delete(obj)
-	_, err = r.Get(name)
-	if err == nil {
-		t.Fatal("expected error when key does not exist")
-	}
-	if err != ErrNameNotReserved {
-		t.Fatal(err)
-	}
-}

From bc3209bc156fc5a5bc6e76e5f79a64c60e9a5f7b Mon Sep 17 00:00:00 2001
From: Aaron Lehmann <aaron.lehmann@docker.com>
Date: Mon, 10 Jul 2017 10:05:14 -0700
Subject: [PATCH 2/3] container: Abort transactions when memdb calls fail

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
---
 container/view.go      | 43 +++++++++++++++++++++---------------------
 container/view_test.go |  4 ++--
 2 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/container/view.go b/container/view.go
index 449cade149..e6055cc932 100644
--- a/container/view.go
+++ b/container/view.go
@@ -66,7 +66,7 @@ type ViewDB interface {
 	Delete(*Container) error
 
 	ReserveName(name, containerID string) error
-	ReleaseName(name string)
+	ReleaseName(name string) error
 }
 
 // View can be used by readers to avoid locking
@@ -150,28 +150,20 @@ func (db *memDB) Save(c *Container) error {
 // Delete removes an item by ID
 func (db *memDB) Delete(c *Container) error {
 	txn := db.store.Txn(true)
-	defer txn.Commit()
 
-	// Delete any names referencing this container's ID.
-	iter, err := txn.Get(memdbNamesTable, memdbContainerIDIndex, c.ID)
-	if err != nil {
-		return err
-	}
-
-	var names []string
-	for {
-		item := iter.Next()
-		if item == nil {
-			break
-		}
-		names = append(names, item.(nameAssociation).name)
-	}
+	view := &memdbView{txn: txn}
+	names := view.getNames(c.ID)
 
 	for _, name := range names {
 		txn.Delete(memdbNamesTable, nameAssociation{name: name})
 	}
 
-	return txn.Delete(memdbContainersTable, NewBaseContainer(c.ID, c.Root))
+	if err := txn.Delete(memdbContainersTable, NewBaseContainer(c.ID, c.Root)); err != nil {
+		txn.Abort()
+		return err
+	}
+	txn.Commit()
+	return nil
 }
 
 // ReserveName registers a container ID to a name
@@ -180,29 +172,38 @@ func (db *memDB) Delete(c *Container) error {
 // A name reservation is globally unique
 func (db *memDB) ReserveName(name, containerID string) error {
 	txn := db.store.Txn(true)
-	defer txn.Commit()
 
 	s, err := txn.First(memdbNamesTable, memdbIDIndex, name)
 	if err != nil {
+		txn.Abort()
 		return err
 	}
 	if s != nil {
+		txn.Abort()
 		if s.(nameAssociation).containerID != containerID {
 			return ErrNameReserved
 		}
 		return nil
 	}
 
-	txn.Insert(memdbNamesTable, nameAssociation{name: name, containerID: containerID})
+	if err := txn.Insert(memdbNamesTable, nameAssociation{name: name, containerID: containerID}); err != nil {
+		txn.Abort()
+		return err
+	}
+	txn.Commit()
 	return nil
 }
 
 // ReleaseName releases the reserved name
 // Once released, a name can be reserved again
-func (db *memDB) ReleaseName(name string) {
+func (db *memDB) ReleaseName(name string) error {
 	txn := db.store.Txn(true)
-	txn.Delete(memdbNamesTable, nameAssociation{name: name})
+	if err := txn.Delete(memdbNamesTable, nameAssociation{name: name}); err != nil {
+		txn.Abort()
+		return err
+	}
 	txn.Commit()
+	return nil
 }
 
 type memdbView struct {
diff --git a/container/view_test.go b/container/view_test.go
index 2e81998ca4..09ba343830 100644
--- a/container/view_test.go
+++ b/container/view_test.go
@@ -114,7 +114,7 @@ func TestNames(t *testing.T) {
 	assert.EqualError(t, db.ReserveName("name2", "containerid3"), ErrNameReserved.Error())
 
 	// Releasing a name allows the name to point to something else later.
-	db.ReleaseName("name2")
+	assert.NoError(t, db.ReleaseName("name2"))
 	assert.NoError(t, db.ReserveName("name2", "containerid3"))
 
 	view := db.Snapshot()
@@ -131,7 +131,7 @@ func TestNames(t *testing.T) {
 	assert.EqualError(t, err, ErrNameNotReserved.Error())
 
 	// Releasing and re-reserving a name doesn't affect the snapshot.
-	db.ReleaseName("name2")
+	assert.NoError(t, db.ReleaseName("name2"))
 	assert.NoError(t, db.ReserveName("name2", "containerid4"))
 
 	id, err = view.GetID("name1")

From 0e57eb95c5989d0f4e93b7d12efe735a6287781b Mon Sep 17 00:00:00 2001
From: Aaron Lehmann <aaron.lehmann@docker.com>
Date: Mon, 10 Jul 2017 13:36:36 -0700
Subject: [PATCH 3/3] container: Use wrapper to ensure commit/abort happens

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
---
 container/view.go | 92 ++++++++++++++++++++++++-----------------------
 1 file changed, 48 insertions(+), 44 deletions(-)

diff --git a/container/view.go b/container/view.go
index e6055cc932..e865e4d5df 100644
--- a/container/view.go
+++ b/container/view.go
@@ -139,26 +139,10 @@ func (db *memDB) Snapshot() View {
 	}
 }
 
-// Save atomically updates the in-memory store state for a Container.
-// Only read only (deep) copies of containers may be passed in.
-func (db *memDB) Save(c *Container) error {
+func (db *memDB) withTxn(cb func(*memdb.Txn) error) error {
 	txn := db.store.Txn(true)
-	defer txn.Commit()
-	return txn.Insert(memdbContainersTable, c)
-}
-
-// Delete removes an item by ID
-func (db *memDB) Delete(c *Container) error {
-	txn := db.store.Txn(true)
-
-	view := &memdbView{txn: txn}
-	names := view.getNames(c.ID)
-
-	for _, name := range names {
-		txn.Delete(memdbNamesTable, nameAssociation{name: name})
-	}
-
-	if err := txn.Delete(memdbContainersTable, NewBaseContainer(c.ID, c.Root)); err != nil {
+	err := cb(txn)
+	if err != nil {
 		txn.Abort()
 		return err
 	}
@@ -166,44 +150,64 @@ func (db *memDB) Delete(c *Container) error {
 	return nil
 }
 
+// Save atomically updates the in-memory store state for a Container.
+// Only read only (deep) copies of containers may be passed in.
+func (db *memDB) Save(c *Container) error {
+	return db.withTxn(func(txn *memdb.Txn) error {
+		return txn.Insert(memdbContainersTable, c)
+	})
+}
+
+// Delete removes an item by ID
+func (db *memDB) Delete(c *Container) error {
+	return db.withTxn(func(txn *memdb.Txn) error {
+		view := &memdbView{txn: txn}
+		names := view.getNames(c.ID)
+
+		for _, name := range names {
+			txn.Delete(memdbNamesTable, nameAssociation{name: name})
+		}
+
+		if err := txn.Delete(memdbContainersTable, NewBaseContainer(c.ID, c.Root)); err != nil {
+			return err
+		}
+		return nil
+	})
+}
+
 // ReserveName registers a container ID to a name
 // ReserveName is idempotent
 // Attempting to reserve a container ID to a name that already exists results in an `ErrNameReserved`
 // A name reservation is globally unique
 func (db *memDB) ReserveName(name, containerID string) error {
-	txn := db.store.Txn(true)
+	return db.withTxn(func(txn *memdb.Txn) error {
+		s, err := txn.First(memdbNamesTable, memdbIDIndex, name)
+		if err != nil {
+			return err
+		}
+		if s != nil {
+			if s.(nameAssociation).containerID != containerID {
+				return ErrNameReserved
+			}
+			return nil
+		}
 
-	s, err := txn.First(memdbNamesTable, memdbIDIndex, name)
-	if err != nil {
-		txn.Abort()
-		return err
-	}
-	if s != nil {
-		txn.Abort()
-		if s.(nameAssociation).containerID != containerID {
-			return ErrNameReserved
+		if err := txn.Insert(memdbNamesTable, nameAssociation{name: name, containerID: containerID}); err != nil {
+			return err
 		}
 		return nil
-	}
-
-	if err := txn.Insert(memdbNamesTable, nameAssociation{name: name, containerID: containerID}); err != nil {
-		txn.Abort()
-		return err
-	}
-	txn.Commit()
-	return nil
+	})
 }
 
 // ReleaseName releases the reserved name
 // Once released, a name can be reserved again
 func (db *memDB) ReleaseName(name string) error {
-	txn := db.store.Txn(true)
-	if err := txn.Delete(memdbNamesTable, nameAssociation{name: name}); err != nil {
-		txn.Abort()
-		return err
-	}
-	txn.Commit()
-	return nil
+	return db.withTxn(func(txn *memdb.Txn) error {
+		if err := txn.Delete(memdbNamesTable, nameAssociation{name: name}); err != nil {
+			return err
+		}
+		return nil
+	})
 }
 
 type memdbView struct {