From d39c15881a2ea5a11893ab4664e384d4a3f2d4ed Mon Sep 17 00:00:00 2001 From: fukua95 Date: Sun, 11 May 2025 20:41:15 +0800 Subject: [PATCH 1/4] optime: reduce unnecessary copy operations --- ring.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/ring.go b/ring.go index 555ea2a16..6c6dc426d 100644 --- a/ring.go +++ b/ring.go @@ -350,16 +350,13 @@ func (c *ringSharding) newRingShards( } func (c *ringSharding) List() []*ringShard { - var list []*ringShard - c.mu.RLock() - if !c.closed { - list = make([]*ringShard, len(c.shards.list)) - copy(list, c.shards.list) - } - c.mu.RUnlock() + defer c.mu.RUnlock() - return list + if c.closed { + return nil + } + return c.shards.list } func (c *ringSharding) Hash(key string) string { @@ -810,7 +807,7 @@ func (c *Ring) Watch(ctx context.Context, fn func(*Tx) error, keys ...string) er for _, key := range keys { if key != "" { - shard, err := c.sharding.GetByKey(hashtag.Key(key)) + shard, err := c.sharding.GetByKey(key) if err != nil { return err } From 944b067e146ba6e1c17939c8d37adca6f318f99b Mon Sep 17 00:00:00 2001 From: fukua95 Date: Thu, 15 May 2025 21:06:43 +0800 Subject: [PATCH 2/4] add a comment --- ring.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ring.go b/ring.go index 6c6dc426d..608f288f4 100644 --- a/ring.go +++ b/ring.go @@ -349,6 +349,8 @@ func (c *ringSharding) newRingShards( return } +// Warning: External exposure of `c.shards.list` may cause data races. +// So keep internal or implement deep copy if exposed. func (c *ringSharding) List() []*ringShard { c.mu.RLock() defer c.mu.RUnlock() From 1c02c68ab5ec334bcaecb3b632834ef450abbc4f Mon Sep 17 00:00:00 2001 From: fukua95 Date: Thu, 15 May 2025 21:18:50 +0800 Subject: [PATCH 3/4] trigger CI without code changes, because the bug of docker From 6c3765b378e9e77ccb065fe0dd977cbdcf58bd74 Mon Sep 17 00:00:00 2001 From: fukua95 Date: Thu, 15 May 2025 23:06:07 +0800 Subject: [PATCH 4/4] add comments --- ring.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ring.go b/ring.go index 608f288f4..ab3d06266 100644 --- a/ring.go +++ b/ring.go @@ -422,6 +422,7 @@ func (c *ringSharding) Heartbeat(ctx context.Context, frequency time.Duration) { case <-ticker.C: var rebalance bool + // note: `c.List()` return a shadow copy of `[]*ringShard`. for _, shard := range c.List() { err := shard.Client.Ping(ctx).Err() isUp := err == nil || err == pool.ErrPoolTimeout @@ -581,6 +582,7 @@ func (c *Ring) retryBackoff(attempt int) time.Duration { // PoolStats returns accumulated connection pool stats. func (c *Ring) PoolStats() *PoolStats { + // note: `c.List()` return a shadow copy of `[]*ringShard`. shards := c.sharding.List() var acc PoolStats for _, shard := range shards { @@ -650,6 +652,7 @@ func (c *Ring) ForEachShard( ctx context.Context, fn func(ctx context.Context, client *Client) error, ) error { + // note: `c.List()` return a shadow copy of `[]*ringShard`. shards := c.sharding.List() var wg sync.WaitGroup errCh := make(chan error, 1) @@ -681,6 +684,7 @@ func (c *Ring) ForEachShard( } func (c *Ring) cmdsInfo(ctx context.Context) (map[string]*CommandInfo, error) { + // note: `c.List()` return a shadow copy of `[]*ringShard`. shards := c.sharding.List() var firstErr error for _, shard := range shards {