Skip to content

Commit 25a6fa1

Browse files
ning0515ning0515
committed
There is no need to set the worker to nil to avoid potential timing issues.
1 parent 5e220e4 commit 25a6fa1

File tree

2 files changed

+41
-51
lines changed

2 files changed

+41
-51
lines changed

‎pkg/controller/tainteviction/timed_workers.go‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -105,13 +105,7 @@ func (q *TimedWorkerQueue) getWrappedWorkerFunc(key string) func(ctx context.Con
105105
err := q.workFunc(ctx, fireAt, args)
106106
q.Lock()
107107
defer q.Unlock()
108-
if err == nil {
109-
// To avoid duplicated calls we keep the key in the queue, to prevent
110-
// subsequent additions.
111-
q.workers[key] = nil
112-
} else {
113-
delete(q.workers, key)
114-
}
108+
delete(q.workers, key)
115109
return err
116110
}
117111
}

‎pkg/controller/tainteviction/timed_workers_test.go‎

Lines changed: 40 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
)
2929

3030
func TestExecute(t *testing.T) {
31+
_, ctx := ktesting.NewTestContext(t)
3132
testVal := int32(0)
3233
wg := sync.WaitGroup{}
3334
wg.Add(5)
@@ -37,17 +38,11 @@ func TestExecute(t *testing.T) {
3738
return nil
3839
})
3940
now := time.Now()
40-
queue.AddWork(context.TODO(), NewWorkArgs("1", "1"), now, now)
41-
queue.AddWork(context.TODO(), NewWorkArgs("2", "2"), now, now)
42-
queue.AddWork(context.TODO(), NewWorkArgs("3", "3"), now, now)
43-
queue.AddWork(context.TODO(), NewWorkArgs("4", "4"), now, now)
44-
queue.AddWork(context.TODO(), NewWorkArgs("5", "5"), now, now)
45-
// Adding the same thing second time should be no-op
46-
queue.AddWork(context.TODO(), NewWorkArgs("1", "1"), now, now)
47-
queue.AddWork(context.TODO(), NewWorkArgs("2", "2"), now, now)
48-
queue.AddWork(context.TODO(), NewWorkArgs("3", "3"), now, now)
49-
queue.AddWork(context.TODO(), NewWorkArgs("4", "4"), now, now)
50-
queue.AddWork(context.TODO(), NewWorkArgs("5", "5"), now, now)
41+
queue.AddWork(ctx, NewWorkArgs("1", "1"), now, now)
42+
queue.AddWork(ctx, NewWorkArgs("2", "2"), now, now)
43+
queue.AddWork(ctx, NewWorkArgs("3", "3"), now, now)
44+
queue.AddWork(ctx, NewWorkArgs("4", "4"), now, now)
45+
queue.AddWork(ctx, NewWorkArgs("5", "5"), now, now)
5146
wg.Wait()
5247
lastVal := atomic.LoadInt32(&testVal)
5348
if lastVal != 5 {
@@ -56,6 +51,7 @@ func TestExecute(t *testing.T) {
5651
}
5752

5853
func TestExecuteDelayed(t *testing.T) {
54+
_, ctx := ktesting.NewTestContext(t)
5955
testVal := int32(0)
6056
wg := sync.WaitGroup{}
6157
wg.Add(5)
@@ -68,16 +64,16 @@ func TestExecuteDelayed(t *testing.T) {
6864
then := now.Add(10 * time.Second)
6965
fakeClock := testingclock.NewFakeClock(now)
7066
queue.clock = fakeClock
71-
queue.AddWork(context.TODO(), NewWorkArgs("1", "1"), now, then)
72-
queue.AddWork(context.TODO(), NewWorkArgs("2", "2"), now, then)
73-
queue.AddWork(context.TODO(), NewWorkArgs("3", "3"), now, then)
74-
queue.AddWork(context.TODO(), NewWorkArgs("4", "4"), now, then)
75-
queue.AddWork(context.TODO(), NewWorkArgs("5", "5"), now, then)
76-
queue.AddWork(context.TODO(), NewWorkArgs("1", "1"), now, then)
77-
queue.AddWork(context.TODO(), NewWorkArgs("2", "2"), now, then)
78-
queue.AddWork(context.TODO(), NewWorkArgs("3", "3"), now, then)
79-
queue.AddWork(context.TODO(), NewWorkArgs("4", "4"), now, then)
80-
queue.AddWork(context.TODO(), NewWorkArgs("5", "5"), now, then)
67+
queue.AddWork(ctx, NewWorkArgs("1", "1"), now, then)
68+
queue.AddWork(ctx, NewWorkArgs("2", "2"), now, then)
69+
queue.AddWork(ctx, NewWorkArgs("3", "3"), now, then)
70+
queue.AddWork(ctx, NewWorkArgs("4", "4"), now, then)
71+
queue.AddWork(ctx, NewWorkArgs("5", "5"), now, then)
72+
queue.AddWork(ctx, NewWorkArgs("1", "1"), now, then)
73+
queue.AddWork(ctx, NewWorkArgs("2", "2"), now, then)
74+
queue.AddWork(ctx, NewWorkArgs("3", "3"), now, then)
75+
queue.AddWork(ctx, NewWorkArgs("4", "4"), now, then)
76+
queue.AddWork(ctx, NewWorkArgs("5", "5"), now, then)
8177
fakeClock.Step(11 * time.Second)
8278
wg.Wait()
8379
lastVal := atomic.LoadInt32(&testVal)
@@ -87,6 +83,7 @@ func TestExecuteDelayed(t *testing.T) {
8783
}
8884

8985
func TestCancel(t *testing.T) {
86+
logger, ctx := ktesting.NewTestContext(t)
9087
testVal := int32(0)
9188
wg := sync.WaitGroup{}
9289
wg.Add(3)
@@ -99,17 +96,16 @@ func TestCancel(t *testing.T) {
9996
then := now.Add(10 * time.Second)
10097
fakeClock := testingclock.NewFakeClock(now)
10198
queue.clock = fakeClock
102-
queue.AddWork(context.TODO(), NewWorkArgs("1", "1"), now, then)
103-
queue.AddWork(context.TODO(), NewWorkArgs("2", "2"), now, then)
104-
queue.AddWork(context.TODO(), NewWorkArgs("3", "3"), now, then)
105-
queue.AddWork(context.TODO(), NewWorkArgs("4", "4"), now, then)
106-
queue.AddWork(context.TODO(), NewWorkArgs("5", "5"), now, then)
107-
queue.AddWork(context.TODO(), NewWorkArgs("1", "1"), now, then)
108-
queue.AddWork(context.TODO(), NewWorkArgs("2", "2"), now, then)
109-
queue.AddWork(context.TODO(), NewWorkArgs("3", "3"), now, then)
110-
queue.AddWork(context.TODO(), NewWorkArgs("4", "4"), now, then)
111-
queue.AddWork(context.TODO(), NewWorkArgs("5", "5"), now, then)
112-
logger, _ := ktesting.NewTestContext(t)
99+
queue.AddWork(ctx, NewWorkArgs("1", "1"), now, then)
100+
queue.AddWork(ctx, NewWorkArgs("2", "2"), now, then)
101+
queue.AddWork(ctx, NewWorkArgs("3", "3"), now, then)
102+
queue.AddWork(ctx, NewWorkArgs("4", "4"), now, then)
103+
queue.AddWork(ctx, NewWorkArgs("5", "5"), now, then)
104+
queue.AddWork(ctx, NewWorkArgs("1", "1"), now, then)
105+
queue.AddWork(ctx, NewWorkArgs("2", "2"), now, then)
106+
queue.AddWork(ctx, NewWorkArgs("3", "3"), now, then)
107+
queue.AddWork(ctx, NewWorkArgs("4", "4"), now, then)
108+
queue.AddWork(ctx, NewWorkArgs("5", "5"), now, then)
113109
queue.CancelWork(logger, NewWorkArgs("2", "2").KeyFromWorkArgs())
114110
queue.CancelWork(logger, NewWorkArgs("4", "4").KeyFromWorkArgs())
115111
fakeClock.Step(11 * time.Second)
@@ -121,6 +117,7 @@ func TestCancel(t *testing.T) {
121117
}
122118

123119
func TestCancelAndReadd(t *testing.T) {
120+
logger, ctx := ktesting.NewTestContext(t)
124121
testVal := int32(0)
125122
wg := sync.WaitGroup{}
126123
wg.Add(4)
@@ -133,20 +130,19 @@ func TestCancelAndReadd(t *testing.T) {
133130
then := now.Add(10 * time.Second)
134131
fakeClock := testingclock.NewFakeClock(now)
135132
queue.clock = fakeClock
136-
queue.AddWork(context.TODO(), NewWorkArgs("1", "1"), now, then)
137-
queue.AddWork(context.TODO(), NewWorkArgs("2", "2"), now, then)
138-
queue.AddWork(context.TODO(), NewWorkArgs("3", "3"), now, then)
139-
queue.AddWork(context.TODO(), NewWorkArgs("4", "4"), now, then)
140-
queue.AddWork(context.TODO(), NewWorkArgs("5", "5"), now, then)
141-
queue.AddWork(context.TODO(), NewWorkArgs("1", "1"), now, then)
142-
queue.AddWork(context.TODO(), NewWorkArgs("2", "2"), now, then)
143-
queue.AddWork(context.TODO(), NewWorkArgs("3", "3"), now, then)
144-
queue.AddWork(context.TODO(), NewWorkArgs("4", "4"), now, then)
145-
queue.AddWork(context.TODO(), NewWorkArgs("5", "5"), now, then)
146-
logger, _ := ktesting.NewTestContext(t)
133+
queue.AddWork(ctx, NewWorkArgs("1", "1"), now, then)
134+
queue.AddWork(ctx, NewWorkArgs("2", "2"), now, then)
135+
queue.AddWork(ctx, NewWorkArgs("3", "3"), now, then)
136+
queue.AddWork(ctx, NewWorkArgs("4", "4"), now, then)
137+
queue.AddWork(ctx, NewWorkArgs("5", "5"), now, then)
138+
queue.AddWork(ctx, NewWorkArgs("1", "1"), now, then)
139+
queue.AddWork(ctx, NewWorkArgs("2", "2"), now, then)
140+
queue.AddWork(ctx, NewWorkArgs("3", "3"), now, then)
141+
queue.AddWork(ctx, NewWorkArgs("4", "4"), now, then)
142+
queue.AddWork(ctx, NewWorkArgs("5", "5"), now, then)
147143
queue.CancelWork(logger, NewWorkArgs("2", "2").KeyFromWorkArgs())
148144
queue.CancelWork(logger, NewWorkArgs("4", "4").KeyFromWorkArgs())
149-
queue.AddWork(context.TODO(), NewWorkArgs("2", "2"), now, then)
145+
queue.AddWork(ctx, NewWorkArgs("2", "2"), now, then)
150146
fakeClock.Step(11 * time.Second)
151147
wg.Wait()
152148
lastVal := atomic.LoadInt32(&testVal)

0 commit comments

Comments
 (0)