From 511a909ae6687b22f4bf0c6f08fa8392852e61b0 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 21 Sep 2022 12:20:13 +0200 Subject: [PATCH] container: remove ViewDB and View interfaces, use concrete types These interfaces were added in aacddda89df05b88a6d15fb33c42864760385ab2, with no clear motivation, other than "Also hide ViewDB behind an interface". This patch removes the interface in favor of using a concrete implementation; There's currently only one implementation of this interface, and if we would decide to change to an alternative implementation, we could define relevant interfaces on the receiver side. Signed-off-by: Sebastiaan van Stijn --- container/container.go | 2 +- container/view.go | 65 ++++++++++++++++-------------------------- container/view_test.go | 2 +- daemon/daemon.go | 2 +- daemon/list.go | 6 ++-- 5 files changed, 30 insertions(+), 47 deletions(-) diff --git a/container/container.go b/container/container.go index 09fab63511..ead7960f4f 100644 --- a/container/container.go +++ b/container/container.go @@ -193,7 +193,7 @@ func (container *Container) toDisk() (*Container, error) { // CheckpointTo makes the Container's current state visible to queries, and persists state. // Callers must hold a Container lock. -func (container *Container) CheckpointTo(store ViewDB) error { +func (container *Container) CheckpointTo(store *ViewDB) error { deepCopy, err := container.toDisk() if err != nil { return err diff --git a/container/view.go b/container/view.go index e061cf4007..3c48644946 100644 --- a/container/view.go +++ b/container/view.go @@ -77,27 +77,6 @@ type nameAssociation struct { containerID string } -// ViewDB provides an in-memory transactional (ACID) container Store -type ViewDB interface { - Snapshot() View - Save(*Container) error - Delete(*Container) error - - GetByPrefix(string) (string, error) - - ReserveName(name, containerID string) error - ReleaseName(name string) error -} - -// 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{ memdbContainersTable: { @@ -128,7 +107,8 @@ var schema = &memdb.DBSchema{ }, } -type memDB struct { +// ViewDB provides an in-memory transactional (ACID) container store. +type ViewDB struct { store *memdb.MemDB } @@ -144,15 +124,17 @@ func (e NoSuchContainerError) Error() string { } // NewViewDB provides the default implementation, with the default schema -func NewViewDB() (ViewDB, error) { +func NewViewDB() (*ViewDB, error) { store, err := memdb.NewMemDB(schema) if err != nil { return nil, err } - return &memDB{store: store}, nil + return &ViewDB{store: store}, nil } -func (db *memDB) GetByPrefix(s string) (string, error) { +// GetByPrefix returns a container with the given ID prefix. It returns an +// error if an empty prefix was given or if multiple containers match the prefix. +func (db *ViewDB) GetByPrefix(s string) (string, error) { if s == "" { return "", ErrEmptyPrefix } @@ -184,14 +166,14 @@ func (db *memDB) GetByPrefix(s string) (string, error) { return "", ErrNotExist } -// Snapshot provides a consistent read-only View of the database -func (db *memDB) Snapshot() View { - return &memdbView{ +// Snapshot provides a consistent read-only view of the database. +func (db *ViewDB) Snapshot() *View { + return &View{ txn: db.store.Txn(false), } } -func (db *memDB) withTxn(cb func(*memdb.Txn) error) error { +func (db *ViewDB) withTxn(cb func(*memdb.Txn) error) error { txn := db.store.Txn(true) err := cb(txn) if err != nil { @@ -204,16 +186,16 @@ func (db *memDB) withTxn(cb func(*memdb.Txn) error) error { // 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 *ViewDB) 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 { +func (db *ViewDB) Delete(c *Container) error { return db.withTxn(func(txn *memdb.Txn) error { - view := &memdbView{txn: txn} + view := &View{txn: txn} names := view.getNames(c.ID) for _, name := range names { @@ -231,7 +213,7 @@ func (db *memDB) Delete(c *Container) error { // 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 { +func (db *ViewDB) ReserveName(name, containerID string) error { return db.withTxn(func(txn *memdb.Txn) error { s, err := txn.First(memdbNamesTable, memdbIDIndex, name) if err != nil { @@ -249,18 +231,19 @@ func (db *memDB) ReserveName(name, containerID string) error { // ReleaseName releases the reserved name // Once released, a name can be reserved again -func (db *memDB) ReleaseName(name string) error { +func (db *ViewDB) ReleaseName(name string) error { return db.withTxn(func(txn *memdb.Txn) error { return txn.Delete(memdbNamesTable, nameAssociation{name: name}) }) } -type memdbView struct { +// View provides a consistent read-only view of the database. +type View struct { txn *memdb.Txn } // All returns a all items in this snapshot. Returned objects must never be modified. -func (v *memdbView) All() ([]Snapshot, error) { +func (v *View) All() ([]Snapshot, error) { var all []Snapshot iter, err := v.txn.Get(memdbContainersTable, memdbIDIndex) if err != nil { @@ -278,7 +261,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) { +func (v *View) Get(id string) (*Snapshot, error) { s, err := v.txn.First(memdbContainersTable, memdbIDIndex, id) if err != nil { return nil, err @@ -290,7 +273,7 @@ func (v *memdbView) Get(id string) (*Snapshot, error) { } // getNames lists all the reserved names for the given container ID. -func (v *memdbView) getNames(containerID string) []string { +func (v *View) getNames(containerID string) []string { iter, err := v.txn.Get(memdbNamesTable, memdbContainerIDIndex, containerID) if err != nil { return nil @@ -309,7 +292,7 @@ func (v *memdbView) getNames(containerID string) []string { } // GetID returns the container ID that the passed in name is reserved to. -func (v *memdbView) GetID(name string) (string, error) { +func (v *View) GetID(name string) (string, error) { s, err := v.txn.First(memdbNamesTable, memdbIDIndex, name) if err != nil { return "", err @@ -321,7 +304,7 @@ func (v *memdbView) GetID(name string) (string, error) { } // GetAllNames returns all registered names. -func (v *memdbView) GetAllNames() map[string][]string { +func (v *View) GetAllNames() map[string][]string { iter, err := v.txn.Get(memdbNamesTable, memdbContainerIDIndex) if err != nil { return nil @@ -342,7 +325,7 @@ func (v *memdbView) GetAllNames() map[string][]string { // 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 { +func (v *View) transform(container *Container) *Snapshot { health := types.NoHealthcheck if container.Health != nil { health = container.Health.Status() diff --git a/container/view_test.go b/container/view_test.go index 099da49df9..903d8d1e8c 100644 --- a/container/view_test.go +++ b/container/view_test.go @@ -373,7 +373,7 @@ func TestTruncIndex(t *testing.T) { } } -func assertIndexGet(t *testing.T, snapshot ViewDB, input, expectedResult string, expectError bool) { +func assertIndexGet(t *testing.T, snapshot *ViewDB, input, expectedResult string, expectError bool) { if result, err := snapshot.GetByPrefix(input); err != nil && !expectError { t.Fatalf("Unexpected error getting '%s': %s", input, err) } else if err == nil && expectError { diff --git a/daemon/daemon.go b/daemon/daemon.go index 10230b1580..bb38689fc1 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -73,7 +73,7 @@ type Daemon struct { id string repository string containers container.Store - containersReplica container.ViewDB + containersReplica *container.ViewDB execCommands *container.ExecStore imageService ImageService configStore *config.Config diff --git a/daemon/list.go b/daemon/list.go index 7c796939a7..5d4dc80079 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -110,7 +110,7 @@ func (daemon *Daemon) Containers(config *types.ContainerListOptions) ([]*types.C return daemon.reduceContainers(config, daemon.refreshImage) } -func (daemon *Daemon) filterByNameIDMatches(view container.View, filter *listContext) ([]container.Snapshot, error) { +func (daemon *Daemon) filterByNameIDMatches(view *container.View, filter *listContext) ([]container.Snapshot, error) { idSearch := false names := filter.filters.Get("name") ids := filter.filters.Get("id") @@ -243,7 +243,7 @@ func (daemon *Daemon) reducePsContainer(container *container.Snapshot, filter *l } // foldFilter generates the container filter based on the user's filtering options. -func (daemon *Daemon) foldFilter(view container.View, config *types.ContainerListOptions) (*listContext, error) { +func (daemon *Daemon) foldFilter(view *container.View, config *types.ContainerListOptions) (*listContext, error) { ctx := context.TODO() psFilters := config.Filters @@ -363,7 +363,7 @@ func (daemon *Daemon) foldFilter(view container.View, config *types.ContainerLis }, nil } -func idOrNameFilter(view container.View, value string) (*container.Snapshot, error) { +func idOrNameFilter(view *container.View, value string) (*container.Snapshot, error) { filter, err := view.Get(value) switch err.(type) { case container.NoSuchContainerError: