Skip to content

Commit cc58718

Browse files
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 34fdeeb commit cc58718

File tree

3 files changed

+38
-13
lines changed

3 files changed

+38
-13
lines changed

host_source.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ func (h *HostInfo) Equal(host *HostInfo) bool {
141141
return true
142142
}
143143

144-
return h.ConnectAddress().Equal(host.ConnectAddress())
144+
return h.HostID() == host.HostID() && h.ConnectAddressAndPort() == host.ConnectAddressAndPort()
145145
}
146146

147147
func (h *HostInfo) Peer() net.IP {
@@ -402,10 +402,10 @@ func (h *HostInfo) HostnameAndPort() string {
402402
}
403403

404404
func (h *HostInfo) ConnectAddressAndPort() string {
405-
h.mu.Lock()
406-
defer h.mu.Unlock()
407-
addr, _ := h.connectAddressLocked()
408-
return net.JoinHostPort(addr.String(), strconv.Itoa(h.port))
405+
h.mu.Lock()
406+
defer h.mu.Unlock()
407+
addr, _ := h.connectAddressLocked()
408+
return net.JoinHostPort(addr.String(), strconv.Itoa(h.port))
409409
}
410410

411411
func (h *HostInfo) String() string {

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
@@ -333,7 +332,7 @@ func (r *roundRobinHostPolicy) AddHost(host *HostInfo) {
333332
}
334333

335334
func (r *roundRobinHostPolicy) RemoveHost(host *HostInfo) {
336-
r.hosts.remove(host.ConnectAddress())
335+
r.hosts.remove(host)
337336
}
338337

339338
func (r *roundRobinHostPolicy) HostUp(host *HostInfo) {
@@ -499,7 +498,7 @@ func (t *tokenAwareHostPolicy) AddHosts(hosts []*HostInfo) {
499498

500499
func (t *tokenAwareHostPolicy) RemoveHost(host *HostInfo) {
501500
t.mu.Lock()
502-
if t.hosts.remove(host.ConnectAddress()) {
501+
if t.hosts.remove(host) {
503502
meta := t.getMetadataForUpdate()
504503
meta.resetTokenRing(t.partitioner, t.hosts.get(), t.logger)
505504
t.updateReplicas(meta, t.getKeyspaceName())
@@ -843,9 +842,9 @@ func (d *dcAwareRR) AddHost(host *HostInfo) {
843842

844843
func (d *dcAwareRR) RemoveHost(host *HostInfo) {
845844
if d.IsLocal(host) {
846-
d.localHosts.remove(host.ConnectAddress())
845+
d.localHosts.remove(host)
847846
} else {
848-
d.remoteHosts.remove(host.ConnectAddress())
847+
d.remoteHosts.remove(host)
849848
}
850849
}
851850

@@ -950,7 +949,7 @@ func (d *rackAwareRR) AddHost(host *HostInfo) {
950949

951950
func (d *rackAwareRR) RemoveHost(host *HostInfo) {
952951
dist := d.HostTier(host)
953-
d.hosts[dist].remove(host.ConnectAddress())
952+
d.hosts[dist].remove(host)
954953
}
955954

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

policies_test.go

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

46+
func TestRoundRobbinSameConnectAddress(t *testing.T) {
47+
policy := RoundRobinHostPolicy()
48+
49+
hosts := [...]*HostInfo{
50+
{hostId: "0", connectAddress: net.IPv4(0, 0, 0, 1), port: 9042},
51+
{hostId: "1", connectAddress: net.IPv4(0, 0, 0, 1), port: 9043},
52+
}
53+
54+
for _, host := range hosts {
55+
policy.AddHost(host)
56+
}
57+
58+
got := make(map[string]bool)
59+
it := policy.Pick(nil)
60+
for h := it(); h != nil; h = it() {
61+
id := h.Info().hostId
62+
if got[id] {
63+
t.Fatalf("got duplicate host: %v", id)
64+
}
65+
got[id] = true
66+
}
67+
if len(got) != len(hosts) {
68+
t.Fatalf("expected %d hosts got %d", len(hosts), len(got))
69+
}
70+
}
71+
4672
// Tests of the token-aware host selection policy implementation with a
4773
// round-robin host selection policy fallback.
4874
func TestHostPolicy_TokenAware_SimpleStrategy(t *testing.T) {

0 commit comments

Comments
 (0)