Skip to content

Commit b4cda95

Browse files
matix522matix522
committed
Fix the restart issues with perfdash
The perfdash pod have been restarting every few hours due to `concurrent map write`. Added a RWMutex to guard the map access
1 parent fef7446 commit b4cda95

File tree

7 files changed

+117
-29
lines changed

7 files changed

+117
-29
lines changed

‎perfdash/Dockerfile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
ARG GOLANG_VERSION=1.23
15+
ARG GOLANG_VERSION=1.24
1616
FROM golang:${GOLANG_VERSION} AS builder
1717

1818
WORKDIR /workspace

‎perfdash/Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
all: push
22

33
# See deployment.yaml for the version currently running-- bump this ahead before rebuilding!
4-
TAG?=2.54
4+
TAG?=2.55
55

66
REPO?=gcr.io/k8s-staging-perf-tests
77

‎perfdash/deployment.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ spec:
1616
spec:
1717
containers:
1818
- name: perfdash
19-
image: gcr.io/k8s-staging-perf-tests/perfdash:2.54
19+
image: gcr.io/k8s-staging-perf-tests/perfdash:2.55
2020
command:
2121
- /perfdash
2222
- --www=true

‎perfdash/metrics-downloader-helper.go‎

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,57 @@ package main
1919
import (
2020
"encoding/json"
2121
"fmt"
22-
"k8s.io/klog"
2322
"net/http"
2423
"sort"
24+
"sync"
2525

26+
"k8s.io/klog"
2627
"k8s.io/kubernetes/test/e2e/perftype"
2728
)
2829

2930
// BuildData contains job name and a map from build number to perf data.
3031
type BuildData struct {
31-
Builds map[string][]perftype.DataItem `json:"builds"`
32-
Job string `json:"job"`
33-
Version string `json:"version"`
32+
Builds Builds `json:"builds"`
33+
Job string `json:"job"`
34+
Version string `json:"version"`
35+
}
36+
37+
// Builds is a structure contains build number to perf data map guarded against concurent modification
38+
type Builds struct {
39+
builds map[string][]perftype.DataItem
40+
mu *sync.RWMutex
41+
}
42+
43+
func (b *Builds) MarshalJSON() ([]byte, error) {
44+
return json.Marshal(b.builds)
45+
}
46+
47+
func (b *Builds) UnmarshalJSON(data []byte) error {
48+
b.mu = &sync.RWMutex{}
49+
return json.Unmarshal(data, &b.builds)
50+
}
51+
52+
// NewBuilds returns a guarded build number to perf data map.
53+
func NewBuilds(builds map[string][]perftype.DataItem) Builds {
54+
if builds == nil {
55+
builds = make(map[string][]perftype.DataItem)
56+
}
57+
return Builds{
58+
builds: builds,
59+
mu: &sync.RWMutex{},
60+
}
61+
}
62+
63+
func (b *Builds) AddBuildData(buildNumber string, data perftype.DataItem) {
64+
b.mu.Lock()
65+
defer b.mu.Unlock()
66+
b.builds[buildNumber] = append(b.builds[buildNumber], data)
67+
}
68+
69+
func (b *Builds) Builds(buildNumber string) []perftype.DataItem {
70+
b.mu.RLock()
71+
defer b.mu.RUnlock()
72+
return b.builds[buildNumber]
3473
}
3574

3675
// MetricToBuildData is a map from metric name to BuildData pointer.

‎perfdash/metrics-downloader.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ func getBuildData(result JobToCategoryData, prefix string, category string, labe
224224
result[prefix][category] = make(MetricToBuildData)
225225
}
226226
if _, found := result[prefix][category][label]; !found {
227-
result[prefix][category][label] = &BuildData{Job: job, Version: "", Builds: map[string][]perftype.DataItem{}}
227+
result[prefix][category][label] = &BuildData{Job: job, Version: "", Builds: NewBuilds(map[string][]perftype.DataItem{})}
228228
}
229229
return result[prefix][category][label]
230230
}

‎perfdash/parser.go‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func parseContainerRestarts(data []byte, buildNumber int, testResult *BuildData)
7373
}
7474

7575
res := perftype.DataItem{Unit: "", Labels: map[string]string{"RestartCount": "RestartCount"}, Data: restartsByContainer}
76-
testResult.Builds[build] = append(testResult.Builds[build], res)
76+
testResult.Builds.AddBuildData(build, res)
7777
}
7878

