Skip to content

Commit fdd0dd2

Browse files
nikagraclaude
andcommitted
refactor: address PR #839 review comments
- Rename proxyProtocol → nlbShardAwareness throughout (ClientRoutesConfig, DefaultDriverOption, TypedDriverOption, OptionsMap, DefaultDriverContext). The config key path (advanced.client-routes.proxy-protocol) is unchanged. - Remove PP2 integration test infrastructure (ClientRoutesIT, TcpProxy, RoundRobinProxy, NlbSimulator) per reviewer request. - Fix warning log to say "has no effect" instead of "will be ignored". - Fix reference.conf comment to match actual behaviour (logs warning, does not silently ignore). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 5e8e598 commit fdd0dd2

File tree

10 files changed

+48
-285
lines changed

10 files changed

+48
-285
lines changed

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

Lines changed: 25 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,10 @@ public final class ClientRoutesConfig {
7373

7474
private final List<ClientRouteProxy> endpoints;
7575
private final String tableName;
76-
private final boolean proxyProtocol;
76+
private final boolean nlbShardAwareness;
7777

7878
private ClientRoutesConfig(
79-
List<ClientRouteProxy> endpoints, String tableName, boolean proxyProtocol) {
79+
List<ClientRouteProxy> endpoints, String tableName, boolean nlbShardAwareness) {
8080
if (endpoints == null || endpoints.isEmpty()) {
8181
throw new IllegalArgumentException("At least one endpoint must be specified");
8282
}
@@ -88,7 +88,7 @@ private ClientRoutesConfig(
8888
}
8989
this.endpoints = Collections.unmodifiableList(new ArrayList<>(endpoints));
9090
this.tableName = tableName;
91-
this.proxyProtocol = proxyProtocol;
91+
this.nlbShardAwareness = nlbShardAwareness;
9292
}
9393

9494
/**
@@ -112,18 +112,17 @@ public String getTableName() {
112112
}
113113

114114
/**
115-
* Returns whether Proxy Protocol v2 is in use between the NLB and ScyllaDB nodes.
115+
* Returns whether NLB shard awareness is enabled.
116116
*
117-
* <p>When {@code true}, the driver assumes the NLB prepends a PP2 binary header to each
118-
* connection it opens to ScyllaDB. The header carries the original client IP and source port,
119-
* which ScyllaDB uses for shard-aware routing. This restores shard-awareness end-to-end through
120-
* the NLB. Requires both the NLB and ScyllaDB to be configured for Proxy Protocol v2, and {@code
121-
* advanced-shard-awareness.enabled} must be {@code true} (the default).
117+
* <p>When {@code true}, the driver assumes the NLB is configured to forward the driver's original
118+
* source port to ScyllaDB (e.g. via Proxy Protocol v2), enabling shard-aware connection routing
119+
* end-to-end through the NLB. Requires {@code advanced-shard-awareness.enabled} to be {@code
120+
* true} (the default).
122121
*
123-
* @return {@code true} if Proxy Protocol v2 is enabled.
122+
* @return {@code true} if NLB shard awareness is enabled.
124123
*/
125-
public boolean isProxyProtocol() {
126-
return proxyProtocol;
124+
public boolean isNlbShardAwareness() {
125+
return nlbShardAwareness;
127126
}
128127

129128
/**
@@ -145,14 +144,14 @@ public boolean equals(Object o) {
145144
return false;
146145
}
147146
ClientRoutesConfig that = (ClientRoutesConfig) o;
148-
return proxyProtocol == that.proxyProtocol
147+
return nlbShardAwareness == that.nlbShardAwareness
149148
&& endpoints.equals(that.endpoints)
150149
&& tableName.equals(that.tableName);
151150
}
152151

153152
@Override
154153
public int hashCode() {
155-
return Objects.hash(endpoints, tableName, proxyProtocol);
154+
return Objects.hash(endpoints, tableName, nlbShardAwareness);
156155
}
157156

158157
@Override
@@ -163,8 +162,8 @@ public String toString() {
163162
+ ", tableName='"
164163
+ tableName
165164
+ '\''
166-
+ ", proxyProtocol="
167-
+ proxyProtocol
165+
+ ", nlbShardAwareness="
166+
+ nlbShardAwareness
168167
+ '}';
169168
}
170169

@@ -173,7 +172,7 @@ public static final class Builder {
173172
private final List<ClientRouteProxy> endpoints = new ArrayList<>();
174173
private final Set<String> seenConnectionIds = new HashSet<>();
175174
private String tableName = DEFAULT_TABLE_NAME;
176-
private boolean proxyProtocol = false;
175+
private boolean nlbShardAwareness = false;
177176

178177
/**
179178
* Adds an endpoint to the configuration.
@@ -232,22 +231,20 @@ Builder withTableName(@NonNull String tableName) {
232231
}
233232

234233
/**
235-
* Sets whether Proxy Protocol v2 (PP2) is in use between the NLB and ScyllaDB nodes.
234+
* Sets whether NLB shard awareness is enabled.
236235
*
237-
* <p>When {@code true}, the driver assumes the NLB prepends a PP2 binary header to each
238-
* connection it opens to ScyllaDB. The header carries the original client source IP and port,
239-
* which ScyllaDB uses to route the connection to the correct shard. This restores
240-
* shard-awareness end-to-end through the NLB.
236+
* <p>When {@code true}, the driver assumes the NLB is configured to forward the driver's
237+
* original source port to ScyllaDB (e.g. via Proxy Protocol v2), enabling shard-aware
238+
* connection routing end-to-end through the NLB.
241239
*
242-
* <p>Requires: (1) the NLB configured with PP2, (2) ScyllaDB configured to accept PP2, (3)
243-
* {@code advanced-shard-awareness.enabled = true} (the default).
240+
* <p>Requires {@code advanced-shard-awareness.enabled = true} (the default).
244241
*
245-
* @param proxyProtocol {@code true} to enable PP2 mode.
242+
* @param nlbShardAwareness {@code true} to enable NLB shard awareness.
246243
* @return this builder.
247244
*/
248245
@NonNull
249-
public Builder withProxyProtocol(boolean proxyProtocol) {
250-
this.proxyProtocol = proxyProtocol;
246+
public Builder withNlbShardAwareness(boolean nlbShardAwareness) {
247+
this.nlbShardAwareness = nlbShardAwareness;
251248
return this;
252249
}
253250

@@ -259,7 +256,7 @@ public Builder withProxyProtocol(boolean proxyProtocol) {
259256
*/
260257
@NonNull
261258
public ClientRoutesConfig build() {
262-
return new ClientRoutesConfig(endpoints, tableName, proxyProtocol);
259+
return new ClientRoutesConfig(endpoints, tableName, nlbShardAwareness);
263260
}
264261
}
265262
}

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,17 +1099,16 @@ public enum DefaultDriverOption implements DriverOption {
10991099
*/
11001100
CLIENT_ROUTES_ENDPOINTS("advanced.client-routes.endpoints"),
11011101
/**
1102-
* Whether Proxy Protocol v2 (PP2) is in use between the NLB and ScyllaDB nodes.
1102+
* Whether NLB shard awareness is enabled for client-routes deployments.
11031103
*
1104-
* <p>When {@code true}, the driver assumes the NLB prepends a PP2 binary header to each
1105-
* connection it opens to ScyllaDB. The header carries the original client source IP and port,
1106-
* enabling shard-aware routing through the NLB. Requires: (1) NLB configured with PP2, (2)
1107-
* ScyllaDB configured to accept PP2, (3) {@code advanced-shard-awareness.enabled = true} (the
1104+
* <p>When {@code true}, the driver assumes the NLB is configured to forward the driver's original
1105+
* source port to ScyllaDB (e.g. via Proxy Protocol v2), enabling shard-aware connection routing
1106+
* end-to-end through the NLB. Requires {@code advanced-shard-awareness.enabled = true} (the
11081107
* default).
11091108
*
11101109
* <p>Value type: boolean
11111110
*/
1112-
CLIENT_ROUTES_PROXY_PROTOCOL("advanced.client-routes.proxy-protocol");
1111+
CLIENT_ROUTES_NLB_SHARD_AWARENESS("advanced.client-routes.proxy-protocol");
11131112

11141113
private final String path;
11151114

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,7 +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_PROXY_PROTOCOL, false);
401+
map.put(TypedDriverOption.CLIENT_ROUTES_NLB_SHARD_AWARENESS, false);
402402
}
403403

404404
@Immutable

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

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

947-
/** Whether Proxy Protocol v2 is in use between the NLB and ScyllaDB nodes. */
948-
public static final TypedDriverOption<Boolean> CLIENT_ROUTES_PROXY_PROTOCOL =
947+
/** Whether NLB shard awareness is enabled for client-routes deployments. */
948+
public static final TypedDriverOption<Boolean> CLIENT_ROUTES_NLB_SHARD_AWARENESS =
949949
new TypedDriverOption<>(
950-
DefaultDriverOption.CLIENT_ROUTES_PROXY_PROTOCOL, GenericType.BOOLEAN);
950+
DefaultDriverOption.CLIENT_ROUTES_NLB_SHARD_AWARENESS, GenericType.BOOLEAN);
951951

