Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix data-race in reflector test in Go 1.5.1 #18499

Merged
merged 1 commit into from
Dec 15, 2015
Merged

Fix data-race in reflector test in Go 1.5.1 #18499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 6 additions & 5 deletions pkg/client/cache/reflector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ type Reflector struct {
// the beginning of the next one.
period time.Duration
resyncPeriod time.Duration
// now() returns current time - exposed for testing purposes
now func() time.Time
// nextResync is approximate time of next resync (0 if not scheduled)
nextResync time.Time
// lastSyncResourceVersion is the resource version token last
Expand All @@ -79,8 +81,6 @@ var (
// However, it can be modified to avoid periodic resync to break the
// TCP connection.
minWatchTimeout = 5 * time.Minute

now func() time.Time = time.Now
// If we are within 'forceResyncThreshold' from the next planned resync
// and are just before issueing Watch(), resync will be forced now.
forceResyncThreshold = 3 * time.Second
Expand Down Expand Up @@ -116,6 +116,7 @@ func NewNamedReflector(name string, lw ListerWatcher, expectedType interface{},
expectedType: reflect.TypeOf(expectedType),
period: time.Second,
resyncPeriod: resyncPeriod,
now: time.Now,
}
return r
}
Expand Down Expand Up @@ -187,7 +188,7 @@ func (r *Reflector) resyncChan() (<-chan time.Time, func() bool) {
// always fail so we end up listing frequently. Then, if we don't
// manually stop the timer, we could end up with many timers active
// concurrently.
r.nextResync = now().Add(r.resyncPeriod)
r.nextResync = r.now().Add(r.resyncPeriod)
t := time.NewTimer(r.resyncPeriod)
return t.C, t.Stop
}
Expand All @@ -204,7 +205,7 @@ func (r *Reflector) resyncChan() (<-chan time.Time, func() bool) {
// TODO: This should be parametrizable based on server load.
func (r *Reflector) timeoutForWatch() *int64 {
randTimeout := time.Duration(float64(minWatchTimeout) * (rand.Float64() + 1.0))
timeout := r.nextResync.Sub(now()) - timeoutThreshold
timeout := r.nextResync.Sub(r.now()) - timeoutThreshold
if timeout < 0 || randTimeout < timeout {
timeout = randTimeout
}
Expand All @@ -218,7 +219,7 @@ func (r *Reflector) canForceResyncNow() bool {
if r.nextResync.IsZero() {
return false
}
return now().Add(forceResyncThreshold).After(r.nextResync)
return r.now().Add(forceResyncThreshold).After(r.nextResync)
}

// Returns error if ListAndWatch didn't even tried to initialize watch.
Expand Down
2 changes: 1 addition & 1 deletion pkg/client/cache/reflector_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,6 @@ func TestReflectorResync(t *testing.T) {
s := NewStore(MetaNamespaceKeyFunc)

currentTime := time.Time{}
now = func() time.Time { return currentTime }
iteration := 0

lw := &testLW{
Expand Down Expand Up @@ -396,6 +395,7 @@ func TestReflectorResync(t *testing.T) {
}
resyncPeriod := time.Minute
r := NewReflector(lw, &api.Pod{}, s, resyncPeriod)
r.now = func() time.Time { return currentTime }

r.ListAndWatch(util.NeverStop)
if iteration != 2 {
Expand Down