7979
func parsePerfData(data []byte, buildNumber int, testResult *BuildData) {
@@ -89,7 +89,7 @@ func parsePerfData(data []byte, buildNumber int, testResult *BuildData) {
8989
if testResult.Version == obj.Version {
9090
for i := range obj.DataItems {
9191
stripCount(&obj.DataItems[i])
92-
testResult.Builds[build] = append(testResult.Builds[build], obj.DataItems[i])
92+
testResult.Builds.AddBuildData(build, obj.DataItems[i])
9393
}
9494
}
9595
}
@@ -142,8 +142,8 @@ func parseResourceUsageData(data []byte, buildNumber int, testResult *BuildData)
142142
cpu.Data[percentile] = usage.CPU
143143
memory.Data[percentile] = usage.Memory / (1024 * 1024)
144144
}
145-
testResult.Builds[build] = append(testResult.Builds[build], cpu)
146-
testResult.Builds[build] = append(testResult.Builds[build], memory)
145+
testResult.Builds.AddBuildData(build, cpu)
146+
testResult.Builds.AddBuildData(build, memory)
147147
}
148148
}
149149

@@ -164,7 +164,7 @@ func parseRequestCountData(data []byte, buildNumber int, testResult *BuildData)
164164
continue
165165
}
166166
stripCount(&obj.DataItems[i])
167-
testResult.Builds[build] = append(testResult.Builds[build], obj.DataItems[i])
167+
testResult.Builds.AddBuildData(build, obj.DataItems[i])
168168
}
169169
}
170170
}
@@ -217,7 +217,7 @@ func parseApiserverRequestCount(data []byte, buildNumber int, testResult *BuildD
217217
continue
218218
}
219219
resultMap[key] = &perfData
220-
testResult.Builds[build] = append(testResult.Builds[build], perfData)
220+
testResult.Builds.AddBuildData(build, perfData)
221221
}
222222
}
223223

@@ -245,7 +245,7 @@ func parseApiserverInitEventsCount(data []byte, buildNumber int, testResult *Bui
245245
}
246246
delete(perfData.Labels, "__name__")
247247
perfData.Data["InitEventsCount"] = float64(metric[i].Value)
248-
testResult.Builds[build] = append(testResult.Builds[build], perfData)
248+
testResult.Builds.AddBuildData(build, perfData)
249249
}
250250
}
251251

@@ -296,15 +296,15 @@ func parseSchedulingLatency(testName string) func([]byte, int, *BuildData) {
296296
return
297297
}
298298
preemptionEvaluation := parseOperationLatency(obj.PreemptionEvaluationLatency, testName, "preemption_evaluation")
299-
testResult.Builds[build] = append(testResult.Builds[build], preemptionEvaluation)
299+
testResult.Builds.AddBuildData(build, preemptionEvaluation)
300300
e2eScheduling := parseOperationLatency(obj.E2eSchedulingLatency, testName, "e2eScheduling")
301-
testResult.Builds[build] = append(testResult.Builds[build], e2eScheduling)
301+
testResult.Builds.AddBuildData(build, e2eScheduling)
302302
scheduling := parseOperationLatency(obj.SchedulingLatency, testName, "scheduling")
303-
testResult.Builds[build] = append(testResult.Builds[build], scheduling)
303+
testResult.Builds.AddBuildData(build, scheduling)
304304

305305
for name, metric := range obj.FrameworkExtensionPointDuration {
306306
frameworkExtensionPointDuration := parseOperationLatency(metric, testName, name)
307-
testResult.Builds[build] = append(testResult.Builds[build], frameworkExtensionPointDuration)
307+
testResult.Builds.AddBuildData(build, frameworkExtensionPointDuration)
308308
}
309309
}
310310
}
@@ -332,7 +332,7 @@ func parseSchedulingThroughputCL(testName string) func([]byte, int, *BuildData)
332332
perfData.Data["Perc99"] = obj.Perc99
333333
perfData.Data["Average"] = obj.Average
334334
perfData.Data["Max"] = obj.Max
335-
testResult.Builds[build] = append(testResult.Builds[build], perfData)
335+
testResult.Builds.AddBuildData(build, perfData)
336336
}
337337
}
338338

@@ -393,7 +393,7 @@ func parseHistogramMetric(metricName string) func(data []byte, buildNumber int,
393393
perfData.Data["<= "+bucket+"s"] = float64(buckerVal) / float64(count) * 100
394394
}
395395
}
396-
testResult.Builds[build] = append(testResult.Builds[build], perfData)
396+
testResult.Builds.AddBuildData(build, perfData)
397397
}
398398
}
399399
}
@@ -439,5 +439,5 @@ func parseSystemPodMetrics(data []byte, buildNumber int, testResult *BuildData)
439439
},
440440
Data: restartCounts,
441441
}
442-
testResult.Builds[build] = append(testResult.Builds[build], perfData)
442+
testResult.Builds.AddBuildData(build, perfData)
443443
}

