Skip to content

Commit 3642984

Browse files
dkropachevdkropachev
committed
Fix cowHostList can't have hosts with same ConnectAddress
cowHostList uses HostInfo.Equal to confirm host uniqueness, which relies on `ConnectAddress.Equal`, which does not allow to have different hosts with same `ConnectAddress`
1 parent bef4e8a commit 3642984

File tree

3 files changed

+40
-15
lines changed

3 files changed

+40
-15
lines changed

‎host_source.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ func (h *HostInfo) Equal(host *HostInfo) bool {
145145
return true
146146
}
147147

148-
return h.ConnectAddress().Equal(host.ConnectAddress())
148+
return h.HostID() == host.HostID() && h.ConnectAddressAndPort() == host.ConnectAddressAndPort()
149149
}
150150

151151
func (h *HostInfo) Peer() net.IP {

‎policies.go

+7-8
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"fmt"
1313
"math"
1414
"math/rand"
15-
"net"
1615
"sync"
1716
"sync/atomic"
1817
"time"
@@ -64,7 +63,7 @@ func (c *cowHostList) add(host *HostInfo) bool {
6463
return true
6564
}
6665

67-
func (c *cowHostList) remove(ip net.IP) bool {
66+
func (c *cowHostList) remove(host *HostInfo) bool {
6867
c.mu.Lock()
6968
l := c.get()
7069
size := len(l)
@@ -76,7 +75,7 @@ func (c *cowHostList) remove(ip net.IP) bool {
7675
found := false
7776
newL := make([]*HostInfo, 0, size)
7877
for i := 0; i < len(l); i++ {
79-
if !l[i].ConnectAddress().Equal(ip) {
78+
if !l[i].Equal(host) {
8079
newL = append(newL, l[i])
8180
} else {
8281
found = true
@@ -374,7 +373,7 @@ func (r *roundRobinHostPolicy) AddHost(host *HostInfo) {
374373
}
375374

376375
func (r *roundRobinHostPolicy) RemoveHost(host *HostInfo) {
377-
r.hosts.remove(host.ConnectAddress())
376+
r.hosts.remove(host)
378377
}
379378

380379
func (r *roundRobinHostPolicy) HostUp(host *HostInfo) {
@@ -566,7 +565,7 @@ func (t *tokenAwareHostPolicy) AddHosts(hosts []*HostInfo) {
566565

567566
func (t *tokenAwareHostPolicy) RemoveHost(host *HostInfo) {
568567
t.mu.Lock()
569-
if t.hosts.remove(host.ConnectAddress()) {
568+
if t.hosts.remove(host) {
570569
meta := t.getMetadataForUpdate()
571570
meta.resetTokenRing(t.partitioner, t.hosts.get(), t.logger)
572571
t.updateReplicas(meta, t.getKeyspaceName())
@@ -981,9 +980,9 @@ func (d *dcAwareRR) AddHost(host *HostInfo) {
981980

982981
func (d *dcAwareRR) RemoveHost(host *HostInfo) {
983982
if d.IsLocal(host) {
984-
d.localHosts.remove(host.ConnectAddress())
983+
d.localHosts.remove(host)
985984
} else {
986-
d.remoteHosts.remove(host.ConnectAddress())
985+
d.remoteHosts.remove(host)
987986
}
988987
}
989988

@@ -1090,7 +1089,7 @@ func (d *rackAwareRR) AddHost(host *HostInfo) {
10901089

10911090
func (d *rackAwareRR) RemoveHost(host *HostInfo) {
10921091
dist := d.HostTier(host)
1093-
d.hosts[dist].remove(host.ConnectAddress())
1092+
d.hosts[dist].remove(host)
10941093
}
10951094

10961095
func (d *rackAwareRR) HostUp(host *HostInfo) { d.AddHost(host) }

‎policies_test.go

+32-6
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,32 @@ func TestRoundRobbin(t *testing.T) {
4444
}
4545
}
4646

47+
func TestRoundRobbinSameConnectAddress(t *testing.T) {
48+
policy := RoundRobinHostPolicy()
49+
50+
hosts := [...]*HostInfo{
51+
{hostId: "0", connectAddress: net.IPv4(0, 0, 0, 1), port: 9042},
52+
{hostId: "1", connectAddress: net.IPv4(0, 0, 0, 1), port: 9043},
53+
}
54+
55+
for _, host := range hosts {
56+
policy.AddHost(host)
57+
}
58+
59+
got := make(map[string]bool)
60+
it := policy.Pick(nil)
61+
for h := it(); h != nil; h = it() {
62+
id := h.Info().hostId
63+
if got[id] {
64+
t.Fatalf("got duplicate host: %v", id)
65+
}
66+
got[id] = true
67+
}
68+
if len(got) != len(hosts) {
69+
t.Fatalf("expected %d hosts got %d", len(hosts), len(got))
70+
}
71+
}
72+
4773
// Tests of the token-aware host selection policy implementation with a
4874
// round-robin host selection policy fallback.
4975
func TestHostPolicy_TokenAware_SimpleStrategy(t *testing.T) {
@@ -132,47 +158,47 @@ func TestHostPolicy_TokenAware_LWT_DisablesHostShuffling(t *testing.T) {
132158
{hostId: "3", connectAddress: net.IPv4(10, 0, 0, 4), tokens: []string{"25", "35", "45"}},
133159
{hostId: "4", connectAddress: net.IPv4(10, 0, 0, 3), tokens: []string{"50", "60", "70"}},
134160
{hostId: "5", connectAddress: net.IPv4(10, 0, 0, 4), tokens: []string{"50", "60", "70"}},
135-
}, routingKey: "8", lwt: true, shuffle: true, want: []string{"0", "2", "3", "1"}},
161+
}, routingKey: "8", lwt: true, shuffle: true, want: []string{"0", "2", "3", "4", "5", "1"}},
136162
"token 08 shuffling not configured": {hosts: []*HostInfo{
137163
{hostId: "0", connectAddress: net.IPv4(10, 0, 0, 1), tokens: []string{"00", "10", "20"}},
138164
{hostId: "1", connectAddress: net.IPv4(10, 0, 0, 3), tokens: []string{"25", "35", "45"}},
139165
{hostId: "2", connectAddress: net.IPv4(10, 0, 0, 2), tokens: []string{"00", "10", "20"}},
140166
{hostId: "3", connectAddress: net.IPv4(10, 0, 0, 4), tokens: []string{"25", "35", "45"}},
141167
{hostId: "4", connectAddress: net.IPv4(10, 0, 0, 3), tokens: []string{"50", "60", "70"}},
142168
{hostId: "5", connectAddress: net.IPv4(10, 0, 0, 4), tokens: []string{"50", "60", "70"}},
143-
}, routingKey: "8", lwt: true, shuffle: false, want: []string{"0", "2", "3", "1"}},
169+
}, routingKey: "8", lwt: true, shuffle: false, want: []string{"0", "2", "3", "4", "5", "1"}},
144170
"token 30 shuffling configured": {hosts: []*HostInfo{
145171
{hostId: "0", connectAddress: net.IPv4(10, 0, 0, 1), tokens: []string{"00", "10", "20"}},
146172
{hostId: "1", connectAddress: net.IPv4(10, 0, 0, 3), tokens: []string{"25", "35", "45"}},
147173
{hostId: "2", connectAddress: net.IPv4(10, 0, 0, 2), tokens: []string{"00", "10", "20"}},
148174
{hostId: "3", connectAddress: net.IPv4(10, 0, 0, 4), tokens: []string{"25", "35", "45"}},
149175
{hostId: "4", connectAddress: net.IPv4(10, 0, 0, 3), tokens: []string{"50", "60", "70"}},
150176
{hostId: "5", connectAddress: net.IPv4(10, 0, 0, 4), tokens: []string{"50", "60", "70"}},
151-
}, routingKey: "30", lwt: true, shuffle: true, want: []string{"1", "3", "2", "0"}},
177+
}, routingKey: "30", lwt: true, shuffle: true, want: []string{"1", "3", "2", "4", "5", "0"}},
152178
"token 30 shuffling not configured": {hosts: []*HostInfo{
153179
{hostId: "0", connectAddress: net.IPv4(10, 0, 0, 1), tokens: []string{"00", "10", "20"}},
154180
{hostId: "1", connectAddress: net.IPv4(10, 0, 0, 3), tokens: []string{"25", "35", "45"}},
155181
{hostId: "2", connectAddress: net.IPv4(10, 0, 0, 2), tokens: []string{"00", "10", "20"}},
156182
{hostId: "3", connectAddress: net.IPv4(10, 0, 0, 4), tokens: []string{"25", "35", "45"}},
157183
{hostId: "4", connectAddress: net.IPv4(10, 0, 0, 3), tokens: []string{"50", "60", "70"}},
158184
{hostId: "5", connectAddress: net.IPv4(10, 0, 0, 4), tokens: []string{"50", "60", "70"}},
159-
}, routingKey: "30", lwt: true, shuffle: false, want: []string{"1", "3", "2", "0"}},
185+
}, routingKey: "30", lwt: true, shuffle: false, want: []string{"1", "3", "2", "4", "5", "0"}},
160186
"token 55 shuffling configured": {hosts: []*HostInfo{
161187
{hostId: "0", connectAddress: net.IPv4(10, 0, 0, 1), tokens: []string{"00", "10", "20"}},
162188
{hostId: "1", connectAddress: net.IPv4(10, 0, 0, 3), tokens: []string{"25", "35", "45"}},
163189
{hostId: "2", connectAddress: net.IPv4(10, 0, 0, 2), tokens: []string{"00", "10", "20"}},
164190
{hostId: "3", connectAddress: net.IPv4(10, 0, 0, 4), tokens: []string{"25", "35", "45"}},
165191
{hostId: "4", connectAddress: net.IPv4(10, 0, 0, 3), tokens: []string{"50", "60", "70"}},
166192
{hostId: "5", connectAddress: net.IPv4(10, 0, 0, 4), tokens: []string{"50", "60", "70"}},
167-
}, routingKey: "55", lwt: true, shuffle: true, want: []string{"0", "2", "3", "1"}},
193+
}, routingKey: "55", lwt: true, shuffle: true, want: []string{"4", "5", "2", "3", "0", "1"}},
168194
"token 55 shuffling not configured": {hosts: []*HostInfo{
169195
{hostId: "0", connectAddress: net.IPv4(10, 0, 0, 1), tokens: []string{"00", "10", "20"}},
170196
{hostId: "1", connectAddress: net.IPv4(10, 0, 0, 3), tokens: []string{"25", "35", "45"}},
171197
{hostId: "2", connectAddress: net.IPv4(10, 0, 0, 2), tokens: []string{"00", "10", "20"}},
172198
{hostId: "3", connectAddress: net.IPv4(10, 0, 0, 4), tokens: []string{"25", "35", "45"}},
173199
{hostId: "4", connectAddress: net.IPv4(10, 0, 0, 3), tokens: []string{"50", "60", "70"}},
174200
{hostId: "5", connectAddress: net.IPv4(10, 0, 0, 4), tokens: []string{"50", "60", "70"}},
175-
}, routingKey: "55", lwt: true, shuffle: false, want: []string{"0", "2", "3", "1"}},
201+
}, routingKey: "55", lwt: true, shuffle: false, want: []string{"4", "5", "2", "3", "0", "1"}},
176202
}
177203
const keyspace = "myKeyspace"
178204
for name, tc := range tests {

0 commit comments

Comments
 (0)