Skip to content

Commit cd1ddbf

Browse files
authored
Merge pull request kubernetes#88600 from apelisse/at-most-every
SHOULD NOT HAPPEN: logging "SHOULD NOT HAPPEN" errors more than once per second
2 parents 7a513b5 + 389dd0a commit cd1ddbf

File tree

4 files changed

+120
-2
lines changed

4 files changed

+120
-2
lines changed

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
33
go_library(
44
name = "go_default_library",
55
srcs = [
6+
"atmostevery.go",
67
"conflict.go",
78
"fields.go",
89
"gvkparser.go",
@@ -33,6 +34,7 @@ go_library(
3334
go_test(
3435
name = "go_default_test",
3536
srcs = [
37+
"atmostevery_test.go",
3638
"conflict_test.go",
3739
"fields_test.go",
3840
"managedfields_test.go",
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/*
2+
Copyright 2020 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package internal
18+
19+
import (
20+
"sync"
21+
"time"
22+
)
23+
24+
// AtMostEvery will never run the method more than once every specified
25+
// duration.
26+
type AtMostEvery struct {
27+
delay time.Duration
28+
lastCall time.Time
29+
mutex sync.Mutex
30+
}
31+
32+
// NewAtMostEvery creates a new AtMostEvery, that will run the method at
33+
// most every given duration.
34+
func NewAtMostEvery(delay time.Duration) *AtMostEvery {
35+
return &AtMostEvery{
36+
delay: delay,
37+
}
38+
}
39+
40+
// updateLastCall returns true if the lastCall time has been updated,
41+
// false if it was too early.
42+
func (s *AtMostEvery) updateLastCall() bool {
43+
s.mutex.Lock()
44+
defer s.mutex.Unlock()
45+
if time.Since(s.lastCall) < s.delay {
46+
return false
47+
}
48+
s.lastCall = time.Now()
49+
return true
50+
}
51+
52+
// Do will run the method if enough time has passed, and return true.
53+
// Otherwise, it does nothing and returns false.
54+
func (s *AtMostEvery) Do(fn func()) bool {
55+
if !s.updateLastCall() {
56+
return false
57+
}
58+
fn()
59+
return true
60+
}
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*
2+
Copyright 2020 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package internal_test
18+
19+
import (
20+
"testing"
21+
"time"
22+
23+
"k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/internal"
24+
)
25+
26+
func TestAtMostEvery(t *testing.T) {
27+
duration := time.Second
28+
delay := 179 * time.Millisecond
29+
atMostEvery := internal.NewAtMostEvery(delay)
30+
count := 0
31+
exit := time.NewTicker(duration)
32+
tick := time.NewTicker(2 * time.Millisecond)
33+
defer exit.Stop()
34+
defer tick.Stop()
35+
36+
done := false
37+
for !done {
38+
select {
39+
case <-exit.C:
40+
done = true
41+
case <-tick.C:
42+
atMostEvery.Do(func() {
43+
count++
44+
})
45+
}
46+
}
47+
48+
if expected := int(duration/delay) + 1; count != expected {
49+
t.Fatalf("Function called %d times, should have been called exactly %d times", count, expected)
50+
}
51+
}

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/structuredmerge.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ type structuredMergeManager struct {
4242
}
4343

4444
var _ Manager = &structuredMergeManager{}
45+
var atMostEverySecond = internal.NewAtMostEvery(time.Second)
4546

4647
// NewStructuredMergeManager creates a new Manager that merges apply requests
4748
// and update managed fields for other types of requests.
@@ -99,13 +100,17 @@ func (f *structuredMergeManager) Update(liveObj, newObj runtime.Object, managed
99100
newObjTyped, err := f.typeConverter.ObjectToTyped(newObjVersioned)
100101
if err != nil {
101102
// Return newObj and just by-pass fields update. This really shouldn't happen.
102-
klog.Errorf("[SHOULD NOT HAPPEN] failed to create typed new object of type %v: %v", newObjVersioned.GetObjectKind().GroupVersionKind(), err)
103+
atMostEverySecond.Do(func() {
104+
klog.Errorf("[SHOULD NOT HAPPEN] failed to create typed new object of type %v: %v", newObjVersioned.GetObjectKind().GroupVersionKind(), err)
105+
})
103106
return newObj, managed, nil
104107
}
105108
liveObjTyped, err := f.typeConverter.ObjectToTyped(liveObjVersioned)
106109
if err != nil {
107110
// Return newObj and just by-pass fields update. This really shouldn't happen.
108-
klog.Errorf("[SHOULD NOT HAPPEN] failed to create typed live object of type %v: %v", liveObjVersioned.GetObjectKind().GroupVersionKind(), err)
111+
atMostEverySecond.Do(func() {
112+
klog.Errorf("[SHOULD NOT HAPPEN] failed to create typed live object of type %v: %v", liveObjVersioned.GetObjectKind().GroupVersionKind(), err)
113+
})
109114
return newObj, managed, nil
110115
}
111116
apiVersion := fieldpath.APIVersion(f.groupVersion.String())

0 commit comments

Comments
 (0)