Skip to content

Commit 3c1267a

Browse files
committed
fix: use native transport port for broadcastRpcAddress in ClientRoutes
ClientRoutesTopologyMonitor.savePort() was saving the NLB proxy port (e.g. 29043) as the fallback for broadcastRpcAddress construction. This caused Metadata.findNode() to fail matching TOPOLOGY_CHANGE REMOVED_NODE events (which carry the real native transport port 9042), so decommissioned nodes were never removed from metadata. Add a nativeTransportPort field to ClientRoutesConfig (default 9042, configurable via advanced.client-routes.native-transport-port) and set it in the ClientRoutesTopologyMonitor constructor. The savePort() override is now a no-op since the port is already initialized. broadcastRpcAddress is used only for event matching — connections go through ClientRoutesEndPoint which resolves via NLB proxy addresses in the routes cache.
1 parent 62dc65f commit 3c1267a

8 files changed

Lines changed: 93 additions & 56 deletions

File tree

core/src/main/java/com/datastax/oss/driver/api/core/config/ClientRoutesConfig.java

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,16 @@ public final class ClientRoutesConfig {
6363

6464
private static final String DEFAULT_TABLE_NAME = "system.client_routes";
6565

66+
/**
67+
* Default native transport port used by Scylla/Cassandra nodes. This is used as the fallback port
68+
* for {@code broadcastRpcAddress} when the system tables do not include port information (e.g.
69+
* older Scylla versions without {@code system.peers_v2}). The {@code broadcastRpcAddress} is how
70+
* the driver matches TOPOLOGY_CHANGE events (node added/removed) to nodes in its metadata — it is
71+
* NOT used for opening connections. Connections go through {@code ClientRoutesEndPoint} which
72+
* resolves via the NLB proxy addresses in the routes cache.
73+
*/
74+
public static final int DEFAULT_NATIVE_TRANSPORT_PORT = 9042;
75+
6676
/**
6777
* Pattern for valid unquoted CQL table names: must start with a letter or underscore, followed by
6878
* letters, digits, or underscores. Optionally qualified with a keyspace prefix using the same
@@ -73,8 +83,10 @@ public final class ClientRoutesConfig {
7383

7484
private final List<ClientRouteProxy> endpoints;
7585
private final String tableName;
86+
private final int nativeTransportPort;
7687

77-
private ClientRoutesConfig(List<ClientRouteProxy> endpoints, String tableName) {
88+
private ClientRoutesConfig(
89+
List<ClientRouteProxy> endpoints, String tableName, int nativeTransportPort) {
7890
if (endpoints == null || endpoints.isEmpty()) {
7991
throw new IllegalArgumentException("At least one endpoint must be specified");
8092
}
@@ -86,6 +98,7 @@ private ClientRoutesConfig(List<ClientRouteProxy> endpoints, String tableName) {
8698
}
8799
this.endpoints = Collections.unmodifiableList(new ArrayList<>(endpoints));
88100
this.tableName = tableName;
101+
this.nativeTransportPort = nativeTransportPort;
89102
}
90103

91104
/**
@@ -108,6 +121,18 @@ public String getTableName() {
108121
return tableName;
109122
}
110123

124+
/**
125+
* Returns the native transport port of the cluster nodes. This port is used as the fallback for
126+
* building {@code broadcastRpcAddress} when system tables lack port columns. The {@code
127+
* broadcastRpcAddress} is used only to match TOPOLOGY_CHANGE events to metadata nodes — it is NOT
128+
* used to open connections (those go through the NLB proxy endpoints from the routes cache).
129+
*
130+
* @return the native transport port (defaults to {@value #DEFAULT_NATIVE_TRANSPORT_PORT}).
131+
*/
132+
public int getNativeTransportPort() {
133+
return nativeTransportPort;
134+
}
135+
111136
/**
112137
* Creates a new builder for constructing a {@link ClientRoutesConfig}.
113138
*
@@ -127,12 +152,14 @@ public boolean equals(Object o) {
127152
return false;
128153
}
129154
ClientRoutesConfig that = (ClientRoutesConfig) o;
130-
return endpoints.equals(that.endpoints) && tableName.equals(that.tableName);
155+
return endpoints.equals(that.endpoints)
156+
&& tableName.equals(that.tableName)
157+
&& nativeTransportPort == that.nativeTransportPort;
131158
}
132159

133160
@Override
134161
public int hashCode() {
135-
return Objects.hash(endpoints, tableName);
162+
return Objects.hash(endpoints, tableName, nativeTransportPort);
136163
}
137164

138165
@Override
@@ -143,6 +170,8 @@ public String toString() {
143170
+ ", tableName='"
144171
+ tableName
145172
+ '\''
173+
+ ", nativeTransportPort="
174+
+ nativeTransportPort
146175
+ '}';
147176
}
148177

@@ -151,6 +180,7 @@ public static final class Builder {
151180
private final List<ClientRouteProxy> endpoints = new ArrayList<>();
152181
private final Set<String> seenConnectionIds = new HashSet<>();
153182
private String tableName = DEFAULT_TABLE_NAME;
183+
private int nativeTransportPort = DEFAULT_NATIVE_TRANSPORT_PORT;
154184

155185
/**
156186
* Adds an endpoint to the configuration.
@@ -208,6 +238,21 @@ Builder withTableName(@NonNull String tableName) {
208238
return this;
209239
}
210240

241+
/**
242+
* Sets the native transport port of the cluster nodes. This is used as the fallback port for
243+
* {@code broadcastRpcAddress} when system tables do not include port information. Only needed
244+
* for clusters using a non-standard native transport port. Defaults to {@value
245+
* #DEFAULT_NATIVE_TRANSPORT_PORT}.
246+
*
247+
* @param port the native transport port.
248+
* @return this builder.
249+
*/
250+
@NonNull
251+
public Builder withNativeTransportPort(int port) {
252+
this.nativeTransportPort = port;
253+
return this;
254+
}
255+
211256
/**
212257
* Builds the {@link ClientRoutesConfig} with the configured endpoints and table name.
213258
*
@@ -216,7 +261,7 @@ Builder withTableName(@NonNull String tableName) {
216261
*/
217262
@NonNull
218263
public ClientRoutesConfig build() {
219-
return new ClientRoutesConfig(endpoints, tableName);
264+
return new ClientRoutesConfig(endpoints, tableName, nativeTransportPort);
220265
}
221266
}
222267
}