952952
private static Iterable<TypedDriverOption<?>> introspectBuiltInValues() {
953953
try {

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

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -514,9 +514,9 @@ ClientRoutesConfig buildClientRoutesConfigFromFile() {
514514

515515
ClientRoutesConfig.Builder builder = ClientRoutesConfig.builder();
516516

517-
if (defaultProfile.isDefined(DefaultDriverOption.CLIENT_ROUTES_PROXY_PROTOCOL)) {
518-
builder.withProxyProtocol(
519-
defaultProfile.getBoolean(DefaultDriverOption.CLIENT_ROUTES_PROXY_PROTOCOL));
517+
if (defaultProfile.isDefined(DefaultDriverOption.CLIENT_ROUTES_NLB_SHARD_AWARENESS)) {
518+
builder.withNlbShardAwareness(
519+
defaultProfile.getBoolean(DefaultDriverOption.CLIENT_ROUTES_NLB_SHARD_AWARENESS));
520520
}
521521

522522
for (int i = 0; i < endpointsList.size(); i++) {
@@ -649,24 +649,17 @@ private void validateClientRoutesConfiguration(ClientRoutesConfig clientRoutesCo
649649
+ "They are mutually exclusive. Please use either a secure connect bundle OR "
650650
+ "client routes configuration, but not both.");
651651
}
652-
if (clientRoutesConfig.isProxyProtocol()) {
652+
if (clientRoutesConfig.isNlbShardAwareness()) {
653653
boolean shardAwarenessEnabled =
654654
getConfig()
655655
.getDefaultProfile()
656656
.getBoolean(DefaultDriverOption.CONNECTION_ADVANCED_SHARD_AWARENESS_ENABLED);
657657
if (!shardAwarenessEnabled) {
658-
// proxyProtocol=true signals that the NLB will forward the driver's original source port
659-
// to ScyllaDB via a PP2 header, enabling shard-aware connection routing through the proxy.
660-
// However, this only has effect when advanced shard awareness is also enabled — that is the
661-
// mechanism that binds a shard-specific local port in the first place. With shard awareness
662-
// disabled the driver uses a random local port, so PP2 forwards a port that carries no
663-
// shard intent. The setting is therefore ignored. If shard-aware routing through the NLB
664-
// is desired, enable advanced-shard-awareness (it is on by default).
665658
LOG.warn(
666-
"[{}] ClientRoutesConfig has proxyProtocol=true but {} is false. "
667-
+ "Proxy Protocol v2 has no effect without advanced shard awareness — "
668-
+ "the proxyProtocol flag will be ignored. To enable shard-aware routing "
669-
+ "through the NLB, set advanced-shard-awareness.enabled=true (the default).",
659+
"[{}] ClientRoutesConfig has nlbShardAwareness=true but {} is false. "
660+
+ "NLB shard awareness has no effect without advanced shard awareness enabled — "
661+
+ "the nlbShardAwareness setting will have no effect. To enable shard-aware "
662+
+ "routing through the NLB, set advanced-shard-awareness.enabled=true (the default).",
670663
getSessionName(),
671664
DefaultDriverOption.CONNECTION_ADVANCED_SHARD_AWARENESS_ENABLED.getPath());
672665
}

core/src/main/resources/reference.conf

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1158,8 +1158,8 @@ datastax-java-driver {
11581158
# Requires: (1) NLB configured with PP2, (2) ScyllaDB configured to accept PP2,
11591159
# (3) advanced.connection.advanced-shard-awareness.enabled = true (the default).
11601160
#
1161-
# If this is true but advanced-shard-awareness.enabled is false, the driver logs a warning and
1162-
# ignores the setting — there is no shard-specific port to forward.
1161+
# If this is true but advanced-shard-awareness.enabled is false, the driver logs a warning
1162+
# (the flag has no effect in this configuration) — there is no shard-specific port to forward.
11631163
#
11641164
# Required: no
11651165
# Default: false

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

Lines changed: 0 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -820,59 +820,6 @@ public void should_select_tls_port_when_ssl_configured() throws Exception {
820820
}
821821
}
822822

823-
@Test
824-
public void should_use_shard_awareness_through_pp2_nlb() throws Exception {
825-
// Verify server supports client_routes
826-
try (CqlSession admin = openAdminSession()) {
827-
requireSystemClientRoutesTable(admin);
828-
}
829-
try (CcmBridge ccm = CcmBridge.builder().withNodes(1).build()) {
830-
ccm.create();
831-
ccm.start();
832-
833-
// Create PP2-enabled NLB
834-
NlbSimulator nlb =
835-
new NlbSimulator(ccm, NLB_ADDRESS, NLB_BASE_PORT, /* proxyProtocol= */ true);
836-
try {
837-
nlb.addNode(1);
838-
Map<Integer, UUID> hostIds = collectHostIds(ccm, 1);
839-
postClientRoutes(ccm, hostIds, nlb);
840-
waitForRoutesVisibleOnAllNodes(ccm, hostIds.keySet(), hostIds.size());
841-
842-
ClientRoutesConfig config =
843-
ClientRoutesConfig.builder()
844-
.addEndpoint(new ClientRouteProxy(CONNECTION_ID, NLB_ADDRESS))
845-
.withProxyProtocol(true)
846-
.build();
847-
848-
try (CqlSession session = openNlbSession(config, nlb)) {
849-
assertQueryWorks(session);
850-
851-
// With PP2, the NLB forwards the driver's original source port to ScyllaDB.
852-
// The driver binds shard-specific local ports (advanced shard awareness, on by default),
853-
// so ScyllaDB can route each connection to the correct shard. Verify the pool works
854-
// correctly by running queries and waiting for the full pool to be established.
855-
Node node = session.getMetadata().getNodes().values().iterator().next();
856-
int shardCount =
857-
node.getShardingInfo() != null ? node.getShardingInfo().getShardsCount() : 1;
858-
859-
// Wait for full shard-aware pool to be established (one connection per shard)
860-
await()
861-
.atMost(30, TimeUnit.SECONDS)
862-
.pollInterval(1, TimeUnit.SECONDS)
863-
.until(() -> node.getOpenConnections() >= shardCount);
864-
865-
// Run queries to verify shard-aware routing works end-to-end through the PP2 NLB
866-
for (int i = 0; i < 20; i++) {
867-
assertQueryWorks(session);
868-
}
869-
}
870-
} finally {
871-
nlb.close();
872-
}
873-
}
874-
}
875-
876823
@Test
877824
public void should_work_with_mixed_proxy_and_direct_nodes() throws Exception {
878825
try (CqlSession admin = openAdminSession()) {

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

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ public class NlbSimulator implements Closeable {
5757
private final CcmBridge ccmBridge;
5858
private final String bindAddress;
5959
private final int basePort;
60-
private final boolean proxyProtocol;
6160

6261
// Discovery port proxy: round-robins across all nodes
6362
private volatile RoundRobinProxy discoveryProxy;
@@ -75,24 +74,9 @@ public class NlbSimulator implements Closeable {
7574
* @param basePort the base port for NLB (discovery on basePort, per-node on basePort+nodeId)
7675
*/
7776
public NlbSimulator(CcmBridge ccmBridge, String bindAddress, int basePort) {
78-
this(ccmBridge, bindAddress, basePort, false);
79-
}
80-
81-
/**
82-
* Creates an NLB simulator with optional Proxy Protocol v2 support.
83-
*
84-
* @param ccmBridge the CCM bridge to get node addresses from
85-
* @param bindAddress the IP address to bind proxy listeners on (e.g. "127.254.254.254")
86-
* @param basePort the base port for NLB (discovery on basePort, per-node on basePort+nodeId)
87-
* @param proxyProtocol when {@code true}, all proxied connections will include a PP2 binary
88-
* header carrying the original client IP and source port
89-
*/
90-
public NlbSimulator(
91-
CcmBridge ccmBridge, String bindAddress, int basePort, boolean proxyProtocol) {
9277
this.ccmBridge = ccmBridge;
9378
this.bindAddress = bindAddress;
9479
this.basePort = basePort;
95-
this.proxyProtocol = proxyProtocol;
9680
}
9781

9882
/** Returns the bind address used by this NLB simulator. */
@@ -130,7 +114,7 @@ public synchronized void addNode(int nodeId) throws IOException {
130114

131115
// Create per-node proxy first, before updating state
132116
int nodePort = getNodePort(nodeId);
133-
TcpProxy proxy = new TcpProxy(bindAddress, nodePort, nodeAddr, proxyProtocol);
117+
TcpProxy proxy = new TcpProxy(bindAddress, nodePort, nodeAddr);
134118

135119
// Update state and rebuild discovery proxy; if rebuild fails, close the new proxy
136120
activeNodes.add(nodeId);
@@ -180,7 +164,7 @@ private void rebuildDiscoveryProxy() throws IOException {
180164
}
181165

182166
// Create new proxy before closing old one to avoid inconsistent state on failure
183-
discoveryProxy = new RoundRobinProxy(bindAddress, basePort, targets, proxyProtocol);
167+
discoveryProxy = new RoundRobinProxy(bindAddress, basePort, targets);
184168
if (oldProxy != null) {
185169
oldProxy.close();
186170
}

0 commit comments

Comments
 (0)