Browse code

Merge pull request #26819 from ripcurld00d/fix_stats_mutex

Hide the mutex lock in formatter.ContainerStats

Sebastiaan van Stijn authored on 2016/10/14 09:38:22
Showing 3 changed files
... ...
@@ -163,11 +163,10 @@ func runStats(dockerCli *command.DockerCli, opts *statsOptions) error {
163 163
 		var errs []string
164 164
 		cStats.mu.Lock()
165 165
 		for _, c := range cStats.cs {
166
-			c.Mu.Lock()
167
-			if c.Err != nil {
168
-				errs = append(errs, fmt.Sprintf("%s: %v", c.Name, c.Err))
166
+			cErr := c.GetError()
167
+			if cErr != nil {
168
+				errs = append(errs, fmt.Sprintf("%s: %v", c.Name, cErr))
169 169
 			}
170
-			c.Mu.Unlock()
171 170
 		}
172 171
 		cStats.mu.Unlock()
173 172
 		if len(errs) > 0 {
... ...
@@ -186,7 +185,7 @@ func runStats(dockerCli *command.DockerCli, opts *statsOptions) error {
186 186
 		Format: formatter.NewStatsFormat(f, daemonOSType),
187 187
 	}
188 188
 
189
-	cleanHeader := func() {
189
+	cleanScreen := func() {
190 190
 		if !opts.noStream {
191 191
 			fmt.Fprint(dockerCli.Out(), "\033[2J")
192 192
 			fmt.Fprint(dockerCli.Out(), "\033[H")
... ...
@@ -195,14 +194,17 @@ func runStats(dockerCli *command.DockerCli, opts *statsOptions) error {
195 195
 
196 196
 	var err error
197 197
 	for range time.Tick(500 * time.Millisecond) {
198
-		cleanHeader()
199
-		cStats.mu.RLock()
200
-		csLen := len(cStats.cs)
201
-		if err = formatter.ContainerStatsWrite(statsCtx, cStats.cs); err != nil {
198
+		cleanScreen()
199
+		ccstats := []formatter.StatsEntry{}
200
+		cStats.mu.Lock()
201
+		for _, c := range cStats.cs {
202
+			ccstats = append(ccstats, c.GetStatistics())
203
+		}
204
+		cStats.mu.Unlock()
205
+		if err = formatter.ContainerStatsWrite(statsCtx, ccstats); err != nil {
202 206
 			break
203 207
 		}
204
-		cStats.mu.RUnlock()
205
-		if csLen == 0 && !showAll {
208
+		if len(cStats.cs) == 0 && !showAll {
206 209
 			break
207 210
 		}
208 211
 		if opts.noStream {
... ...
@@ -17,7 +17,7 @@ import (
17 17
 
18 18
 type stats struct {
19 19
 	ostype string
20
-	mu     sync.RWMutex
20
+	mu     sync.Mutex
21 21
 	cs     []*formatter.ContainerStats
22 22
 }
23 23
 
... ...
@@ -72,9 +72,7 @@ func collect(s *formatter.ContainerStats, ctx context.Context, cli client.APICli
72 72
 
73 73
 	response, err := cli.ContainerStats(ctx, s.Name, streamStats)
74 74
 	if err != nil {
75
-		s.Mu.Lock()
76
-		s.Err = err
77
-		s.Mu.Unlock()
75
+		s.SetError(err)
78 76
 		return
79 77
 	}
80 78
 	defer response.Body.Close()
... ...
@@ -88,6 +86,9 @@ func collect(s *formatter.ContainerStats, ctx context.Context, cli client.APICli
88 88
 				cpuPercent        = 0.0
89 89
 				blkRead, blkWrite uint64 // Only used on Linux
90 90
 				mem               = 0.0
91
+				memLimit          = 0.0
92
+				memPerc           = 0.0
93
+				pidsStatsCurrent  uint64
91 94
 			)
92 95
 
93 96
 			if err := dec.Decode(&v); err != nil {
... ...
@@ -113,26 +114,27 @@ func collect(s *formatter.ContainerStats, ctx context.Context, cli client.APICli
113 113
 				cpuPercent = calculateCPUPercentUnix(previousCPU, previousSystem, v)
114 114
 				blkRead, blkWrite = calculateBlockIO(v.BlkioStats)
115 115
 				mem = float64(v.MemoryStats.Usage)
116
-
116
+				memLimit = float64(v.MemoryStats.Limit)
117
+				memPerc = memPercent
118
+				pidsStatsCurrent = v.PidsStats.Current
117 119
 			} else {
118 120
 				cpuPercent = calculateCPUPercentWindows(v)
119 121
 				blkRead = v.StorageStats.ReadSizeBytes
120 122
 				blkWrite = v.StorageStats.WriteSizeBytes
121 123
 				mem = float64(v.MemoryStats.PrivateWorkingSet)
122 124
 			}
123
-
124
-			s.Mu.Lock()
125
-			s.CPUPercentage = cpuPercent
126
-			s.Memory = mem
127
-			s.NetworkRx, s.NetworkTx = calculateNetwork(v.Networks)
128
-			s.BlockRead = float64(blkRead)
129
-			s.BlockWrite = float64(blkWrite)
130
-			if daemonOSType != "windows" {
131
-				s.MemoryLimit = float64(v.MemoryStats.Limit)
132
-				s.MemoryPercentage = memPercent
133
-				s.PidsCurrent = v.PidsStats.Current
134
-			}
135
-			s.Mu.Unlock()
125
+			netRx, netTx := calculateNetwork(v.Networks)
126
+			s.SetStatistics(formatter.StatsEntry{
127
+				CPUPercentage:    cpuPercent,
128
+				Memory:           mem,
129
+				MemoryPercentage: memPerc,
130
+				MemoryLimit:      memLimit,
131
+				NetworkRx:        netRx,
132
+				NetworkTx:        netTx,
133
+				BlockRead:        float64(blkRead),
134
+				BlockWrite:       float64(blkWrite),
135
+				PidsCurrent:      pidsStatsCurrent,
136
+			})
136 137
 			u <- nil
137 138
 			if !streamStats {
138 139
 				return
... ...
@@ -144,18 +146,7 @@ func collect(s *formatter.ContainerStats, ctx context.Context, cli client.APICli
144 144
 		case <-time.After(2 * time.Second):
145 145
 			// zero out the values if we have not received an update within
146 146
 			// the specified duration.
147
-			s.Mu.Lock()
148
-			s.CPUPercentage = 0
149
-			s.Memory = 0
150
-			s.MemoryPercentage = 0
151
-			s.MemoryLimit = 0
152
-			s.NetworkRx = 0
153
-			s.NetworkTx = 0
154
-			s.BlockRead = 0
155
-			s.BlockWrite = 0
156
-			s.PidsCurrent = 0
157
-			s.Err = errors.New("timeout waiting for stats")
158
-			s.Mu.Unlock()
147
+			s.SetErrorAndReset(errors.New("timeout waiting for stats"))
159 148
 			// if this is the first stat you get, release WaitGroup
160 149
 			if !getFirst {
161 150
 				getFirst = true
... ...
@@ -163,12 +154,10 @@ func collect(s *formatter.ContainerStats, ctx context.Context, cli client.APICli
163 163
 			}
164 164
 		case err := <-u:
165 165
 			if err != nil {
166
-				s.Mu.Lock()
167
-				s.Err = err
168
-				s.Mu.Unlock()
166
+				s.SetError(err)
169 167
 				continue
170 168
 			}
171
-			s.Err = nil
169
+			s.SetError(nil)
172 170
 			// if this is the first stat you get, release WaitGroup
173 171
 			if !getFirst {
174 172
 				getFirst = true
... ...
@@ -4,27 +4,27 @@ import (
4 4
 	"fmt"
5 5
 	"sync"
6 6
 
7
-	"github.com/docker/go-units"
7
+	units "src/github.com/docker/go-units"
8 8
 )
9 9
 
10 10
 const (
11
-	defaultStatsTableFormat    = "table {{.Container}}\t{{.CPUPrec}}\t{{.MemUsage}}\t{{.MemPrec}}\t{{.NetIO}}\t{{.BlockIO}}\t{{.PIDs}}"
12
-	winDefaultStatsTableFormat = "table {{.Container}}\t{{.CPUPrec}}\t{{{.MemUsage}}\t{.NetIO}}\t{{.BlockIO}}"
11
+	winOSType                  = "windows"
12
+	defaultStatsTableFormat    = "table {{.Container}}\t{{.CPUPerc}}\t{{.MemUsage}}\t{{.MemPerc}}\t{{.NetIO}}\t{{.BlockIO}}\t{{.PIDs}}"
13
+	winDefaultStatsTableFormat = "table {{.Container}}\t{{.CPUPerc}}\t{{{.MemUsage}}\t{.NetIO}}\t{{.BlockIO}}"
13 14
 	emptyStatsTableFormat      = "Waiting for statistics..."
14 15
 
15 16
 	containerHeader  = "CONTAINER"
16
-	cpuPrecHeader    = "CPU %"
17
+	cpuPercHeader    = "CPU %"
17 18
 	netIOHeader      = "NET I/O"
18 19
 	blockIOHeader    = "BLOCK I/O"
19
-	winMemPrecHeader = "PRIV WORKING SET"  // Used only on Window
20
-	memPrecHeader    = "MEM %"             // Used only on Linux
20
+	winMemPercHeader = "PRIV WORKING SET"  // Used only on Window
21
+	memPercHeader    = "MEM %"             // Used only on Linux
21 22
 	memUseHeader     = "MEM USAGE / LIMIT" // Used only on Linux
22 23
 	pidsHeader       = "PIDS"              // Used only on Linux
23 24
 )
24 25
 
25
-// ContainerStatsAttrs represents the statistics data collected from a container.
26
-type ContainerStatsAttrs struct {
27
-	Windows          bool
26
+// StatsEntry represents represents the statistics data collected from a container
27
+type StatsEntry struct {
28 28
 	Name             string
29 29
 	CPUPercentage    float64
30 30
 	Memory           float64 // On Windows this is the private working set
... ...
@@ -35,19 +35,73 @@ type ContainerStatsAttrs struct {
35 35
 	BlockRead        float64
36 36
 	BlockWrite       float64
37 37
 	PidsCurrent      uint64 // Not used on Windows
38
+	IsInvalid        bool
39
+	OSType           string
38 40
 }
39 41
 
40
-// ContainerStats represents the containers statistics data.
42
+// ContainerStats represents an entity to store containers statistics synchronously
41 43
 type ContainerStats struct {
42
-	Mu sync.RWMutex
43
-	ContainerStatsAttrs
44
-	Err error
44
+	mutex sync.Mutex
45
+	StatsEntry
46
+	err error
47
+}
48
+
49
+// GetError returns the container statistics error.
50
+// This is used to determine whether the statistics are valid or not
51
+func (cs *ContainerStats) GetError() error {
52
+	cs.mutex.Lock()
53
+	defer cs.mutex.Unlock()
54
+	return cs.err
55
+}
56
+
57
+// SetErrorAndReset zeroes all the container statistics and store the error.
58
+// It is used when receiving time out error during statistics collecting to reduce lock overhead
59
+func (cs *ContainerStats) SetErrorAndReset(err error) {
60
+	cs.mutex.Lock()
61
+	defer cs.mutex.Unlock()
62
+	cs.CPUPercentage = 0
63
+	cs.Memory = 0
64
+	cs.MemoryPercentage = 0
65
+	cs.MemoryLimit = 0
66
+	cs.NetworkRx = 0
67
+	cs.NetworkTx = 0
68
+	cs.BlockRead = 0
69
+	cs.BlockWrite = 0
70
+	cs.PidsCurrent = 0
71
+	cs.err = err
72
+	cs.IsInvalid = true
73
+}
74
+
75
+// SetError sets container statistics error
76
+func (cs *ContainerStats) SetError(err error) {
77
+	cs.mutex.Lock()
78
+	defer cs.mutex.Unlock()
79
+	cs.err = err
80
+	if err != nil {
81
+		cs.IsInvalid = true
82
+	}
83
+}
84
+
85
+// SetStatistics set the container statistics
86
+func (cs *ContainerStats) SetStatistics(s StatsEntry) {
87
+	cs.mutex.Lock()
88
+	defer cs.mutex.Unlock()
89
+	s.Name = cs.Name
90
+	s.OSType = cs.OSType
91
+	cs.StatsEntry = s
92
+}
93
+
94
+// GetStatistics returns container statistics with other meta data such as the container name
95
+func (cs *ContainerStats) GetStatistics() StatsEntry {
96
+	cs.mutex.Lock()
97
+	defer cs.mutex.Unlock()
98
+	return cs.StatsEntry
45 99
 }
46 100
 
47 101
 // NewStatsFormat returns a format for rendering an CStatsContext
48 102
 func NewStatsFormat(source, osType string) Format {
49 103
 	if source == TableFormatKey {
50
-		if osType == "windows" {
104
+		if osType == winOSType {
51 105
 			return Format(winDefaultStatsTableFormat)
52 106
 		}
53 107
 		return Format(defaultStatsTableFormat)
... ...
@@ -58,22 +112,16 @@ func NewStatsFormat(source, osType string) Format {
58 58
 // NewContainerStats returns a new ContainerStats entity and sets in it the given name
59 59
 func NewContainerStats(name, osType string) *ContainerStats {
60 60
 	return &ContainerStats{
61
-		ContainerStatsAttrs: ContainerStatsAttrs{
62
-			Name:    name,
63
-			Windows: (osType == "windows"),
64
-		},
61
+		StatsEntry: StatsEntry{Name: name, OSType: osType},
65 62
 	}
66 63
 }
67 64
 
68 65
 // ContainerStatsWrite renders the context for a list of containers statistics
69
-func ContainerStatsWrite(ctx Context, containerStats []*ContainerStats) error {
66
+func ContainerStatsWrite(ctx Context, containerStats []StatsEntry) error {
70 67
 	render := func(format func(subContext subContext) error) error {
71 68
 		for _, cstats := range containerStats {
72
-			cstats.Mu.RLock()
73
-			cstatsAttrs := cstats.ContainerStatsAttrs
74
-			cstats.Mu.RUnlock()
75 69
 			containerStatsCtx := &containerStatsContext{
76
-				s: cstatsAttrs,
70
+				s: cstats,
77 71
 			}
78 72
 			if err := format(containerStatsCtx); err != nil {
79 73
 				return err
... ...
@@ -86,7 +134,7 @@ func ContainerStatsWrite(ctx Context, containerStats []*ContainerStats) error {
86 86
 
87 87
 type containerStatsContext struct {
88 88
 	HeaderContext
89
-	s ContainerStatsAttrs
89
+	s StatsEntry
90 90
 }
91 91
 
92 92
 func (c *containerStatsContext) Container() string {
... ...
@@ -94,42 +142,54 @@ func (c *containerStatsContext) Container() string {
94 94
 	return c.s.Name
95 95
 }
96 96
 
97
-func (c *containerStatsContext) CPUPrec() string {
98
-	c.AddHeader(cpuPrecHeader)
97
+func (c *containerStatsContext) CPUPerc() string {
98
+	c.AddHeader(cpuPercHeader)
99
+	if c.s.IsInvalid {
100
+		return fmt.Sprintf("--")
101
+	}
99 102
 	return fmt.Sprintf("%.2f%%", c.s.CPUPercentage)
100 103
 }
101 104
 
102 105
 func (c *containerStatsContext) MemUsage() string {
103 106
 	c.AddHeader(memUseHeader)
104
-	if !c.s.Windows {
105
-		return fmt.Sprintf("%s / %s", units.BytesSize(c.s.Memory), units.BytesSize(c.s.MemoryLimit))
107
+	if c.s.IsInvalid || c.s.OSType == winOSType {
108
+		return fmt.Sprintf("-- / --")
106 109
 	}
107
-	return fmt.Sprintf("-- / --")
110
+	return fmt.Sprintf("%s / %s", units.BytesSize(c.s.Memory), units.BytesSize(c.s.MemoryLimit))
108 111
 }
109 112
 
110
-func (c *containerStatsContext) MemPrec() string {
111
-	header := memPrecHeader
112
-	if c.s.Windows {
113
-		header = winMemPrecHeader
113
+func (c *containerStatsContext) MemPerc() string {
114
+	header := memPercHeader
115
+	if c.s.OSType == winOSType {
116
+		header = winMemPercHeader
114 117
 	}
115 118
 	c.AddHeader(header)
119
+	if c.s.IsInvalid {
120
+		return fmt.Sprintf("--")
121
+	}
116 122
 	return fmt.Sprintf("%.2f%%", c.s.MemoryPercentage)
117 123
 }
118 124
 
119 125
 func (c *containerStatsContext) NetIO() string {
120 126
 	c.AddHeader(netIOHeader)
127
+	if c.s.IsInvalid {
128
+		return fmt.Sprintf("--")
129
+	}
121 130
 	return fmt.Sprintf("%s / %s", units.HumanSizeWithPrecision(c.s.NetworkRx, 3), units.HumanSizeWithPrecision(c.s.NetworkTx, 3))
122 131
 }
123 132
 
124 133
 func (c *containerStatsContext) BlockIO() string {
125 134
 	c.AddHeader(blockIOHeader)
135
+	if c.s.IsInvalid {
136
+		return fmt.Sprintf("--")
137
+	}
126 138
 	return fmt.Sprintf("%s / %s", units.HumanSizeWithPrecision(c.s.BlockRead, 3), units.HumanSizeWithPrecision(c.s.BlockWrite, 3))
127 139
 }
128 140
 
129 141
 func (c *containerStatsContext) PIDs() string {
130 142
 	c.AddHeader(pidsHeader)
131
-	if !c.s.Windows {
132
-		return fmt.Sprintf("%d", c.s.PidsCurrent)
143
+	if c.s.IsInvalid || c.s.OSType == winOSType {
144
+		return fmt.Sprintf("--")
133 145
 	}
134
-	return fmt.Sprintf("-")
146
+	return fmt.Sprintf("%d", c.s.PidsCurrent)
135 147
 }