core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1097,7 +1097,17 @@ public enum DefaultDriverOption implements DriverOption {
10971097
*
10981098
* <p>Value type: list of HOCON objects
10991099
*/
1100-
CLIENT_ROUTES_ENDPOINTS("advanced.client-routes.endpoints");
1100+
CLIENT_ROUTES_ENDPOINTS("advanced.client-routes.endpoints"),
1101+
1102+
/**
1103+
* The native transport port of the cluster nodes.
1104+
*
1105+
* <p>Used as the fallback port for {@code broadcastRpcAddress} when system tables lack port
1106+
* columns. Only needed for clusters using a non-standard native transport port. Defaults to 9042.
1107+
*
1108+
* <p>Value type: {@link Integer}
1109+
*/
1110+
CLIENT_ROUTES_NATIVE_TRANSPORT_PORT("advanced.client-routes.native-transport-port");
11011111

11021112
private final String path;
11031113

core/src/main/java/com/datastax/oss/driver/api/core/config/OptionsMap.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,7 @@ protected static void fillWithDriverDefaults(OptionsMap map) {
398398
"PRESERVE_REPLICA_ORDER");
399399
// CLIENT_ROUTES_ENDPOINTS is intentionally omitted: it is a list-of-objects (compound HOCON
400400
// values) with no sensible scalar default, analogous to how CONFIG_RELOAD_INTERVAL is omitted.
401+
map.put(TypedDriverOption.CLIENT_ROUTES_NATIVE_TRANSPORT_PORT, 9042);
401402
}
402403

403404
@Immutable

