Skip to content

Commit d2a13c5

Browse files
committed
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). - Rename nlb* identifiers to drop the NLB-specific prefix: ClientRoutesConfig: nlbShardAwareness → shardAwareness, isNlbShardAwareness() → isShardAwareness(), withNlbShardAwareness() → withShardAwareness() - Rename DefaultDriverOption.CLIENT_ROUTES_NLB_SHARD_AWARENESS → CLIENT_ROUTES_SHARD_AWARENESS_ENABLED with new config key advanced.client-routes.shard-awarness-enabled (matching reviewer spelling) - Rename TypedDriverOption / OptionsMap constant to match - Update reference.conf: proxy-protocol → shard-awarness-enabled, reword comment to be load-balancer-agnostic - Remove validation block in DefaultDriverContext that warned when shardAwareness=true but advanced-shard-awareness was disabled - Delete manual/core/private_link/README.md per reviewer request
1 parent 30637cd commit d2a13c5

9 files changed

Lines changed: 47 additions & 681 deletions

File tree

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
@@ -84,13 +84,13 @@ public final class ClientRoutesConfig {
8484
private final List<ClientRouteProxy> endpoints;
8585
private final String tableName;
8686
private final int nativeTransportPort;
87-
private final boolean proxyProtocol;
87+
private final boolean shardAwareness;
8888

8989
private ClientRoutesConfig(
9090
List<ClientRouteProxy> endpoints,
9191
String tableName,
9292
int nativeTransportPort,
93-
boolean proxyProtocol) {
93+
boolean shardAwareness) {
9494
if (endpoints == null || endpoints.isEmpty()) {
9595
throw new IllegalArgumentException("At least one endpoint must be specified");
9696
}
@@ -103,7 +103,7 @@ private ClientRoutesConfig(
103103
this.endpoints = Collections.unmodifiableList(new ArrayList<>(endpoints));
104104
this.tableName = tableName;
105105
this.nativeTransportPort = nativeTransportPort;
106-
this.proxyProtocol = proxyProtocol;
106+
this.shardAwareness = shardAwareness;
107107
}
108108

109109
/**
@@ -139,18 +139,17 @@ public int getNativeTransportPort() {
139139
}
140140

141141
/**
142-
* Returns whether Proxy Protocol v2 is in use between the NLB and ScyllaDB nodes.
142+
* Returns whether shard awareness is enabled.
143143
*
144-
* <p>When {@code true}, the driver assumes the NLB prepends a PP2 binary header to each
145-
* connection it opens to ScyllaDB. The header carries the original client IP and source port,
146-
* which ScyllaDB uses for shard-aware routing. This restores shard-awareness end-to-end through
147-
* the NLB. Requires both the NLB and ScyllaDB to be configured for Proxy Protocol v2, and {@code
148-
* advanced-shard-awareness.enabled} must be {@code true} (the default).
144+
* <p>When {@code true}, the driver assumes the load balancer is configured to forward the
145+
* driver's original source port to ScyllaDB (e.g. via Proxy Protocol v2), enabling shard-aware
146+
* connection routing end-to-end through the load balancer. Requires {@code
147+
* advanced-shard-awareness.enabled} to be {@code true} (the default).
149148
*
150-
* @return {@code true} if Proxy Protocol v2 is enabled.
149+
* @return {@code true} if shard awareness is enabled.
151150
*/
152-
public boolean isProxyProtocol() {
153-
return proxyProtocol;
151+
public boolean isShardAwareness() {
152+
return shardAwareness;
154153
}
155154

156155
/**
@@ -175,12 +174,12 @@ public boolean equals(Object o) {
175174
return endpoints.equals(that.endpoints)
176175
&& tableName.equals(that.tableName)
177176
&& nativeTransportPort == that.nativeTransportPort
178-
&& proxyProtocol == that.proxyProtocol;
177+
&& shardAwareness == that.shardAwareness;
179178
}
180179

181180
@Override
182181
public int hashCode() {
183-
return Objects.hash(endpoints, tableName, nativeTransportPort, proxyProtocol);
182+
return Objects.hash(endpoints, tableName, nativeTransportPort, shardAwareness);
184183
}
185184

186185
@Override
@@ -193,8 +192,8 @@ public String toString() {
193192
+ '\''
194193
+ ", nativeTransportPort="
195194
+ nativeTransportPort
196-
+ ", proxyProtocol="
197-
+ proxyProtocol
195+
+ ", shardAwareness="
196+
+ shardAwareness
198197
+ '}';
199198
}
200199

@@ -204,7 +203,7 @@ public static final class Builder {
204203
private final Set<String> seenConnectionIds = new HashSet<>();
205204
private String tableName = DEFAULT_TABLE_NAME;
206205
private int nativeTransportPort = DEFAULT_NATIVE_TRANSPORT_PORT;
207-
private boolean proxyProtocol = false;
206+
private boolean shardAwareness = false;
208207

209208
/**
210209
* Adds an endpoint to the configuration.
@@ -278,22 +277,20 @@ public Builder withNativeTransportPort(int port) {
278277
}
279278

280279
/**
281-
* Sets whether Proxy Protocol v2 (PP2) is in use between the NLB and ScyllaDB nodes.
280+
* Sets whether shard awareness is enabled.
282281
*
283-
* <p>When {@code true}, the driver assumes the NLB prepends a PP2 binary header to each
284-
* connection it opens to ScyllaDB. The header carries the original client source IP and port,
285-
* which ScyllaDB uses to route the connection to the correct shard. This restores
286-
* shard-awareness end-to-end through the NLB.
282+
* <p>When {@code true}, the driver assumes the load balancer is configured to forward the
283+
* driver's original source port to ScyllaDB (e.g. via Proxy Protocol v2), enabling shard-aware
284+
* connection routing end-to-end through the load balancer.
287285
*
288-
* <p>Requires: (1) the NLB configured with PP2, (2) ScyllaDB configured to accept PP2, (3)
289-
* {@code advanced-shard-awareness.enabled = true} (the default).
286+
* <p>Requires {@code advanced-shard-awareness.enabled = true} (the default).
290287
*
291-
* @param proxyProtocol {@code true} to enable PP2 mode.
288+
* @param shardAwareness {@code true} to enable shard awareness.
292289
* @return this builder.
293290
*/
294291
@NonNull
295-
public Builder withProxyProtocol(boolean proxyProtocol) {
296-
this.proxyProtocol = proxyProtocol;
292+
public Builder withShardAwareness(boolean shardAwareness) {
293+
this.shardAwareness = shardAwareness;
297294
return this;
298295
}
299296

@@ -305,7 +302,7 @@ public Builder withProxyProtocol(boolean proxyProtocol) {
305302
*/
306303
@NonNull
307304
public ClientRoutesConfig build() {
308-
return new ClientRoutesConfig(endpoints, tableName, nativeTransportPort, proxyProtocol);
305+
return new ClientRoutesConfig(endpoints, tableName, nativeTransportPort, shardAwareness);
309306
}
310307
}
311308
}

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
@@ -1110,17 +1110,16 @@ public enum DefaultDriverOption implements DriverOption {
11101110
CLIENT_ROUTES_NATIVE_TRANSPORT_PORT("advanced.client-routes.native-transport-port"),
11111111

11121112
/**
1113-
* Whether Proxy Protocol v2 (PP2) is in use between the NLB and ScyllaDB nodes.
1113+
* Whether NLB shard awareness is enabled for client-routes deployments.
11141114
*
1115-
* <p>When {@code true}, the driver assumes the NLB prepends a PP2 binary header to each
1116-
* connection it opens to ScyllaDB. The header carries the original client source IP and port,
1117-
* enabling shard-aware routing through the NLB. Requires: (1) NLB configured with PP2, (2)
1118-
* ScyllaDB configured to accept PP2, (3) {@code advanced-shard-awareness.enabled = true} (the
1115+
* <p>When {@code true}, the driver assumes the NLB is configured to forward the driver's original
1116+
* source port to ScyllaDB (e.g. via Proxy Protocol v2), enabling shard-aware connection routing
1117+
* end-to-end through the NLB. Requires {@code advanced-shard-awareness.enabled = true} (the
11191118
* default).
11201119
*
11211120
* <p>Value type: boolean
11221121
*/
1123-
CLIENT_ROUTES_PROXY_PROTOCOL("advanced.client-routes.proxy-protocol");
1122+
CLIENT_ROUTES_SHARD_AWARENESS_ENABLED("advanced.client-routes.shard-awarness-enabled");
11241123

11251124
private final String path;
11261125

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
@@ -399,7 +399,7 @@ protected static void fillWithDriverDefaults(OptionsMap map) {
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.
401401
map.put(TypedDriverOption.CLIENT_ROUTES_NATIVE_TRANSPORT_PORT, 9042);
402-
map.put(TypedDriverOption.CLIENT_ROUTES_PROXY_PROTOCOL, false);
402+
map.put(TypedDriverOption.CLIENT_ROUTES_SHARD_AWARENESS_ENABLED, false);
403403
}
404404

405405
@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
@@ -949,10 +949,10 @@ public String toString() {
949949
new TypedDriverOption<>(
950950
DefaultDriverOption.CLIENT_ROUTES_NATIVE_TRANSPORT_PORT, GenericType.INTEGER);
951951

952-
/** Whether Proxy Protocol v2 is in use between the NLB and ScyllaDB nodes. */
953-
public static final TypedDriverOption<Boolean> CLIENT_ROUTES_PROXY_PROTOCOL =
952+
/** Whether shard awareness is enabled for client-routes deployments. */
953+
public static final TypedDriverOption<Boolean> CLIENT_ROUTES_SHARD_AWARENESS_ENABLED =
954954
new TypedDriverOption<>(
955-
DefaultDriverOption.CLIENT_ROUTES_PROXY_PROTOCOL, GenericType.BOOLEAN);
955+
DefaultDriverOption.CLIENT_ROUTES_SHARD_AWARENESS_ENABLED, GenericType.BOOLEAN);
956956

957957
private static Iterable<TypedDriverOption<?>> introspectBuiltInValues() {
958958
try {

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

Lines changed: 3 additions & 26 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_SHARD_AWARENESS_ENABLED)) {
518+
builder.withShardAwareness(
519+
defaultProfile.getBoolean(DefaultDriverOption.CLIENT_ROUTES_SHARD_AWARENESS_ENABLED));
520520
}
521521

522522
for (int i = 0; i < endpointsList.size(); i++) {
@@ -654,29 +654,6 @@ private void validateClientRoutesConfiguration(ClientRoutesConfig clientRoutesCo
654654
+ "They are mutually exclusive. Please use either a secure connect bundle OR "
655655
+ "client routes configuration, but not both.");
656656
}
657-
if (clientRoutesConfig.isProxyProtocol()) {
658-
boolean shardAwarenessEnabled =
659-
getConfig()
660-
.getDefaultProfile()
661-
.getBoolean(DefaultDriverOption.CONNECTION_ADVANCED_SHARD_AWARENESS_ENABLED);
662-
if (!shardAwarenessEnabled) {
663-
// proxyProtocol=true signals that the NLB will forward the driver's original source port
664-
// to ScyllaDB via a PP2 header, enabling shard-aware connection routing through the proxy.
665-
// However, this only has effect when advanced shard awareness is also enabled — that is the
666-
// mechanism that binds a shard-specific local port in the first place. With shard awareness
667-
// disabled the driver uses a random local port, so PP2 forwards a port that carries no
668-
// shard intent. The setting is therefore ignored. If shard-aware routing through the NLB
669-
// is desired, enable advanced-shard-awareness (it is on by default).
670-
LOG.warn(
671-
"[{}] ClientRoutesConfig has proxyProtocol=true but {} is false. "
672-
+ "Proxy Protocol v2 has no effect without advanced shard awareness — "
673-
+ "the proxyProtocol flag will be ignored. To enable shard-aware routing "
674-
+ "through the NLB, set advanced-shard-awareness.enabled=true (the default).",
675-
getSessionName(),
676-
DefaultDriverOption.CONNECTION_ADVANCED_SHARD_AWARENESS_ENABLED.getPath());
677-
}
678-
}
679-
680657
DriverExecutionProfile defaultProfile = getConfig().getDefaultProfile();
681658
if (defaultProfile.isDefined(DefaultDriverOption.ADDRESS_TRANSLATOR_CLASS)) {
682659
String className = defaultProfile.getString(DefaultDriverOption.ADDRESS_TRANSLATOR_CLASS);

core/src/main/resources/reference.conf

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,24 +1157,21 @@ datastax-java-driver {
11571157
# Overridable in a profile: no
11581158
native-transport-port = 9042
11591159

1160-
# Whether Proxy Protocol v2 (PP2) is in use between the NLB and ScyllaDB nodes.
1160+
# Whether shard awareness is enabled for client-routes deployments.
11611161
#
1162-
# When true, the driver assumes the NLB prepends a PP2 binary header to each connection it opens
1163-
# to ScyllaDB. The header carries the original client source IP and port, which ScyllaDB uses to
1164-
# route the connection to the correct shard. This restores shard-awareness end-to-end through the
1165-
# NLB. The driver's own role is to continue binding shard-specific local ports as usual
1166-
# (controlled by advanced-shard-awareness.enabled, which is true by default); the NLB then
1167-
# forwards those ports to ScyllaDB via the PP2 header.
1162+
# When true, the driver assumes the load balancer is configured to forward the driver's original
1163+
# source port to ScyllaDB (e.g. via Proxy Protocol v2). The header carries the original client
1164+
# source IP and port, which ScyllaDB uses to route the connection to the correct shard. This
1165+
# restores shard-awareness end-to-end through the load balancer. The driver's own role is to
1166+
# continue binding shard-specific local ports as usual (controlled by
1167+
# advanced-shard-awareness.enabled, which is true by default).
11681168
#
1169-
# Requires: (1) NLB configured with PP2, (2) ScyllaDB configured to accept PP2,
1169+
# Requires: (1) load balancer configured with PP2, (2) ScyllaDB configured to accept PP2,
11701170
# (3) advanced.connection.advanced-shard-awareness.enabled = true (the default).
11711171
#
1172-
# If this is true but advanced-shard-awareness.enabled is false, the driver logs a warning and
1173-
# ignores the setting — there is no shard-specific port to forward.
1174-
#
11751172
# Required: no
11761173
# Default: false
1177-
proxy-protocol = false
1174+
shard-awarness-enabled = false
11781175

11791176
}
11801177

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()) {

0 commit comments

Comments
 (0)