From 9755d1918d87096ec7d5b71be7da9742d8b41b98 Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Thu, 6 Apr 2017 10:43:10 -0700 Subject: [PATCH] avoid re-reading json files when copying containers Signed-off-by: Fabio Kung Upstream-commit: a43be3431e51b914ab170569b9e58239d64b199c Component: engine --- components/engine/container/container.go | 72 +++++++++++++------ components/engine/container/view.go | 10 +-- .../engine/daemon/container_operations.go | 2 +- components/engine/daemon/daemon_unix.go | 3 +- 4 files changed, 55 insertions(+), 32 deletions(-) diff --git a/components/engine/container/container.go b/components/engine/container/container.go index b0ff5b7c9b..fe050bcd17 100644 --- a/components/engine/container/container.go +++ b/components/engine/container/container.go @@ -1,6 +1,7 @@ package container import ( + "bytes" "encoding/json" "fmt" "io" @@ -14,8 +15,6 @@ import ( "syscall" "time" - "golang.org/x/net/context" - "github.com/Sirupsen/logrus" containertypes "github.com/docker/docker/api/types/container" mounttypes "github.com/docker/docker/api/types/mount" @@ -45,6 +44,7 @@ import ( "github.com/docker/libnetwork/options" "github.com/docker/libnetwork/types" agentexec "github.com/docker/swarmkit/agent/exec" + "golang.org/x/net/context" ) const configFileName = "config.v2.json" @@ -154,36 +154,48 @@ func (container *Container) FromDisk() error { return container.readHostConfig() } -// ToDisk saves the container configuration on disk. -func (container *Container) toDisk() error { +// toDisk saves the container configuration on disk and returns a deep copy. +func (container *Container) toDisk() (*Container, error) { + var ( + buf bytes.Buffer + deepCopy Container + ) pth, err := container.ConfigPath() if err != nil { - return err + return nil, err } - jsonSource, err := ioutils.NewAtomicFileWriter(pth, 0644) - if err != nil { - return err - } - defer jsonSource.Close() - - enc := json.NewEncoder(jsonSource) - // Save container settings - if err := enc.Encode(container); err != nil { - return err + f, err := ioutils.NewAtomicFileWriter(pth, 0644) + if err != nil { + return nil, err + } + defer f.Close() + + w := io.MultiWriter(&buf, f) + if err := json.NewEncoder(w).Encode(container); err != nil { + return nil, err } - return container.WriteHostConfig() + if err := json.NewDecoder(&buf).Decode(&deepCopy); err != nil { + return nil, err + } + deepCopy.HostConfig, err = container.WriteHostConfig() + if err != nil { + return nil, err + } + + return &deepCopy, nil } // 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 { - if err := container.toDisk(); err != nil { + deepCopy, err := container.toDisk() + if err != nil { return err } - return store.Save(container) + return store.Save(deepCopy) } // readHostConfig reads the host configuration from disk for the container. @@ -215,20 +227,34 @@ func (container *Container) readHostConfig() error { return nil } -// WriteHostConfig saves the host configuration on disk for the container. -func (container *Container) WriteHostConfig() error { +// WriteHostConfig saves the host configuration on disk for the container, +// and returns a deep copy of the saved object. Callers must hold a Container lock. +func (container *Container) WriteHostConfig() (*containertypes.HostConfig, error) { + var ( + buf bytes.Buffer + deepCopy containertypes.HostConfig + ) + pth, err := container.HostConfigPath() if err != nil { - return err + return nil, err } f, err := ioutils.NewAtomicFileWriter(pth, 0644) if err != nil { - return err + return nil, err } defer f.Close() - return json.NewEncoder(f).Encode(&container.HostConfig) + w := io.MultiWriter(&buf, f) + if err := json.NewEncoder(w).Encode(&container.HostConfig); err != nil { + return nil, err + } + + if err := json.NewDecoder(&buf).Decode(&deepCopy); err != nil { + return nil, err + } + return &deepCopy, nil } // SetupWorkingDirectory sets up the container's working directory as set in container.Config.WorkingDir diff --git a/components/engine/container/view.go b/components/engine/container/view.go index 5c501fd9b4..13c7161852 100644 --- a/components/engine/container/view.go +++ b/components/engine/container/view.go @@ -90,16 +90,12 @@ func (db *memDB) Snapshot(index *registrar.Registrar) View { } } -// Save atomically updates the in-memory store from the current on-disk state of a Container. +// 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 { txn := db.store.Txn(true) defer txn.Commit() - deepCopy := NewBaseContainer(c.ID, c.Root) - err := deepCopy.FromDisk() - if err != nil { - return err - } - return txn.Insert(memdbTable, deepCopy) + return txn.Insert(memdbTable, c) } // Delete removes an item by ID diff --git a/components/engine/daemon/container_operations.go b/components/engine/daemon/container_operations.go index 8a066bc899..e4b67893d0 100644 --- a/components/engine/daemon/container_operations.go +++ b/components/engine/daemon/container_operations.go @@ -579,7 +579,7 @@ func (daemon *Daemon) allocateNetwork(container *container.Container) error { } - if err := container.WriteHostConfig(); err != nil { + if _, err := container.WriteHostConfig(); err != nil { return err } networkActions.WithValues("allocate").UpdateSince(start) diff --git a/components/engine/daemon/daemon_unix.go b/components/engine/daemon/daemon_unix.go index 662098ed19..0778dde4f7 100644 --- a/components/engine/daemon/daemon_unix.go +++ b/components/engine/daemon/daemon_unix.go @@ -1146,7 +1146,8 @@ func (daemon *Daemon) registerLinks(container *container.Container, hostConfig * // After we load all the links into the daemon // set them to nil on the hostconfig - return container.WriteHostConfig() + _, err := container.WriteHostConfig() + return err } // conditionalMountOnStart is a platform specific helper function during the