Skip to content

Commit 62dc65f

Browse files
committed
fix: avoid discovery proxy rebind in NlbSimulator
NlbSimulator.addNode() was tearing down and recreating the discovery RoundRobinProxy on every call, which failed with "Address already in use" because the new proxy tried to bind while the old one still held the port. Add addTarget()/removeTarget() to RoundRobinProxy so the discovery proxy is created once (on the first addNode) and updated in-place as nodes join or leave. Fixes #851
1 parent b05a55f commit 62dc65f

2 files changed

Lines changed: 28 additions & 37 deletions

File tree

integration-tests/src/test/java/com/datastax/oss/driver/core/clientroutes/NlbSimulator.java

Lines changed: 14 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -112,20 +112,16 @@ public synchronized void addNode(int nodeId) throws IOException {
112112
String nodeIp = ccmBridge.getNodeIpAddress(nodeId);
113113
InetSocketAddress nodeAddr = new InetSocketAddress(nodeIp, 9042);
114114

115-
// Create per-node proxy first, before updating state
116115
int nodePort = getNodePort(nodeId);
117116
TcpProxy proxy = new TcpProxy(bindAddress, nodePort, nodeAddr);
118117

119-
// Update state and rebuild discovery proxy; if rebuild fails, close the new proxy
120118
activeNodes.add(nodeId);
121119
nodeProxies.put(nodeId, proxy);
122-
try {
123-
rebuildDiscoveryProxy();
124-
} catch (IOException e) {
125-
activeNodes.remove(Integer.valueOf(nodeId));
126-
nodeProxies.remove(nodeId);
127-
proxy.close();
128-
throw e;
120+
121+
if (discoveryProxy == null) {
122+
discoveryProxy = buildDiscoveryProxy(nodeAddr);
123+
} else {
124+
discoveryProxy.addTarget(nodeAddr);
129125
}
130126

131127
LOG.info("NLB: added node{} ({}:{}) -> proxy port {}", nodeId, nodeIp, 9042, nodePort);
@@ -139,37 +135,20 @@ public synchronized void removeNode(int nodeId) throws IOException {
139135
}
140136
activeNodes.remove(Integer.valueOf(nodeId));
141137

142-
// Rebuild discovery proxy with updated node list
143-
rebuildDiscoveryProxy();
138+
if (discoveryProxy != null) {
139+
String nodeIp = ccmBridge.getNodeIpAddress(nodeId);
140+
discoveryProxy.removeTarget(new InetSocketAddress(nodeIp, 9042));
141+
}
144142

145143
LOG.info("NLB: removed node{}", nodeId);
146144
}
147145

148-
private void rebuildDiscoveryProxy() throws IOException {
149-
RoundRobinProxy oldProxy = discoveryProxy;
150-
151-
if (activeNodes.isEmpty()) {
152-
discoveryProxy = null;
153-
if (oldProxy != null) {
154-
oldProxy.close();
155-
}
156-
return;
157-
}
158-
159-
// Build target list from active nodes
146+
private RoundRobinProxy buildDiscoveryProxy(InetSocketAddress firstTarget) throws IOException {
160147
List<InetSocketAddress> targets = new ArrayList<>();
161-
for (int nodeId : activeNodes) {
162-
String nodeIp = ccmBridge.getNodeIpAddress(nodeId);
163-
targets.add(new InetSocketAddress(nodeIp, 9042));
164-
}
165-
166-
// Create new proxy before closing old one to avoid inconsistent state on failure
167-
discoveryProxy = new RoundRobinProxy(bindAddress, basePort, targets);
168-
if (oldProxy != null) {
169-
oldProxy.close();
170-
}
171-
LOG.info(
172-
"NLB: discovery proxy on port {} -> {} nodes: {}", basePort, targets.size(), activeNodes);
148+
targets.add(firstTarget);
149+
RoundRobinProxy proxy = new RoundRobinProxy(bindAddress, basePort, targets);
150+
LOG.info("NLB: discovery proxy on port {} -> {}", basePort, firstTarget);
151+
return proxy;
173152
}
174153

175154
@Override

integration-tests/src/test/java/com/datastax/oss/driver/core/clientroutes/RoundRobinProxy.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public class RoundRobinProxy implements Closeable {
4747
private static final int BUFFER_SIZE = 8192;
4848

4949
private final ServerSocket serverSocket;
50-
private final List<InetSocketAddress> targets;
50+
private final CopyOnWriteArrayList<InetSocketAddress> targets;
5151
private final AtomicInteger counter = new AtomicInteger(0);
5252
private final AtomicBoolean closed = new AtomicBoolean(false);
5353
private final CopyOnWriteArrayList<Socket> activeSockets = new CopyOnWriteArrayList<>();
@@ -59,7 +59,7 @@ public RoundRobinProxy(String bindAddress, int listenPort, List<InetSocketAddres
5959
if (targets.isEmpty()) {
6060
throw new IllegalArgumentException("At least one target required");
6161
}
62-
this.targets = targets;
62+
this.targets = new CopyOnWriteArrayList<>(targets);
6363
this.serverSocket = new ServerSocket();
6464
this.serverSocket.setReuseAddress(true);
6565
this.serverSocket.bind(new InetSocketAddress(bindAddress, listenPort));
@@ -75,6 +75,18 @@ public int getLocalPort() {
7575
return serverSocket.getLocalPort();
7676
}
7777

78+
/** Adds a target to the round-robin pool. */
79+
public void addTarget(InetSocketAddress target) {
80+
targets.add(target);
81+
LOG.debug("RoundRobinProxy on port {} added target: {}", serverSocket.getLocalPort(), target);
82+
}
83+
84+
/** Removes a target from the round-robin pool. */
85+
public void removeTarget(InetSocketAddress target) {
86+
targets.remove(target);
87+
LOG.debug("RoundRobinProxy on port {} removed target: {}", serverSocket.getLocalPort(), target);
88+
}
89+
7890
private void acceptLoop() {
7991
while (!closed.get()) {
8092
try {

0 commit comments

Comments
 (0)