Skip to content

Commit 36558c9

Browse files
committed
rename GetShardClients and GetShardClientForKey
1 parent 7e4329a commit 36558c9

File tree

2 files changed

+23
-11
lines changed

2 files changed

+23
-11
lines changed

ring.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -848,7 +848,9 @@ func (c *Ring) Close() error {
848848
return c.sharding.Close()
849849
}
850850

851-
func (c *Ring) GetShards() []*Client {
851+
// GetShardClients returns a list of all shard clients in the ring.
852+
// This can be used to create dedicated connections (e.g., PubSub) for each shard.
853+
func (c *Ring) GetShardClients() []*Client {
852854
shards := c.sharding.List()
853855
clients := make([]*Client, 0, len(shards))
854856
for _, shard := range shards {
@@ -859,7 +861,9 @@ func (c *Ring) GetShards() []*Client {
859861
return clients
860862
}
861863

862-
func (c *Ring) GetShardByKey(key string) (*Client, error) {
864+
// GetShardClientForKey returns the shard client that would handle the given key.
865+
// This can be used to determine which shard a particular key/channel would be routed to.
866+
func (c *Ring) GetShardClientForKey(key string) (*Client, error) {
863867
shard, err := c.sharding.GetByKey(key)
864868
if err != nil {
865869
return nil, err

ring_test.go

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,7 @@ var _ = Describe("Ring Tx timeout", func() {
783783
})
784784
})
785785

786-
var _ = Describe("Ring GetShards and GetShardByKey", func() {
786+
var _ = Describe("Ring GetShardClients and GetShardClientForKey", func() {
787787
var ring *redis.Ring
788788

789789
BeforeEach(func() {
@@ -799,8 +799,12 @@ var _ = Describe("Ring GetShards and GetShardByKey", func() {
799799
Expect(ring.Close()).NotTo(HaveOccurred())
800800
})
801801

802-
It("GetShards returns active shard clients", func() {
803-
shards := ring.GetShards()
802+
It("GetShardClients returns active shard clients", func() {
803+
shards := ring.GetShardClients()
804+
// Note: This test will pass even if Redis servers are not running,
805+
// because GetShardClients only returns clients that are marked as "up",
806+
// and newly created shards start as "up" until the first health check fails.
807+
804808
if len(shards) == 0 {
805809
// Expected if Redis servers are not running
806810
Skip("No active shards found (Redis servers not running)")
@@ -812,22 +816,24 @@ var _ = Describe("Ring GetShards and GetShardByKey", func() {
812816
}
813817
})
814818

815-
It("GetShardByKey returns correct shard for keys", func() {
819+
It("GetShardClientForKey returns correct shard for keys", func() {
816820
testKeys := []string{"key1", "key2", "user:123", "channel:test"}
817821

818822
for _, key := range testKeys {
819-
client, err := ring.GetShardByKey(key)
823+
client, err := ring.GetShardClientForKey(key)
820824
Expect(err).NotTo(HaveOccurred())
821825
Expect(client).NotTo(BeNil())
822826
}
823827
})
824828

825-
It("GetShardByKey is consistent for same key", func() {
829+
It("GetShardClientForKey is consistent for same key", func() {
826830
key := "test:consistency"
827831

832+
// Call GetShardClientForKey multiple times with the same key
833+
// Should always return the same shard
828834
var firstClient *redis.Client
829835
for i := 0; i < 5; i++ {
830-
client, err := ring.GetShardByKey(key)
836+
client, err := ring.GetShardClientForKey(key)
831837
Expect(err).NotTo(HaveOccurred())
832838
Expect(client).NotTo(BeNil())
833839

@@ -839,17 +845,19 @@ var _ = Describe("Ring GetShards and GetShardByKey", func() {
839845
}
840846
})
841847

842-
It("GetShardByKey distributes keys across shards", func() {
848+
It("GetShardClientForKey distributes keys across shards", func() {
843849
testKeys := []string{"key1", "key2", "key3", "key4", "key5"}
844850
shardMap := make(map[string]int)
845851

846852
for _, key := range testKeys {
847-
client, err := ring.GetShardByKey(key)
853+
client, err := ring.GetShardClientForKey(key)
848854
Expect(err).NotTo(HaveOccurred())
849855
shardMap[client.String()]++
850856
}
851857

858+
// Should have at least 1 shard (could be all keys go to same shard due to hashing)
852859
Expect(len(shardMap)).To(BeNumerically(">=", 1))
860+
// But with multiple keys, we expect some distribution
853861
Expect(len(shardMap)).To(BeNumerically("<=", 2)) // At most 2 shards (our setup)
854862
})
855863
})

0 commit comments

Comments
 (0)