1. docker
  2. Commit 9e6b1852a78eda6ed2cb255d6be8a0d0e5a5ca40
Browse code

Fix N+1 calling `Path()` on `volume ls`

Implements a `CachedPath` function on the volume plugin adapter that we
call from the volume list function instead of `Path.
If a driver does not implement `CachedPath` it will just call `Path`.

Also makes sure we store the path on Mount and remove the path on
Unmount.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Brian Goff authored on 2016/04/13 06:09:55
Showing 6 changed files
daemon/create.go
History View file @ 9e6b185
... ... 
@@ -183,5 +183,7 @@ func (daemon *Daemon) VolumeCreate(name, driverName string, opts, labels map[str
183 183 
 	}
184 184
185 185 
 	daemon.LogVolumeEvent(v.Name(), "create", map[string]string{"driver": v.DriverName()})
186 
-	return volumeToAPIType(v), nil
186 
+	apiV := volumeToAPIType(v)
187 
+	apiV.Mountpoint = v.Path()
188 
+	return apiV, nil
187 189 
 }
daemon/inspect.go
History View file @ 9e6b185
... ... 
@@ -204,7 +204,9 @@ func (daemon *Daemon) VolumeInspect(name string) (*types.Volume, error) {
204 204 
 	if err != nil {
205 205 
 		return nil, err
206 206 
 	}
207 
-	return volumeToAPIType(v), nil
207 
+	apiV := volumeToAPIType(v)
208 
+	apiV.Mountpoint = v.Path()
209 
+	return apiV, nil
208 210 
 }
209 211
210 212 
 func (daemon *Daemon) getBackwardsCompatibleNetworkSettings(settings *network.Settings) *v1p20.NetworkSettings {
daemon/list.go
History View file @ 9e6b185
... ... 
@@ -491,7 +491,15 @@ func (daemon *Daemon) Volumes(filter string) ([]*types.Volume, []string, error)
491 491 
 		return nil, nil, err
492 492 
 	}
493 493 
 	for _, v := range filterVolumes {
494 
-		volumesOut = append(volumesOut, volumeToAPIType(v))
494 
+		apiV := volumeToAPIType(v)
495 
+		if vv, ok := v.(interface {
496 
+			CachedPath() string
497 
+		}); ok {
498 
+			apiV.Mountpoint = vv.CachedPath()
499 
+		} else {
500 
+			apiV.Mountpoint = v.Path()
501 
+		}
502 
+		volumesOut = append(volumesOut, apiV)
495 503 
 	}
496 504 
 	return volumesOut, warnings, nil
497 505 
 }
daemon/volumes.go
History View file @ 9e6b185
... ... 
@@ -25,9 +25,8 @@ type mounts []container.Mount
25 25 
 // volumeToAPIType converts a volume.Volume to the type used by the remote API
26 26 
 func volumeToAPIType(v volume.Volume) *types.Volume {
27 27 
 	tv := &types.Volume{
28 
-		Name:       v.Name(),
29 
-		Driver:     v.DriverName(),
30 
-		Mountpoint: v.Path(),
28 
+		Name:   v.Name(),
29 
+		Driver: v.DriverName(),
31 30 
 	}
32 31 
 	if v, ok := v.(interface {
33 32 
 		Labels() map[string]string
integration-cli/docker_cli_start_volume_driver_unix_test.go
History View file @ 9e6b185
... ... 
@@ -441,3 +441,22 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverGetEmptyResponse(c *
441 441 
 	c.Assert(err, checker.NotNil, check.Commentf(out))
442 442 
 	c.Assert(out, checker.Contains, "No such volume")
443 443 
 }
444 
+
445 
+// Ensure only cached paths are used in volume list to prevent N+1 calls to `VolumeDriver.Path`
446 
+func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverPathCalls(c *check.C) {
447 
+	c.Assert(s.d.Start(), checker.IsNil)
448 
+	c.Assert(s.ec.paths, checker.Equals, 0)
449 
+
450 
+	out, err := s.d.Cmd("volume", "create", "--name=test", "--driver=test-external-volume-driver")
451 
+	c.Assert(err, checker.IsNil, check.Commentf(out))
452 
+	c.Assert(s.ec.paths, checker.Equals, 1)
453 
+
454 
+	out, err = s.d.Cmd("volume", "ls")
455 
+	c.Assert(err, checker.IsNil, check.Commentf(out))
456 
+	c.Assert(s.ec.paths, checker.Equals, 1)
457 
+
458 
+	out, err = s.d.Cmd("volume", "inspect", "--format='{{.Mountpoint}}'", "test")
459 
+	c.Assert(err, checker.IsNil, check.Commentf(out))
460 
+	c.Assert(strings.TrimSpace(out), checker.Not(checker.Equals), "")
461 
+	c.Assert(s.ec.paths, checker.Equals, 1)
462 
+}
volume/drivers/adapter.go
History View file @ 9e6b185
... ... 
@@ -88,11 +88,14 @@ func (a *volumeAdapter) DriverName() string {
88 88 
 }
89 89
90 90 
 func (a *volumeAdapter) Path() string {
91 
-	if len(a.eMount) > 0 {
92 
-		return a.eMount
91 
+	if len(a.eMount) == 0 {
92 
+		a.eMount, _ = a.proxy.Path(a.name)
93 93 
 	}
94 
-	m, _ := a.proxy.Path(a.name)
95 
-	return m
94 
+	return a.eMount
95 
+}
96 
+
97 
+func (a *volumeAdapter) CachedPath() string {
98 
+	return a.eMount
96 99 
 }
97 100
98 101 
 func (a *volumeAdapter) Mount() (string, error) {
... ... 
@@ -102,5 +105,9 @@ func (a *volumeAdapter) Mount() (string, error) {
102 102 
 }
103 103
104 104 
 func (a *volumeAdapter) Unmount() error {
105 
-	return a.proxy.Unmount(a.name)
105 
+	err := a.proxy.Unmount(a.name)
106 
+	if err == nil {
107 
+		a.eMount = ""
108 
+	}
109 
+	return err
106 110 
 }