From 7568d41513753456577a892111dda2925c072b41 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 20 Jan 2016 12:06:03 -0500 Subject: [PATCH 1/2] Fix loading of containerized plugins During daemon startup, all containers are registered before any are started. During container registration it was calling out to initialize volumes. If the volume uses a plugin that is running in a container, this will cause the restart of that container to fail since the plugin is not yet running. This also slowed down daemon startup since volume initialization was happening sequentially, which can be slow (and is flat out slow since initialization would fail but take 8 seconds for each volume to do it). This fix holds off on volume initialization until after containers are restarted and does the initialization in parallel. The containers that are restarted will have thier volumes initialized because they are being started. If any of these containers are using a plugin they will just keep retrying to reach the plugin (up to the timeout, which is 8seconds) until the container with the plugin is up and running. Signed-off-by: Brian Goff Upstream-commit: d85b9f858050dbfb2dde3d68517952958b8e38ee Component: engine --- components/engine/daemon/daemon.go | 21 +++++++++++++++++---- components/engine/daemon/volumes.go | 1 - 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/components/engine/daemon/daemon.go b/components/engine/daemon/daemon.go index 9e0e77e350..99d23908b6 100644 --- a/components/engine/daemon/daemon.go +++ b/components/engine/daemon/daemon.go @@ -282,10 +282,6 @@ func (daemon *Daemon) Register(container *container.Container) error { } } - if err := daemon.prepareMountPoints(container); err != nil { - return err - } - return nil } @@ -408,6 +404,23 @@ func (daemon *Daemon) restore() error { } group.Wait() + // any containers that were started above would already have had this done, + // however we need to now prepare the mountpoints for the rest of the containers as well. + // This shouldn't cause any issue running on the containers that already had this run. + // This must be run after any containers with a restart policy so that containerized plugins + // can have a chance to be running before we try to initialize them. + for _, c := range containers { + group.Add(1) + go func(c *container.Container) { + defer group.Done() + if err := daemon.prepareMountPoints(c); err != nil { + logrus.Error(err) + } + }(c) + } + + group.Wait() + if !debug { if logrus.GetLevel() == logrus.InfoLevel { fmt.Println() diff --git a/components/engine/daemon/volumes.go b/components/engine/daemon/volumes.go index c0097679f6..d807fe84bb 100644 --- a/components/engine/daemon/volumes.go +++ b/components/engine/daemon/volumes.go @@ -159,7 +159,6 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo func (daemon *Daemon) lazyInitializeVolume(containerID string, m *volume.MountPoint) error { if len(m.Driver) > 0 && m.Volume == nil { v, err := daemon.volumes.GetWithRef(m.Name, m.Driver, containerID) - if err != nil { return err } From edde0f73ca4ba235b7b5879dcf867c940aad88e3 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 20 Jan 2016 12:12:51 -0500 Subject: [PATCH 2/2] Use fine-grained locks for plugin loading. This helps ensure that only one thing is trying to intialize a plugin at once while also keeping the global lock free during initialization. Signed-off-by: Brian Goff Upstream-commit: cfb2c667ad0bb17d9a1bd49a294d5c38e4cbf040 Component: engine --- components/engine/volume/drivers/extpoint.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/components/engine/volume/drivers/extpoint.go b/components/engine/volume/drivers/extpoint.go index 6f894e0ae6..dd45d1365a 100644 --- a/components/engine/volume/drivers/extpoint.go +++ b/components/engine/volume/drivers/extpoint.go @@ -6,6 +6,7 @@ import ( "fmt" "sync" + "github.com/docker/docker/pkg/locker" "github.com/docker/docker/pkg/plugins" "github.com/docker/docker/volume" ) @@ -13,7 +14,7 @@ import ( // currently created by hand. generation tool would generate this like: // $ extpoint-gen Driver > volume/extpoint.go -var drivers = &driverExtpoint{extensions: make(map[string]volume.Driver)} +var drivers = &driverExtpoint{extensions: make(map[string]volume.Driver), driverLock: &locker.Locker{}} const extName = "VolumeDriver" @@ -49,16 +50,19 @@ type volumeDriver interface { type driverExtpoint struct { extensions map[string]volume.Driver sync.Mutex + driverLock *locker.Locker } // Register associates the given driver to the given name, checking if // the name is already associated func Register(extension volume.Driver, name string) bool { - drivers.Lock() - defer drivers.Unlock() if name == "" { return false } + + drivers.Lock() + defer drivers.Unlock() + _, exists := drivers.extensions[name] if exists { return false @@ -71,6 +75,7 @@ func Register(extension volume.Driver, name string) bool { func Unregister(name string) bool { drivers.Lock() defer drivers.Unlock() + _, exists := drivers.extensions[name] if !exists { return false @@ -83,12 +88,16 @@ func Unregister(name string) bool { // driver with the given name has not been registered it checks if // there is a VolumeDriver plugin available with the given name. func Lookup(name string) (volume.Driver, error) { + drivers.driverLock.Lock(name) + defer drivers.driverLock.Unlock(name) + drivers.Lock() ext, ok := drivers.extensions[name] drivers.Unlock() if ok { return ext, nil } + pl, err := plugins.Get(name, extName) if err != nil { return nil, fmt.Errorf("Error looking up volume plugin %s: %v", name, err) @@ -118,9 +127,11 @@ func GetDriver(name string) (volume.Driver, error) { // If no driver is registered, empty string list will be returned. func GetDriverList() []string { var driverList []string + drivers.Lock() for driverName := range drivers.extensions { driverList = append(driverList, driverName) } + drivers.Unlock() return driverList } @@ -144,6 +155,7 @@ func GetAllDrivers() ([]volume.Driver, error) { if ok { continue } + ext = NewVolumeDriver(p.Name, p.Client) drivers.extensions[p.Name] = ext ds = append(ds, ext)