absurdfarce commented on code in PR #1910:
URL:
https://github.com/apache/cassandra-java-driver/pull/1910#discussion_r1480702424
##########
core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTopologyMonitor.java:
##########
@@ -506,16 +506,32 @@ protected InetSocketAddress getBroadcastRpcAddress(
}
// system.local for Cassandra >= 4.0
Integer broadcastRpcPort = row.getInteger("rpc_port");
+ Integer broadcastRpcPortSsl;
if (broadcastRpcPort == null || broadcastRpcPort == 0) {
// system.peers_v2
broadcastRpcPort = row.getInteger("native_port");
+ broadcastRpcPortSsl = row.getInteger("native_port_ssl");
+
if (broadcastRpcPort == null || broadcastRpcPort == 0) {
- // use the default port if no port information was found in the row;
- // note that in rare situations, the default port might not be known,
in which case we
- // report zero, as advertised in the javadocs of Node and NodeInfo.
- broadcastRpcPort = port == -1 ? 0 : port;
+ if (broadcastRpcPortSsl == null || broadcastRpcPortSsl == 0) {
+ broadcastRpcPort = port == -1 ? 0 : port;
+ } else {
+ broadcastRpcPort = broadcastRpcPortSsl;
+ }
+ } else {
+ if (broadcastRpcPortSsl != null && broadcastRpcPortSsl != 0) {
+ if (!broadcastRpcPortSsl.equals(broadcastRpcPort)) {
+ broadcastRpcPort = broadcastRpcPortSsl;
Review Comment:
We only want to perform this substitution if the driver is configured to use
SSL. If it isn't it's picture of the nodes should still use what's in
native_port.
##########
core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTopologyMonitor.java:
##########
@@ -506,16 +506,32 @@ protected InetSocketAddress getBroadcastRpcAddress(
}
// system.local for Cassandra >= 4.0
Integer broadcastRpcPort = row.getInteger("rpc_port");
+ Integer broadcastRpcPortSsl;
if (broadcastRpcPort == null || broadcastRpcPort == 0) {
// system.peers_v2
broadcastRpcPort = row.getInteger("native_port");
+ broadcastRpcPortSsl = row.getInteger("native_port_ssl");
+
if (broadcastRpcPort == null || broadcastRpcPort == 0) {
- // use the default port if no port information was found in the row;
- // note that in rare situations, the default port might not be known,
in which case we
- // report zero, as advertised in the javadocs of Node and NodeInfo.
- broadcastRpcPort = port == -1 ? 0 : port;
+ if (broadcastRpcPortSsl == null || broadcastRpcPortSsl == 0) {
+ broadcastRpcPort = port == -1 ? 0 : port;
+ } else {
+ broadcastRpcPort = broadcastRpcPortSsl;
+ }
+ } else {
+ if (broadcastRpcPortSsl != null && broadcastRpcPortSsl != 0) {
+ if (!broadcastRpcPortSsl.equals(broadcastRpcPort)) {
+ broadcastRpcPort = broadcastRpcPortSsl;
+ }
+ }
+ }
+ } else {
+ InetSocketAddress address = (InetSocketAddress) localEndPoint.resolve();
+ if (broadcastRpcPort != address.getPort()) {
+ broadcastRpcPort = address.getPort();
Review Comment:
This is a very interesting case; I'm glad you raised this. I'm genuinely
not sure what to do here.
A couple observations spring to mind right away:
* Whatever we implement for a check here should be used regardless of
whether something was configured for native_port or not.
* We don't want to silently change ports if we detect this case. At a
minimum we should log a large error message saying something along the lines of
"We've detected multiple native transport ports in your cluster. As of
Cassandra x.y.z all native transport ports must be the same. We will use the
native transport port from the control connection for this session" or
something similar
This moves to a more interesting question: do we _want_ to re-write the port
number here? If we discover this condition it seems like we've discovered a
fairly significant configuration issue within the cluster... perhaps it's best
at that point to throw and for the user (or their representative) to fix their
cluster?
Honestly I'm kind of torn here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
