feat: stop pinning the driver to one address per hostname (DRIVER-201)

core/src/main/java/com/datastax/dse/driver/api/core/auth/DseGssApiAuthProviderBase.java

@@ -27,6 +27,7 @@
 import com.datastax.oss.protocol.internal.util.Bytes;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.Nullable;
+import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.nio.ByteBuffer;
 import java.security.PrivilegedActionException;
@@ -319,7 +320,7 @@ protected GssApiAuthenticator(
                 SUPPORTED_MECHANISMS,
                 options.getAuthorizationId(),
                 protocol,
-                ((InetSocketAddress) endPoint.resolve()).getAddress().getCanonicalHostName(),
+                serverName(endPoint),
                 options.getSaslProperties(),
                 null);
       } catch (LoginException | SaslException e) {
@@ -328,6 +329,23 @@ protected GssApiAuthenticator(
       this.endPoint = endPoint;
     }
 
+    /**
+     * The host name to build the Kerberos service principal from.
+     *
+     * <p>Prefers the canonical name of the resolved address, which is what Kerberos expects. The
+     * driver's own endpoints always hand this a resolved address — the channel carries an endpoint
+     * bound to the address it connected to (see {@code PinnableEndPoint}) — but a custom {@link
+     * EndPoint} implementation may still yield an unresolved one, in which case {@code
+     * getAddress()} is null. Fall back to the host string rather than throwing a {@link
+     * NullPointerException}: the hostname is usually the right service name anyway, and a failed
+     * reverse lookup should not take authentication down.
+     */
+    private static String serverName(EndPoint endPoint) {
+      InetSocketAddress address = (InetSocketAddress) endPoint.resolve();
+      InetAddress inetAddress = address.getAddress();
+      return inetAddress != null ? inetAddress.getCanonicalHostName() : address.getHostString();
+    }
+
     @NonNull
     @Override
     protected ByteBuffer getMechanism() {

core/src/main/java/com/datastax/dse/driver/internal/core/insights/InsightsClient.java

@@ -294,7 +294,32 @@ private Map<String, SessionStateForNode> getConnectedNodes() {
         .collect(
             Collectors.toMap(
                 entry -> AddressFormatter.nullSafeToString(entry.getKey().getEndPoint().resolve()),
-                this::constructSessionStateForNode));
+                this::constructSessionStateForNode,
+                InsightsClient::mergeNodeStates));
+  }
+
+  /**
+   * Combines the states of two nodes that report under the same address.
+   *
+   * <p>The key is not unique per node: behind an SNI proxy or a cloud client route, every node's
+   * endpoint resolves to the same proxy address, so any session with more than one node open
+   * produces duplicate keys. Without a merge function {@link Collectors#toMap} throws {@link
+   * IllegalStateException}, which propagates out of the status report and aborts it every interval.
+   * Summing matches what the shared key denotes in that deployment: the totals reached through that
+   * address.
+   */
+  private static SessionStateForNode mergeNodeStates(
+      SessionStateForNode first, SessionStateForNode second) {
+    return new SessionStateForNode(
+        sumNullable(first.getConnections(), second.getConnections()),
+        sumNullable(first.getInFlightQueries(), second.getInFlightQueries()));
+  }
+
+  private static Integer sumNullable(Integer first, Integer second) {
+    if (first == null) {
+      return second;
+    }
+    return second == null ? first : first + second;
   }
 
   private SessionStateForNode constructSessionStateForNode(Map.Entry<Node, ChannelPool> entry) {
@@ -363,6 +388,35 @@ private long getPeriodicStatusInterval() {
     return TimeUnit.MILLISECONDS.toSeconds(insightsConfiguration.getStatusEventDelayMillis());
   }
 
+  /**
+   * Groups the contact points by host name, which for a hostname contact point now means grouping
+   * it with itself.
+   *
+   * <p>The field was name to list-of-addresses, and for a name with several A-records that list was
+   * the interesting part. Contact points are kept unresolved now ({@code SessionBuilder} merges
+   * them with resolution off), so {@code endPoint.resolve()} hands back one unresolved address per
+   * contact point and {@code AddressFormatter} renders it as the name again: {@code
+   * {"db.example.com": ["db.example.com:9042"]}} where it used to be {@code {"db.example.com":
+   * ["10.0.0.1:9042", "10.0.0.2:9042"]}}. A contact point given as an IP literal is unaffected in
+   * shape, though see below for what it used to be keyed on.
+   *
+   * <p>Not resolved here to restore the old shape, deliberately. This runs on the admin executor --
+   * {@code DefaultSession}'s single-threaded init calls {@code onSessionReady} there -- which is
+   * the thread the rest of this change went to some length to keep name lookups off (see issue
+   * #1006), and it is shared with the control connection. Paying a blocking lookup per contact
+   * point on it to fill in a telemetry field is the wrong trade; the addresses the session actually
+   * reached are still reported, one of them by {@code #getControlConnectionSocketAddress}.
+   *
+   * <p>Which is also why the key is {@link InetSocketAddress#getHostString()} and not {@code
+   * getHostName()}. The two agree on everything the paragraph above is about -- an unresolved
+   * address, and a resolved one built from a name, both carry the label and both return it -- but
+   * for a resolved address with no label {@code getHostName()} falls through to {@link
+   * java.net.InetAddress#getHostName()}, a reverse lookup, and that is exactly what {@code
+   * addContactPoints(new InetSocketAddress(InetAddress.getByAddress(...), port))} produces.
+   * Grouping on it would put the one blocking lookup this method refuses to make back on the admin
+   * executor, per such contact point, at each {@code advanced.monitor-reporting} interval, and key
+   * the map on a reverse-zone answer rather than on anything the operator configured.
+   */
   @VisibleForTesting
   static Map<String, List<String>> getResolvedContactPoints(Set<InetSocketAddress> contactPoints) {
     if (contactPoints == null) {
@@ -371,7 +425,7 @@ static Map<String, List<String>> getResolvedContactPoints(Set<InetSocketAddress>
     return contactPoints.stream()
         .collect(
             Collectors.groupingBy(
-                InetSocketAddress::getHostName,
+                InetSocketAddress::getHostString,
                 Collectors.mapping(AddressFormatter::nullSafeToString, Collectors.toList())));
   }
 

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

@@ -135,6 +135,13 @@ public enum DefaultDriverOption implements DriverOption {
    * <p>Value-type: int
    */
   CONNECTION_MAX_ORPHAN_REQUESTS("advanced.connection.max-orphan-requests"),
+  /**
+   * The maximum number of addresses a single connection attempt will try, when the endpoint it
+   * connects to is a DNS name that resolves to several addresses.
+   *
+   * <p>Value-type: int
+   */
+  CONNECTION_MAX_CANDIDATE_ADDRESSES("advanced.connection.max-candidate-addresses"),
   /**
    * Whether to log non-fatal errors when the driver tries to open a new connection.
    *
@@ -701,8 +708,29 @@ public enum DefaultDriverOption implements DriverOption {
   CONTROL_CONNECTION_AGREEMENT_WARN("advanced.control-connection.schema-agreement.warn-on-failure"),
 
   /**
-   * Whether to forcibly add original contact points held by MetadataManager to the reconnection
-   * plan, in case there is no live nodes available according to LBP. Experimental.
+   * Whether to append the original contact points held by MetadataManager to the reconnection plan,
+   * after the live nodes reported by the load balancing policy. Defaults to {@code true}.
+   *
+   * <p>This is also the driver's DNS re-resolution path, for the ordinary case. Contact points are
+   * appended as-is, still unresolved hostnames, and each is expanded to its current DNS IPs at
+   * connection time through Netty's configured resolver. Metadata nodes, in contrast, hold an
+   * already-resolved endpoint that is never re-resolved. Keeping this enabled lets
+   * control-connection reconnects re-resolve the original hostnames and pick up new IPs once the
+   * live-node plan is exhausted.
+   *
+   * <p>Not the only path, though, and for two kinds of deployment it is barely a path at all: an
+   * {@code AddressTranslator} that returns a hostname keeps every node's endpoint unresolved, and a
+   * Cloud (SNI) session builds every endpoint as an {@code SniEndPoint}, which does the same. Both
+   * re-expand on every attempt whatever this is set to -- and for Cloud the append is skipped
+   * anyway unless the live-node plan is empty, since {@link
+   * com.datastax.oss.driver.internal.core.metadata.TopologyMonitor#reresolvesNodeAddresses()} is
+   * true there. Turning it off then removes only the empty-plan fallback.
+   *
+   * <p>A client-routes session is not a third case, though it looks like one. Its endpoints
+   * re-expand only for nodes that have a live route; a route-less node falls back to a static,
+   * already-resolved address, which is why {@code reresolvesNodeAddresses()} answers true there
+   * only while every known node has a route. Below full coverage the contact points are appended as
+   * usual and this option is the only re-resolution those nodes get.
    *
    * <p>Value-type: boolean
    */
@@ -837,7 +865,14 @@ public enum DefaultDriverOption implements DriverOption {
    * Whether to resolve the addresses passed to `basic.contact-points`.
    *
    * <p>Value-type: boolean
+   *
+   * @deprecated Setting this option has no effect. Contact points given in the configuration are
+   *     now always kept as unresolved hostnames and expanded to all of their DNS-mapped IPs lazily
+   *     at connection time. This never applied to programmatic contact points passed to {@code
+   *     SessionBuilder.addContactPoints}, which are used exactly as supplied -- an already-resolved
+   *     address stays bound to that one IP.
    */
+  @Deprecated
   RESOLVE_CONTACT_POINTS("advanced.resolve-contact-points"),
 
   /**

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

@@ -245,6 +245,9 @@ private void readObject(ObjectInputStream stream) throws InvalidObjectException
     throw new InvalidObjectException("Proxy required");
   }
 
+  // RESOLVE_CONTACT_POINTS is deprecated and has no effect, but it is still a driver option, so the
+  // defaults map stays complete by carrying its reference.conf value.
+  @SuppressWarnings("deprecation")
   protected static void fillWithDriverDefaults(OptionsMap map) {
     Duration initQueryTimeout = Duration.ofSeconds(5);
     Duration requestTimeout = Duration.ofSeconds(2);
@@ -276,6 +279,7 @@ protected static void fillWithDriverDefaults(OptionsMap map) {
     map.put(TypedDriverOption.CONNECTION_POOL_INIT_BATCH_SIZE, 0);
     map.put(TypedDriverOption.CONNECTION_MAX_REQUESTS, 1024);
     map.put(TypedDriverOption.CONNECTION_MAX_ORPHAN_REQUESTS, 256);
+    map.put(TypedDriverOption.CONNECTION_MAX_CANDIDATE_ADDRESSES, 5);
     map.put(TypedDriverOption.CONNECTION_WARN_INIT_ERROR, true);
     map.put(TypedDriverOption.CONNECTION_ADVANCED_SHARD_AWARENESS_ENABLED, true);
     map.put(TypedDriverOption.ADVANCED_SHARD_AWARENESS_PORT_LOW, 10000);
@@ -369,7 +373,7 @@ protected static void fillWithDriverDefaults(OptionsMap map) {
     map.put(TypedDriverOption.CONTROL_CONNECTION_AGREEMENT_INTERVAL, Duration.ofMillis(200));
     map.put(TypedDriverOption.CONTROL_CONNECTION_AGREEMENT_TIMEOUT, Duration.ofSeconds(10));
     map.put(TypedDriverOption.CONTROL_CONNECTION_AGREEMENT_WARN, true);
-    map.put(TypedDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS, false);
+    map.put(TypedDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS, true);
     map.put(TypedDriverOption.PREPARE_ON_ALL_NODES, true);
     map.put(TypedDriverOption.REPREPARE_ENABLED, true);
     map.put(TypedDriverOption.REPREPARE_CHECK_SYSTEM_TABLE, false);

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

@@ -172,6 +172,13 @@ public String toString() {
   public static final TypedDriverOption<Integer> CONNECTION_MAX_ORPHAN_REQUESTS =
       new TypedDriverOption<>(
           DefaultDriverOption.CONNECTION_MAX_ORPHAN_REQUESTS, GenericType.INTEGER);
+  /**
+   * The maximum number of addresses a single connection attempt will try, when the endpoint it
+   * connects to is a DNS name that resolves to several addresses.
+   */
+  public static final TypedDriverOption<Integer> CONNECTION_MAX_CANDIDATE_ADDRESSES =
+      new TypedDriverOption<>(
+          DefaultDriverOption.CONNECTION_MAX_CANDIDATE_ADDRESSES, GenericType.INTEGER);
   /** Whether to log non-fatal errors when the driver tries to open a new connection. */
   public static final TypedDriverOption<Boolean> CONNECTION_WARN_INIT_ERROR =
       new TypedDriverOption<>(DefaultDriverOption.CONNECTION_WARN_INIT_ERROR, GenericType.BOOLEAN);
@@ -600,7 +607,15 @@ public String toString() {
   public static final TypedDriverOption<Boolean> CONTROL_CONNECTION_AGREEMENT_WARN =
       new TypedDriverOption<>(
           DefaultDriverOption.CONTROL_CONNECTION_AGREEMENT_WARN, GenericType.BOOLEAN);
-  /** Whether to forcibly try original contacts if no live nodes are available */
+  /**
+   * Whether to append the original contact points to the control-connection reconnection plan,
+   * after the live nodes reported by the load balancing policy (defaults to {@code true}).
+   *
+   * <p>Contact points are appended as-is (unresolved hostnames); each is expanded to all of its
+   * current DNS IPs at connection time, which is also the driver's DNS re-resolution mechanism. The
+   * append is skipped for topology monitors that re-resolve node addresses themselves (such as the
+   * cloud/proxy monitors).
+   */
   public static final TypedDriverOption<Boolean> CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS =
       new TypedDriverOption<>(
           DefaultDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS, GenericType.BOOLEAN);
@@ -664,7 +679,16 @@ public String toString() {
   /** The coalescer reschedule interval. */
   public static final TypedDriverOption<Duration> COALESCER_INTERVAL =
       new TypedDriverOption<>(DefaultDriverOption.COALESCER_INTERVAL, GenericType.DURATION);
-  /** Whether to resolve the addresses passed to `basic.contact-points`. */
+  /**
+   * Whether to resolve the addresses passed to `basic.contact-points`.
+   *
+   * @deprecated Setting this option has no effect. Contact points given in the configuration are
+   *     now always kept as unresolved hostnames and expanded to all of their DNS-mapped IPs lazily
+   *     at connection time. This never applied to programmatic contact points passed to {@code
+   *     SessionBuilder.addContactPoints}, which are used exactly as supplied -- an already-resolved
+   *     address stays bound to that one IP.
+   */
+  @Deprecated
   public static final TypedDriverOption<Boolean> RESOLVE_CONTACT_POINTS =
       new TypedDriverOption<>(DefaultDriverOption.RESOLVE_CONTACT_POINTS, GenericType.BOOLEAN);
   /**