Skip to content

Commit 7633ade

Browse files
vmihailencovmihailenco
authored
Merge pull request #1688 from monkey92t/data_race
fix ring test `Process hook` data race
2 parents f3a31a3 + 4a7d033 commit 7633ade

File tree

1 file changed

+115
-106
lines changed

1 file changed

+115
-106
lines changed

‎ring_test.go

Lines changed: 115 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -188,153 +188,162 @@ var _ = Describe("Redis Ring", func() {
188188
})
189189
})
190190

191-
It("supports Process hook", func() {
192-
err := ring.Ping(ctx).Err()
193-
Expect(err).NotTo(HaveOccurred())
194-
195-
var stack []string
196-
197-
ring.AddHook(&hook{
198-
beforeProcess: func(ctx context.Context, cmd redis.Cmder) (context.Context, error) {
199-
Expect(cmd.String()).To(Equal("ping: "))
200-
stack = append(stack, "ring.BeforeProcess")
201-
return ctx, nil
202-
},
203-
afterProcess: func(ctx context.Context, cmd redis.Cmder) error {
204-
Expect(cmd.String()).To(Equal("ping: PONG"))
205-
stack = append(stack, "ring.AfterProcess")
206-
return nil
207-
},
191+
Describe("Process hook", func() {
192+
BeforeEach(func() {
193+
//the health check leads to data race for variable "stack []string".
194+
//here, the health check time is set to 72 hours to avoid health check
195+
opt := redisRingOptions()
196+
opt.HeartbeatFrequency = 72 * time.Hour
197+
ring = redis.NewRing(opt)
208198
})
199+
It("supports Process hook", func() {
200+
err := ring.Ping(ctx).Err()
201+
Expect(err).NotTo(HaveOccurred())
209202

210-
ring.ForEachShard(ctx, func(ctx context.Context, shard *redis.Client) error {
211-
shard.AddHook(&hook{
203+
var stack []string
204+
205+
ring.AddHook(&hook{
212206
beforeProcess: func(ctx context.Context, cmd redis.Cmder) (context.Context, error) {
213207
Expect(cmd.String()).To(Equal("ping: "))
214-
stack = append(stack, "shard.BeforeProcess")
208+
stack = append(stack, "ring.BeforeProcess")
215209
return ctx, nil
216210
},
217211
afterProcess: func(ctx context.Context, cmd redis.Cmder) error {
218212
Expect(cmd.String()).To(Equal("ping: PONG"))
219-
stack = append(stack, "shard.AfterProcess")
213+
stack = append(stack, "ring.AfterProcess")
220214
return nil
221215
},
222216
})
223-
return nil
224-
})
225217

226-
err = ring.Ping(ctx).Err()
227-
Expect(err).NotTo(HaveOccurred())
228-
Expect(stack).To(Equal([]string{
229-
"ring.BeforeProcess",
230-
"shard.BeforeProcess",
231-
"shard.AfterProcess",
232-
"ring.AfterProcess",
233-
}))
234-
})
218+
ring.ForEachShard(ctx, func(ctx context.Context, shard *redis.Client) error {
219+
shard.AddHook(&hook{
220+
beforeProcess: func(ctx context.Context, cmd redis.Cmder) (context.Context, error) {
221+
Expect(cmd.String()).To(Equal("ping: "))
222+
stack = append(stack, "shard.BeforeProcess")
223+
return ctx, nil
224+
},
225+
afterProcess: func(ctx context.Context, cmd redis.Cmder) error {
226+
Expect(cmd.String()).To(Equal("ping: PONG"))
227+
stack = append(stack, "shard.AfterProcess")
228+
return nil
229+
},
230+
})
231+
return nil
232+
})
235233

236-
It("supports Pipeline hook", func() {
237-
err := ring.Ping(ctx).Err()
238-
Expect(err).NotTo(HaveOccurred())
234+
err = ring.Ping(ctx).Err()
235+
Expect(err).NotTo(HaveOccurred())
236+
Expect(stack).To(Equal([]string{
237+
"ring.BeforeProcess",
238+
"shard.BeforeProcess",
239+
"shard.AfterProcess",
240+
"ring.AfterProcess",
241+
}))
242+
})
239243

240-
var stack []string
244+
It("supports Pipeline hook", func() {
245+
err := ring.Ping(ctx).Err()
246+
Expect(err).NotTo(HaveOccurred())
241247

242-
ring.AddHook(&hook{
243-
beforeProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) (context.Context, error) {
244-
Expect(cmds).To(HaveLen(1))
245-
Expect(cmds[0].String()).To(Equal("ping: "))
246-
stack = append(stack, "ring.BeforeProcessPipeline")
247-
return ctx, nil
248-
},
249-
afterProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) error {
250-
Expect(cmds).To(HaveLen(1))
251-
Expect(cmds[0].String()).To(Equal("ping: PONG"))
252-
stack = append(stack, "ring.AfterProcessPipeline")
253-
return nil
254-
},
255-
})
248+
var stack []string
256249

257-
ring.ForEachShard(ctx, func(ctx context.Context, shard *redis.Client) error {
258-
shard.AddHook(&hook{
250+
ring.AddHook(&hook{
259251
beforeProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) (context.Context, error) {
260252
Expect(cmds).To(HaveLen(1))
261253
Expect(cmds[0].String()).To(Equal("ping: "))
262-
stack = append(stack, "shard.BeforeProcessPipeline")
254+
stack = append(stack, "ring.BeforeProcessPipeline")
263255
return ctx, nil
264256
},
265257
afterProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) error {
266258
Expect(cmds).To(HaveLen(1))
267259
Expect(cmds[0].String()).To(Equal("ping: PONG"))
268-
stack = append(stack, "shard.AfterProcessPipeline")
260+
stack = append(stack, "ring.AfterProcessPipeline")
269261
return nil
270262
},
271263
})
272-
return nil
273-
})
274-
275-
_, err = ring.Pipelined(ctx, func(pipe redis.Pipeliner) error {
276-
pipe.Ping(ctx)
277-
return nil
278-
})
279-
Expect(err).NotTo(HaveOccurred())
280-
Expect(stack).To(Equal([]string{
281-
"ring.BeforeProcessPipeline",
282-
"shard.BeforeProcessPipeline",
283-
"shard.AfterProcessPipeline",
284-
"ring.AfterProcessPipeline",
285-
}))
286-
})
287264

288-
It("supports TxPipeline hook", func() {
289-
err := ring.Ping(ctx).Err()
290-
Expect(err).NotTo(HaveOccurred())
291-
292-
var stack []string
265+
ring.ForEachShard(ctx, func(ctx context.Context, shard *redis.Client) error {
266+
shard.AddHook(&hook{
267+
beforeProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) (context.Context, error) {
268+
Expect(cmds).To(HaveLen(1))
269+
Expect(cmds[0].String()).To(Equal("ping: "))
270+
stack = append(stack, "shard.BeforeProcessPipeline")
271+
return ctx, nil
272+
},
273+
afterProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) error {
274+
Expect(cmds).To(HaveLen(1))
275+
Expect(cmds[0].String()).To(Equal("ping: PONG"))
276+
stack = append(stack, "shard.AfterProcessPipeline")
277+
return nil
278+
},
279+
})
280+
return nil
281+
})
293282

294-
ring.AddHook(&hook{
295-
beforeProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) (context.Context, error) {
296-
Expect(cmds).To(HaveLen(1))
297-
Expect(cmds[0].String()).To(Equal("ping: "))
298-
stack = append(stack, "ring.BeforeProcessPipeline")
299-
return ctx, nil
300-
},
301-
afterProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) error {
302-
Expect(cmds).To(HaveLen(1))
303-
Expect(cmds[0].String()).To(Equal("ping: PONG"))
304-
stack = append(stack, "ring.AfterProcessPipeline")
283+
_, err = ring.Pipelined(ctx, func(pipe redis.Pipeliner) error {
284+
pipe.Ping(ctx)
305285
return nil
306-
},
286+
})
287+
Expect(err).NotTo(HaveOccurred())
288+
Expect(stack).To(Equal([]string{
289+
"ring.BeforeProcessPipeline",
290+
"shard.BeforeProcessPipeline",
291+
"shard.AfterProcessPipeline",
292+
"ring.AfterProcessPipeline",
293+
}))
307294
})
308295

309-
ring.ForEachShard(ctx, func(ctx context.Context, shard *redis.Client) error {
310-
shard.AddHook(&hook{
296+
It("supports TxPipeline hook", func() {
297+
err := ring.Ping(ctx).Err()
298+
Expect(err).NotTo(HaveOccurred())
299+
300+
var stack []string
301+
302+
ring.AddHook(&hook{
311303
beforeProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) (context.Context, error) {
312-
Expect(cmds).To(HaveLen(3))
313-
Expect(cmds[1].String()).To(Equal("ping: "))
314-
stack = append(stack, "shard.BeforeProcessPipeline")
304+
Expect(cmds).To(HaveLen(1))
305+
Expect(cmds[0].String()).To(Equal("ping: "))
306+
stack = append(stack, "ring.BeforeProcessPipeline")
315307
return ctx, nil
316308
},
317309
afterProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) error {
318-
Expect(cmds).To(HaveLen(3))
319-
Expect(cmds[1].String()).To(Equal("ping: PONG"))
320-
stack = append(stack, "shard.AfterProcessPipeline")
310+
Expect(cmds).To(HaveLen(1))
311+
Expect(cmds[0].String()).To(Equal("ping: PONG"))
312+
stack = append(stack, "ring.AfterProcessPipeline")
321313
return nil
322314
},
323315
})
324-
return nil
325-
})
326316

327-
_, err = ring.TxPipelined(ctx, func(pipe redis.Pipeliner) error {
328-
pipe.Ping(ctx)
329-
return nil
317+
ring.ForEachShard(ctx, func(ctx context.Context, shard *redis.Client) error {
318+
shard.AddHook(&hook{
319+
beforeProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) (context.Context, error) {
320+
Expect(cmds).To(HaveLen(3))
321+
Expect(cmds[1].String()).To(Equal("ping: "))
322+
stack = append(stack, "shard.BeforeProcessPipeline")
323+
return ctx, nil
324+
},
325+
afterProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) error {
326+
Expect(cmds).To(HaveLen(3))
327+
Expect(cmds[1].String()).To(Equal("ping: PONG"))
328+
stack = append(stack, "shard.AfterProcessPipeline")
329+
return nil
330+
},
331+
})
332+
return nil
333+
})
334+
335+
_, err = ring.TxPipelined(ctx, func(pipe redis.Pipeliner) error {
336+
pipe.Ping(ctx)
337+
return nil
338+
})
339+
Expect(err).NotTo(HaveOccurred())
340+
Expect(stack).To(Equal([]string{
341+
"ring.BeforeProcessPipeline",
342+
"shard.BeforeProcessPipeline",
343+
"shard.AfterProcessPipeline",
344+
"ring.AfterProcessPipeline",
345+
}))
330346
})
331-
Expect(err).NotTo(HaveOccurred())
332-
Expect(stack).To(Equal([]string{
333-
"ring.BeforeProcessPipeline",
334-
"shard.BeforeProcessPipeline",
335-
"shard.AfterProcessPipeline",
336-
"ring.AfterProcessPipeline",
337-
}))
338347
})
339348
})
340349

0 commit comments

Comments
 (0)