Browse code

Don’t hold container lock for size calculation

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

Tonis Tiigi authored on 2017/02/19 11:11:48
Showing 6 changed files
... ...
@@ -12,7 +12,7 @@ func (daemon *Daemon) setupLinkedContainers(container *container.Container) ([]s
12 12
 }
13 13
 
14 14
 // getSize returns real size & virtual size
15
-func (daemon *Daemon) getSize(container *container.Container) (int64, int64) {
15
+func (daemon *Daemon) getSize(containerID string) (int64, int64) {
16 16
 	// TODO Windows
17 17
 	return 0, 0
18 18
 }
... ...
@@ -4,32 +4,32 @@ package daemon
4 4
 
5 5
 import (
6 6
 	"github.com/Sirupsen/logrus"
7
-	"github.com/docker/docker/container"
8 7
 )
9 8
 
10 9
 // getSize returns the real size & virtual size of the container.
11
-func (daemon *Daemon) getSize(container *container.Container) (int64, int64) {
10
+func (daemon *Daemon) getSize(containerID string) (int64, int64) {
12 11
 	var (
13 12
 		sizeRw, sizeRootfs int64
14 13
 		err                error
15 14
 	)
16 15
 
17
-	if err := daemon.Mount(container); err != nil {
18
-		logrus.Errorf("Failed to compute size of container rootfs %s: %s", container.ID, err)
16
+	rwlayer, err := daemon.layerStore.GetRWLayer(containerID)
17
+	if err != nil {
18
+		logrus.Errorf("Failed to compute size of container rootfs %v: %v", containerID, err)
19 19
 		return sizeRw, sizeRootfs
20 20
 	}
21
-	defer daemon.Unmount(container)
21
+	defer daemon.layerStore.ReleaseRWLayer(rwlayer)
22 22
 
23
-	sizeRw, err = container.RWLayer.Size()
23
+	sizeRw, err = rwlayer.Size()
24 24
 	if err != nil {
25 25
 		logrus.Errorf("Driver %s couldn't return diff size of container %s: %s",
26
-			daemon.GraphDriverName(), container.ID, err)
26
+			daemon.GraphDriverName(), containerID, err)
27 27
 		// FIXME: GetSize should return an error. Not changing it now in case
28 28
 		// there is a side-effect.
29 29
 		sizeRw = -1
30 30
 	}
31 31
 
32
-	if parent := container.RWLayer.Parent(); parent != nil {
32
+	if parent := rwlayer.Parent(); parent != nil {
33 33
 		sizeRootfs, err = parent.Size()
34 34
 		if err != nil {
35 35
 			sizeRootfs = -1
... ...
@@ -36,10 +36,10 @@ func (daemon *Daemon) ContainerInspectCurrent(name string, size bool) (*types.Co
36 36
 	}
37 37
 
38 38
 	container.Lock()
39
-	defer container.Unlock()
40 39
 
41
-	base, err := daemon.getInspectData(container, size)
40
+	base, err := daemon.getInspectData(container)
42 41
 	if err != nil {
42
+		container.Unlock()
43 43
 		return nil, err
44 44
 	}
45 45
 
... ...
@@ -73,6 +73,14 @@ func (daemon *Daemon) ContainerInspectCurrent(name string, size bool) (*types.Co
73 73
 	}
74 74
 	networkSettings.NetworkSettingsBase.Ports = ports
75 75
 
76
+	container.Unlock()
77
+
78
+	if size {
79
+		sizeRw, sizeRootFs := daemon.getSize(base.ID)
80
+		base.SizeRw = &sizeRw
81
+		base.SizeRootFs = &sizeRootFs
82
+	}
83
+
76 84
 	return &types.ContainerJSON{
77 85
 		ContainerJSONBase: base,
78 86
 		Mounts:            mountPoints,
... ...
@@ -91,7 +99,7 @@ func (daemon *Daemon) containerInspect120(name string) (*v1p20.ContainerJSON, er
91 91
 	container.Lock()
92 92
 	defer container.Unlock()
93 93
 
94
-	base, err := daemon.getInspectData(container, false)
94
+	base, err := daemon.getInspectData(container)
95 95
 	if err != nil {
96 96
 		return nil, err
97 97
 	}
... ...
@@ -114,7 +122,7 @@ func (daemon *Daemon) containerInspect120(name string) (*v1p20.ContainerJSON, er
114 114
 	}, nil
115 115
 }
116 116
 
117
-func (daemon *Daemon) getInspectData(container *container.Container, size bool) (*types.ContainerJSONBase, error) {
117
+func (daemon *Daemon) getInspectData(container *container.Container) (*types.ContainerJSONBase, error) {
118 118
 	// make a copy to play with
119 119
 	hostConfig := *container.HostConfig
120 120
 
... ...
@@ -168,16 +176,6 @@ func (daemon *Daemon) getInspectData(container *container.Container, size bool)
168 168
 		HostConfig:   &hostConfig,
169 169
 	}
170 170
 
171
-	var (
172
-		sizeRw     int64
173
-		sizeRootFs int64
174
-	)
175
-	if size {
176
-		sizeRw, sizeRootFs = daemon.getSize(container)
177
-		contJSONBase.SizeRw = &sizeRw
178
-		contJSONBase.SizeRootFs = &sizeRootFs
179
-	}
180
-
181 171
 	// Now set any platform-specific fields
182 172
 	contJSONBase = setPlatformSpecificContainerFields(container, contJSONBase)
183 173
 
... ...
@@ -30,7 +30,7 @@ func (daemon *Daemon) containerInspectPre120(name string) (*v1p19.ContainerJSON,
30 30
 	container.Lock()
31 31
 	defer container.Unlock()
32 32
 
33
-	base, err := daemon.getInspectData(container, false)
33
+	base, err := daemon.getInspectData(container)
34 34
 	if err != nil {
35 35
 		return nil, err
36 36
 	}
... ...
@@ -208,19 +208,32 @@ func (daemon *Daemon) reduceContainers(config *types.ContainerListOptions, reduc
208 208
 // reducePsContainer is the basic representation for a container as expected by the ps command.
209 209
 func (daemon *Daemon) reducePsContainer(container *container.Container, ctx *listContext, reducer containerReducer) (*types.Container, error) {
210 210
 	container.Lock()
211
-	defer container.Unlock()
212 211
 
213 212
 	// filter containers to return
214 213
 	action := includeContainerInList(container, ctx)
215 214
 	switch action {
216 215
 	case excludeContainer:
216
+		container.Unlock()
217 217
 		return nil, nil
218 218
 	case stopIteration:
219
+		container.Unlock()
219 220
 		return nil, errStopIteration
220 221
 	}
221 222
 
222 223
 	// transform internal container struct into api structs
223
-	return reducer(container, ctx)
224
+	newC, err := reducer(container, ctx)
225
+	container.Unlock()
226
+	if err != nil {
227
+		return nil, err
228
+	}
229
+
230
+	// release lock because size calculation is slow
231
+	if ctx.Size {
232
+		sizeRw, sizeRootFs := daemon.getSize(newC.ID)
233
+		newC.SizeRw = sizeRw
234
+		newC.SizeRootFs = sizeRootFs
235
+	}
236
+	return newC, nil
224 237
 }
225 238
 
226 239
 // foldFilter generates the container filter based on the user's filtering options.
... ...
@@ -642,11 +655,6 @@ func (daemon *Daemon) transformContainer(container *container.Container, ctx *li
642 642
 		}
643 643
 	}
644 644
 
645
-	if ctx.Size {
646
-		sizeRw, sizeRootFs := daemon.getSize(container)
647
-		newC.SizeRw = sizeRw
648
-		newC.SizeRootFs = sizeRootFs
649
-	}
650 645
 	newC.Labels = container.Config.Labels
651 646
 	newC.Mounts = addMountPoints(container)
652 647
 
... ...
@@ -16,7 +16,7 @@ import (
16 16
 	"github.com/docker/docker/runconfig"
17 17
 	"github.com/docker/docker/volume"
18 18
 	"github.com/docker/libnetwork"
19
-	"github.com/opencontainers/go-digest"
19
+	digest "github.com/opencontainers/go-digest"
20 20
 )
21 21
 
22 22
 // ContainersPrune removes unused containers
... ...
@@ -34,7 +34,7 @@ func (daemon *Daemon) ContainersPrune(pruneFilters filters.Args) (*types.Contain
34 34
 			if !until.IsZero() && c.Created.After(until) {
35 35
 				continue
36 36
 			}
37
-			cSize, _ := daemon.getSize(c)
37
+			cSize, _ := daemon.getSize(c.ID)
38 38
 			// TODO: sets RmLink to true?
39 39
 			err := daemon.ContainerRm(c.ID, &types.ContainerRmConfig{})
40 40
 			if err != nil {