core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -944,6 +944,11 @@ public String toString() {
944944
// DriverExecutionProfile API); it is excluded from the TypedDriverOptionTest consistency check
945945
// accordingly.
946946

947+
/** The native transport port of the cluster nodes, used for broadcastRpcAddress fallback. */
948+
public static final TypedDriverOption<Integer> CLIENT_ROUTES_NATIVE_TRANSPORT_PORT =
949+
new TypedDriverOption<>(
950+
DefaultDriverOption.CLIENT_ROUTES_NATIVE_TRANSPORT_PORT, GenericType.INTEGER);
951+
947952
private static Iterable<TypedDriverOption<?>> introspectBuiltInValues() {
948953
try {
949954
ImmutableList.Builder<TypedDriverOption<?>> result = ImmutableList.builder();

core/src/main/java/com/datastax/oss/driver/internal/core/context/DefaultDriverContext.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,11 @@ ClientRoutesConfig buildClientRoutesConfigFromFile() {
545545
builder.addEndpoint(new ClientRouteProxy(connectionId, connectionAddr));
546546
}
547547

548+
if (defaultProfile.isDefined(DefaultDriverOption.CLIENT_ROUTES_NATIVE_TRANSPORT_PORT)) {
549+
builder.withNativeTransportPort(
550+
defaultProfile.getInt(DefaultDriverOption.CLIENT_ROUTES_NATIVE_TRANSPORT_PORT));
551+
}
552+
548553
return builder.build();
549554
}
550555

core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesTopologyMonitor.java

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ public ClientRoutesTopologyMonitor(
130130
@NonNull InternalDriverContext context, @NonNull ClientRoutesConfig config) {
131131
super(context);
132132
this.config = config;
133+
this.port = config.getNativeTransportPort();
133134
this.configuredConnectionIds =
134135
Collections.unmodifiableList(
135136
config.getEndpoints().stream()
@@ -438,36 +439,15 @@ private void onClientRoutesUpdateEvent(ClientRoutesUpdateEvent event) {
438439
}
439440

440441
/**
441-
* Overrides the default port discovery to use the port from the client routes cache instead of
442-
* the control connection channel. When connecting through an NLB proxy, the channel port is the
443-
* proxy port, not the real server port. The {@code system.client_routes} table has the correct
444-
* port.
445-
*
446-
* <p>All nodes in a Scylla/Cassandra cluster use the same native transport port, so any route's
447-
* port is correct. We use the minimum host_id for deterministic selection.
442+
* No-op: the port is set in the constructor from {@link ClientRoutesConfig#getNativeTransportPort
443+
* ()}. The default implementation would save the control channel's endpoint port, which is the
444+
* NLB proxy port — not the real native transport port. Using the proxy port would cause {@link
445+
* #getBroadcastRpcAddress} to build incorrect {@code broadcastRpcAddress} values, preventing
446+
* {@code Metadata.findNode()} from matching TOPOLOGY_CHANGE events (which carry the real native
447+
* transport port).
448448
*/
449449
@Override
450-
protected void savePort(DriverChannel channel) {
451-
if (port < 0) {
452-
Map<UUID, ClientRouteRecord> routes = resolvedRoutesCache.get();
453-
if (!routes.isEmpty()) {
454-
// Pick the route with the smallest host_id for deterministic behavior.
455-
UUID minId = null;
456-
ClientRouteRecord chosen = null;
457-
for (Map.Entry<UUID, ClientRouteRecord> entry : routes.entrySet()) {
458-
if (minId == null || entry.getKey().compareTo(minId) < 0) {
459-
minId = entry.getKey();
460-
chosen = entry.getValue();
461-
}
462-
}
463-
if (chosen != null && chosen.getPort() > 0) {
464-
port = chosen.getPort();
465-
return;
466-
}
467-
}
468-
}
469-
super.savePort(channel);
470-
}
450+
protected void savePort(DriverChannel channel) {}
471451

472452
@NonNull
473453
@Override

core/src/main/resources/reference.conf

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,6 +1146,17 @@ datastax-java-driver {
11461146
# Required: yes (if client routes are to be used)
11471147
# endpoints = []
11481148

1149+
# The native transport port of the cluster nodes. Used as the fallback port for
1150+
# broadcastRpcAddress when system tables lack port columns. The broadcastRpcAddress is used
1151+
# only to match TOPOLOGY_CHANGE events to metadata nodes — it is NOT used for opening
1152+
# connections (those go through the NLB proxy endpoints from the routes cache).
1153+
# Only needed for clusters using a non-standard native transport port.
1154+
#
1155+
# Required: no
1156+
# Modifiable at runtime: no
1157+
# Overridable in a profile: no
1158+
native-transport-port = 9042
1159+
11491160
}
11501161

11511162
# Whether to resolve the addresses passed to `basic.contact-points`.

core/src/test/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesTopologyMonitorTest.java

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1161,38 +1161,18 @@ public void should_resolve_to_fallback_when_no_route_for_host_id() {
11611161
// ---- savePort() --------------------------------------------------------
11621162

11631163
@Test
1164-
public void savePort_should_use_route_port_when_routes_available() {
1165-
UUID id1 = UUID.randomUUID();
1166-
UUID id2 = UUID.randomUUID();
1167-
handler.setRoutes(
1168-
ImmutableMap.of(
1169-
id1, new ClientRouteRecord(id1, "127.0.0.1", 19042),
1170-
id2, new ClientRouteRecord(id2, "127.0.0.2", 19042)));
1171-
1172-
DriverChannel channel = Mockito.mock(DriverChannel.class);
1173-
handler.savePort(channel);
1174-
1175-
assertThat(handler.port).isEqualTo(19042);
1176-
}
1177-
1178-
@Test
1179-
public void savePort_should_fall_through_to_super_when_routes_empty() {
1180-
// routes cache is empty by default
1164+
public void savePort_should_use_native_transport_port_from_config() {
11811165
DriverChannel channel = Mockito.mock(DriverChannel.class);
1182-
EndPoint ep = Mockito.mock(EndPoint.class);
1183-
when(ep.resolve()).thenReturn(new InetSocketAddress("127.0.0.1", 9042));
1184-
when(channel.getEndPoint()).thenReturn(ep);
1185-
11861166
handler.savePort(channel);
11871167

1168+
// Should use the default native transport port from ClientRoutesConfig (9042),
1169+
// NOT the NLB proxy port from the routes cache or the channel endpoint.
11881170
assertThat(handler.port).isEqualTo(9042);
11891171
}
11901172

11911173
@Test
11921174
public void savePort_should_skip_when_port_already_set() {
11931175
handler.port = 12345;
1194-
UUID id = UUID.randomUUID();
1195-
handler.setRoutes(ImmutableMap.of(id, new ClientRouteRecord(id, "127.0.0.1", 19042)));
11961176

11971177
DriverChannel channel = Mockito.mock(DriverChannel.class);
11981178
handler.savePort(channel);

0 commit comments

Comments
 (0)