From 783725f3f7a274e14821dbdaaee06722cd855f5b Mon Sep 17 00:00:00 2001 From: monkey Date: Sun, 7 Mar 2021 17:10:19 +0800 Subject: [PATCH 1/2] fix ring test `supports Process hook` data race Signed-off-by: monkey --- ring_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ring_test.go b/ring_test.go index 6ec1ff84..f950f617 100644 --- a/ring_test.go +++ b/ring_test.go @@ -189,6 +189,12 @@ var _ = Describe("Redis Ring", func() { }) It("supports Process hook", func() { + //the health check leads to data race for variable "stack []string". + //here, the health check time is set to 72 hours to avoid health check + opt := redisRingOptions() + opt.HeartbeatFrequency = 72 * time.Hour + ring = redis.NewRing(opt) + err := ring.Ping(ctx).Err() Expect(err).NotTo(HaveOccurred()) From 4a7d03359b2c857d396384503c191c7a43455ac0 Mon Sep 17 00:00:00 2001 From: monkey Date: Sun, 7 Mar 2021 17:34:40 +0800 Subject: [PATCH 2/2] add miss Pipeline and TxPipeline Signed-off-by: monkey --- ring_test.go | 231 ++++++++++++++++++++++++++------------------------- 1 file changed, 117 insertions(+), 114 deletions(-) diff --git a/ring_test.go b/ring_test.go index f950f617..2189cd63 100644 --- a/ring_test.go +++ b/ring_test.go @@ -188,159 +188,162 @@ var _ = Describe("Redis Ring", func() { }) }) - It("supports Process hook", func() { - //the health check leads to data race for variable "stack []string". - //here, the health check time is set to 72 hours to avoid health check - opt := redisRingOptions() - opt.HeartbeatFrequency = 72 * time.Hour - ring = redis.NewRing(opt) - - err := ring.Ping(ctx).Err() - Expect(err).NotTo(HaveOccurred()) - - var stack []string - - ring.AddHook(&hook{ - beforeProcess: func(ctx context.Context, cmd redis.Cmder) (context.Context, error) { - Expect(cmd.String()).To(Equal("ping: ")) - stack = append(stack, "ring.BeforeProcess") - return ctx, nil - }, - afterProcess: func(ctx context.Context, cmd redis.Cmder) error { - Expect(cmd.String()).To(Equal("ping: PONG")) - stack = append(stack, "ring.AfterProcess") - return nil - }, + Describe("Process hook", func() { + BeforeEach(func() { + //the health check leads to data race for variable "stack []string". + //here, the health check time is set to 72 hours to avoid health check + opt := redisRingOptions() + opt.HeartbeatFrequency = 72 * time.Hour + ring = redis.NewRing(opt) }) + It("supports Process hook", func() { + err := ring.Ping(ctx).Err() + Expect(err).NotTo(HaveOccurred()) - ring.ForEachShard(ctx, func(ctx context.Context, shard *redis.Client) error { - shard.AddHook(&hook{ + var stack []string + + ring.AddHook(&hook{ beforeProcess: func(ctx context.Context, cmd redis.Cmder) (context.Context, error) { Expect(cmd.String()).To(Equal("ping: ")) - stack = append(stack, "shard.BeforeProcess") + stack = append(stack, "ring.BeforeProcess") return ctx, nil }, afterProcess: func(ctx context.Context, cmd redis.Cmder) error { Expect(cmd.String()).To(Equal("ping: PONG")) - stack = append(stack, "shard.AfterProcess") + stack = append(stack, "ring.AfterProcess") return nil }, }) - return nil - }) - err = ring.Ping(ctx).Err() - Expect(err).NotTo(HaveOccurred()) - Expect(stack).To(Equal([]string{ - "ring.BeforeProcess", - "shard.BeforeProcess", - "shard.AfterProcess", - "ring.AfterProcess", - })) - }) - - It("supports Pipeline hook", func() { - err := ring.Ping(ctx).Err() - Expect(err).NotTo(HaveOccurred()) - - var stack []string - - ring.AddHook(&hook{ - beforeProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) (context.Context, error) { - Expect(cmds).To(HaveLen(1)) - Expect(cmds[0].String()).To(Equal("ping: ")) - stack = append(stack, "ring.BeforeProcessPipeline") - return ctx, nil - }, - afterProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) error { - Expect(cmds).To(HaveLen(1)) - Expect(cmds[0].String()).To(Equal("ping: PONG")) - stack = append(stack, "ring.AfterProcessPipeline") + ring.ForEachShard(ctx, func(ctx context.Context, shard *redis.Client) error { + shard.AddHook(&hook{ + beforeProcess: func(ctx context.Context, cmd redis.Cmder) (context.Context, error) { + Expect(cmd.String()).To(Equal("ping: ")) + stack = append(stack, "shard.BeforeProcess") + return ctx, nil + }, + afterProcess: func(ctx context.Context, cmd redis.Cmder) error { + Expect(cmd.String()).To(Equal("ping: PONG")) + stack = append(stack, "shard.AfterProcess") + return nil + }, + }) return nil - }, + }) + + err = ring.Ping(ctx).Err() + Expect(err).NotTo(HaveOccurred()) + Expect(stack).To(Equal([]string{ + "ring.BeforeProcess", + "shard.BeforeProcess", + "shard.AfterProcess", + "ring.AfterProcess", + })) }) - ring.ForEachShard(ctx, func(ctx context.Context, shard *redis.Client) error { - shard.AddHook(&hook{ + It("supports Pipeline hook", func() { + err := ring.Ping(ctx).Err() + Expect(err).NotTo(HaveOccurred()) + + var stack []string + + ring.AddHook(&hook{ beforeProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) (context.Context, error) { Expect(cmds).To(HaveLen(1)) Expect(cmds[0].String()).To(Equal("ping: ")) - stack = append(stack, "shard.BeforeProcessPipeline") + stack = append(stack, "ring.BeforeProcessPipeline") return ctx, nil }, afterProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) error { Expect(cmds).To(HaveLen(1)) Expect(cmds[0].String()).To(Equal("ping: PONG")) - stack = append(stack, "shard.AfterProcessPipeline") + stack = append(stack, "ring.AfterProcessPipeline") return nil }, }) - return nil - }) - _, err = ring.Pipelined(ctx, func(pipe redis.Pipeliner) error { - pipe.Ping(ctx) - return nil - }) - Expect(err).NotTo(HaveOccurred()) - Expect(stack).To(Equal([]string{ - "ring.BeforeProcessPipeline", - "shard.BeforeProcessPipeline", - "shard.AfterProcessPipeline", - "ring.AfterProcessPipeline", - })) - }) - - It("supports TxPipeline hook", func() { - err := ring.Ping(ctx).Err() - Expect(err).NotTo(HaveOccurred()) - - var stack []string - - ring.AddHook(&hook{ - beforeProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) (context.Context, error) { - Expect(cmds).To(HaveLen(1)) - Expect(cmds[0].String()).To(Equal("ping: ")) - stack = append(stack, "ring.BeforeProcessPipeline") - return ctx, nil - }, - afterProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) error { - Expect(cmds).To(HaveLen(1)) - Expect(cmds[0].String()).To(Equal("ping: PONG")) - stack = append(stack, "ring.AfterProcessPipeline") + ring.ForEachShard(ctx, func(ctx context.Context, shard *redis.Client) error { + shard.AddHook(&hook{ + beforeProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) (context.Context, error) { + Expect(cmds).To(HaveLen(1)) + Expect(cmds[0].String()).To(Equal("ping: ")) + stack = append(stack, "shard.BeforeProcessPipeline") + return ctx, nil + }, + afterProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) error { + Expect(cmds).To(HaveLen(1)) + Expect(cmds[0].String()).To(Equal("ping: PONG")) + stack = append(stack, "shard.AfterProcessPipeline") + return nil + }, + }) return nil - }, + }) + + _, err = ring.Pipelined(ctx, func(pipe redis.Pipeliner) error { + pipe.Ping(ctx) + return nil + }) + Expect(err).NotTo(HaveOccurred()) + Expect(stack).To(Equal([]string{ + "ring.BeforeProcessPipeline", + "shard.BeforeProcessPipeline", + "shard.AfterProcessPipeline", + "ring.AfterProcessPipeline", + })) }) - ring.ForEachShard(ctx, func(ctx context.Context, shard *redis.Client) error { - shard.AddHook(&hook{ + It("supports TxPipeline hook", func() { + err := ring.Ping(ctx).Err() + Expect(err).NotTo(HaveOccurred()) + + var stack []string + + ring.AddHook(&hook{ beforeProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) (context.Context, error) { - Expect(cmds).To(HaveLen(3)) - Expect(cmds[1].String()).To(Equal("ping: ")) - stack = append(stack, "shard.BeforeProcessPipeline") + Expect(cmds).To(HaveLen(1)) + Expect(cmds[0].String()).To(Equal("ping: ")) + stack = append(stack, "ring.BeforeProcessPipeline") return ctx, nil }, afterProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) error { - Expect(cmds).To(HaveLen(3)) - Expect(cmds[1].String()).To(Equal("ping: PONG")) - stack = append(stack, "shard.AfterProcessPipeline") + Expect(cmds).To(HaveLen(1)) + Expect(cmds[0].String()).To(Equal("ping: PONG")) + stack = append(stack, "ring.AfterProcessPipeline") return nil }, }) - return nil - }) - _, err = ring.TxPipelined(ctx, func(pipe redis.Pipeliner) error { - pipe.Ping(ctx) - return nil + ring.ForEachShard(ctx, func(ctx context.Context, shard *redis.Client) error { + shard.AddHook(&hook{ + beforeProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) (context.Context, error) { + Expect(cmds).To(HaveLen(3)) + Expect(cmds[1].String()).To(Equal("ping: ")) + stack = append(stack, "shard.BeforeProcessPipeline") + return ctx, nil + }, + afterProcessPipeline: func(ctx context.Context, cmds []redis.Cmder) error { + Expect(cmds).To(HaveLen(3)) + Expect(cmds[1].String()).To(Equal("ping: PONG")) + stack = append(stack, "shard.AfterProcessPipeline") + return nil + }, + }) + return nil + }) + + _, err = ring.TxPipelined(ctx, func(pipe redis.Pipeliner) error { + pipe.Ping(ctx) + return nil + }) + Expect(err).NotTo(HaveOccurred()) + Expect(stack).To(Equal([]string{ + "ring.BeforeProcessPipeline", + "shard.BeforeProcessPipeline", + "shard.AfterProcessPipeline", + "ring.AfterProcessPipeline", + })) }) - Expect(err).NotTo(HaveOccurred()) - Expect(stack).To(Equal([]string{ - "ring.BeforeProcessPipeline", - "shard.BeforeProcessPipeline", - "shard.AfterProcessPipeline", - "ring.AfterProcessPipeline", - })) }) })