‎perfdash/parser_test.go‎

Lines changed: 56 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package main
1818

1919
import (
20+
"encoding/json"
2021
"reflect"
2122
"testing"
2223

@@ -37,8 +38,8 @@ func Test_parseSystemPodMetrics(t *testing.T) {
3738
name: "same-container-in-two-pods",
3839
buildNumber: 123,
3940
data: sameContainerInTwoPodsSummary(),
40-
testResult: &BuildData{Job: "", Version: "", Builds: map[string][]perftype.DataItem{}},
41-
want: &BuildData{Job: "", Version: "", Builds: map[string][]perftype.DataItem{
41+
testResult: &BuildData{Job: "", Version: "", Builds: NewBuilds(map[string][]perftype.DataItem{})},
42+
want: &BuildData{Job: "", Version: "", Builds: NewBuilds(map[string][]perftype.DataItem{
4243
"123": {
4344
perftype.DataItem{
4445
Data: map[string]float64{
@@ -51,7 +52,8 @@ func Test_parseSystemPodMetrics(t *testing.T) {
5152
Unit: "",
5253
},
5354
},
54-
}},
55+
}),
56+
},
5557
},
5658
}
5759
for _, tt := range tests {
@@ -122,7 +124,7 @@ func Test_parseContainerRestarts(t *testing.T) {
122124
"RestartCount": 4
123125
}]`),
124126
want: &BuildData{
125-
Builds: map[string][]perftype.DataItem{
127+
Builds: NewBuilds(map[string][]perftype.DataItem{
126128
"123": {
127129
{
128130
Data: map[string]float64{
@@ -132,16 +134,63 @@ func Test_parseContainerRestarts(t *testing.T) {
132134
Labels: map[string]string{"RestartCount": "RestartCount"},
133135
},
134136
},
135-
},
137+
}),
136138
},
137139
},
138140
}
139141
for _, tc := range tests {
140142
t.Run(tc.name, func(t *testing.T) {
141-
got := &BuildData{Builds: map[string][]perftype.DataItem{}}
143+
got := &BuildData{Builds: NewBuilds(nil)}
142144
parseContainerRestarts(tc.data, 123, got)
143145
require.NotNil(t, got.Builds)
144-
assert.ElementsMatch(t, tc.want.Builds["123"], got.Builds["123"])
146+
assert.ElementsMatch(t, tc.want.Builds.Builds("123"), got.Builds.Builds("123"))
147+
})
148+
}
149+
}
150+
151+
func Test_BuildDataToJson(t *testing.T) {
152+
tests := []struct {
153+
name string
154+
buildData *BuildData
155+
want string
156+
}{
157+
{
158+
name: "ToJsonTest",
159+
buildData: &BuildData{Job: "job", Version: "version", Builds: NewBuilds(map[string][]perftype.DataItem{
160+
"123": {
161+
perftype.DataItem{
162+
Data: map[string]float64{
163+
"c1": 2,
164+
"c2": 1,
165+
},
166+
Labels: map[string]string{
167+
"RestartCount": "RestartCount",
168+
},
169+
Unit: "unit",
170+
},
171+
},
172+
}),
173+
},
174+
want: `{"builds":{"123":[{"data":{"c1":2,"c2":1},"unit":"unit","labels":{"RestartCount":"RestartCount"}}]},"job":"job","version":"version"}`,
175+
},
176+
}
177+
178+
for _, tc := range tests {
179+
t.Run(tc.name, func(t *testing.T) {
180+
jsonData, err := json.Marshal(tc.buildData)
181+
if err != nil {
182+
t.Fatalf("failed to marshal data into json %v", err)
183+
}
184+
require.NotNil(t, jsonData)
185+
assert.Equal(t, tc.want, string(jsonData))
186+
got := &BuildData{}
187+
err = json.Unmarshal(jsonData, got)
188+
if err != nil {
189+
t.Fatalf("failed to unmarshal data into json %v", err)
190+
}
191+
if !reflect.DeepEqual(*got, *tc.buildData) {
192+
t.Errorf("want %v, got %v", *tc.buildData, *got)
193+
}
145194
})
146195
}
147196
}

0 commit comments

Comments
 (0)