diff --git a/core/src/main/java/com/datastax/dse/driver/api/core/auth/DseGssApiAuthProviderBase.java b/core/src/main/java/com/datastax/dse/driver/api/core/auth/DseGssApiAuthProviderBase.java index 48a0e5b0ef3..beab2f488a8 100644 --- a/core/src/main/java/com/datastax/dse/driver/api/core/auth/DseGssApiAuthProviderBase.java +++ b/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. + * + *

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() { diff --git a/core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java b/core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java index 3a6e4ed69bb..0949da79443 100644 --- a/core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java +++ b/core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java @@ -701,8 +701,13 @@ 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}. + * + *

This is also the driver's DNS re-resolution path: contact points are expanded to their + * current DNS IPs at query-plan time, whereas metadata nodes 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. * *

Value-type: boolean */ @@ -837,7 +842,11 @@ public enum DefaultDriverOption implements DriverOption { * Whether to resolve the addresses passed to `basic.contact-points`. * *

Value-type: boolean + * + * @deprecated Contact points are now always kept as unresolved hostnames and expanded to all + * their DNS-mapped IPs lazily at connection time. Setting this option has no effect. */ + @Deprecated RESOLVE_CONTACT_POINTS("advanced.resolve-contact-points"), /** diff --git a/core/src/main/java/com/datastax/oss/driver/api/core/config/OptionsMap.java b/core/src/main/java/com/datastax/oss/driver/api/core/config/OptionsMap.java index 1d906caa985..5ea5039ae34 100644 --- a/core/src/main/java/com/datastax/oss/driver/api/core/config/OptionsMap.java +++ b/core/src/main/java/com/datastax/oss/driver/api/core/config/OptionsMap.java @@ -369,7 +369,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); diff --git a/core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java b/core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java index e412b99b404..a46757fb967 100644 --- a/core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java +++ b/core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java @@ -600,7 +600,15 @@ public String toString() { public static final TypedDriverOption 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}). + * + *

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 CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS = new TypedDriverOption<>( DefaultDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS, GenericType.BOOLEAN); @@ -664,7 +672,13 @@ public String toString() { /** The coalescer reschedule interval. */ public static final TypedDriverOption 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 Contact points are now always kept as unresolved hostnames and expanded to all + * their DNS-mapped IPs lazily at connection time. Setting this option has no effect. + */ + @Deprecated public static final TypedDriverOption RESOLVE_CONTACT_POINTS = new TypedDriverOption<>(DefaultDriverOption.RESOLVE_CONTACT_POINTS, GenericType.BOOLEAN); /** diff --git a/core/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.java b/core/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.java index 530f2ad38ac..af15682a0f3 100644 --- a/core/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.java +++ b/core/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.java @@ -18,24 +18,44 @@ package com.datastax.oss.driver.api.core.metadata; import edu.umd.cs.findbugs.annotations.NonNull; -import java.net.InetSocketAddress; import java.net.SocketAddress; /** * Encapsulates the information needed to open connections to a node. * *

By default, the driver assumes plain TCP connections, and this is just a wrapper around an - * {@link InetSocketAddress}. However, more complex deployment scenarios might use a custom + * {@link java.net.InetSocketAddress}. However, more complex deployment scenarios might use a custom * implementation that contains additional information; for example, if the nodes are accessed * through a proxy with SNI routing, an SNI server name is needed in addition to the proxy address. */ public interface EndPoint { /** - * Resolves this instance to a socket address. + * Resolves this instance to the socket address connections should be opened to. * *

This will be called each time the driver opens a new connection to the node. The returned * address cannot be null. + * + *

Returning a hostname is fine, and is how multi-address support works. The returned + * address need not be resolved: an {@linkplain java.net.InetSocketAddress#isUnresolved() + * unresolved} {@link java.net.InetSocketAddress} is expanded by the driver to every + * address the name maps to, and each one is tried in turn until a connection succeeds. That is + * what {@link com.datastax.oss.driver.internal.core.metadata.DefaultEndPoint} does for contact + * points backed by a hostname, so a single unreachable IP behind a multi-record name no longer + * fails the connection. + * + *

Implementations must not resolve names themselves, and must not block. The driver + * calls this from its admin event loop, and it performs the expansion through Netty's configured + * {@code AddressResolverGroup} — the same resolver an unresolved address reaches when it is + * handed to {@code Bootstrap.connect()}. Looking the name up here instead (for example with + * {@link java.net.InetAddress#getAllByName(String)}) would both block that loop and bypass a + * custom resolver installed via {@code NettyOptions#afterBootstrapInitialized(Bootstrap)}. + * + * @apiNote Timeout note: when a name expands to several addresses they are tried in + * sequence, so if every attempt times out the worst-case time before the node is declared + * unreachable is {@code N × advanced.connection.connect-timeout}. In practice DNS round-robin + * entries have only a small number of records, so this is rarely a concern, but it is worth + * bearing in mind when configuring connect timeouts. */ @NonNull SocketAddress resolve(); diff --git a/core/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.java b/core/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.java index 8375f0ef30b..720e3233fd1 100644 --- a/core/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.java +++ b/core/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.java @@ -166,11 +166,15 @@ protected DriverConfigLoader defaultConfigLoader(@Nullable ClassLoader classLoad *

Contact points can also be provided statically in the configuration. If both are specified, * they will be merged. If both are absent, the driver will default to 127.0.0.1:9042. * - *

Contrary to the configuration, DNS names with multiple A-records will not be handled here. - * If you need that, extract them manually with {@link java.net.InetAddress#getAllByName(String)} - * before calling this method. Similarly, if you need connect addresses to stay unresolved, make - * sure you pass unresolved instances here (see {@code advanced.resolve-contact-points} in the - * configuration for more explanations). + *

The driver automatically expands any contact point backed by an unresolved hostname to all + * its DNS-mapped IPs at connection time (through Netty's configured resolver, so a custom {@code + * AddressResolverGroup} still applies), so passing a single hostname is sufficient to try all its + * IPs on initial connect. This applies equally to hostnames provided here programmatically (build + * an unresolved {@link InetSocketAddress} with {@link InetSocketAddress#createUnresolved(String, + * int)} to opt in) and to hostnames specified in the configuration. An already-resolved address + * passed here (the common case when constructing an {@code InetSocketAddress} directly from a + * hostname, which resolves eagerly) is used as provided, with no further expansion. The {@code + * advanced.resolve-contact-points} option is deprecated and has no effect. */ @NonNull public SelfT addContactPoints(@NonNull Collection contactPoints) { @@ -741,6 +745,12 @@ public SelfT withCloudSecureConnectBundle(@NonNull InputStream cloudConfigInputS * *

For more information, please refer to the DataStax Astra documentation. * + *

A proxy given as a hostname is resolved at connection time, to all of its addresses, + * and each is tried in turn. That holds however the {@link InetSocketAddress} was built: the + * driver keeps a proxy hostname unresolved internally, so passing one that the ordinary {@code + * InetSocketAddress(String, int)} constructor already resolved does not bind the session to that + * single address. + * * @param cloudProxyAddress The address of the Cloud proxy to use. * @see Server Name Indication */ @@ -957,11 +967,10 @@ protected final CompletionStage buildDefaultSessionAsync() { programmaticArguments = programmaticArgumentsBuilder.build(); } - boolean resolveAddresses = - defaultConfig.getBoolean(DefaultDriverOption.RESOLVE_CONTACT_POINTS, false); - + // RESOLVE_CONTACT_POINTS is deprecated: contact points are always kept as unresolved + // hostnames, and expanded to all their DNS IPs at connection time by ChannelFactory. Set contactPoints = - ContactPoints.merge(programmaticContactPoints, configContactPoints, resolveAddresses); + ContactPoints.merge(programmaticContactPoints, configContactPoints, false); if (keyspace == null && defaultConfig.isDefined(DefaultDriverOption.SESSION_KEYSPACE)) { keyspace = diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.java b/core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.java index 6bfc355f910..a6d5db48d3f 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.java @@ -39,13 +39,18 @@ import com.datastax.oss.driver.internal.core.context.InternalDriverContext; import com.datastax.oss.driver.internal.core.context.NettyOptions; import com.datastax.oss.driver.internal.core.metadata.DefaultNode; +import com.datastax.oss.driver.internal.core.metadata.PinnableEndPoint; import com.datastax.oss.driver.internal.core.metrics.NodeMetricUpdater; import com.datastax.oss.driver.internal.core.metrics.NoopNodeMetricUpdater; import com.datastax.oss.driver.internal.core.metrics.SessionMetricUpdater; import com.datastax.oss.driver.internal.core.protocol.FrameDecoder; import com.datastax.oss.driver.internal.core.protocol.FrameEncoder; +import com.datastax.oss.driver.internal.core.util.AddressUtils; import com.datastax.oss.driver.shaded.guava.common.annotations.VisibleForTesting; import com.datastax.oss.driver.shaded.guava.common.base.Preconditions; +import com.datastax.oss.driver.shaded.guava.common.cache.CacheBuilder; +import com.datastax.oss.driver.shaded.guava.common.cache.CacheLoader; +import com.datastax.oss.driver.shaded.guava.common.cache.LoadingCache; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap; import com.datastax.oss.protocol.internal.ProtocolFeatures; import io.netty.bootstrap.Bootstrap; @@ -54,14 +59,27 @@ import io.netty.channel.ChannelInitializer; import io.netty.channel.ChannelOption; import io.netty.channel.ChannelPipeline; +import io.netty.channel.EventLoop; +import io.netty.resolver.AddressResolver; +import io.netty.resolver.AddressResolverGroup; +import io.netty.util.concurrent.Future; import java.io.IOException; +import java.net.Inet6Address; +import java.net.InetAddress; import java.net.InetSocketAddress; +import java.net.NetworkInterface; import java.net.ServerSocket; import java.net.SocketAddress; +import java.net.UnknownHostException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import java.util.concurrent.CompletionStage; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicBoolean; @@ -83,6 +101,7 @@ public class ChannelFactory { private static final String DATASTAX_CLOUD_PRODUCT_TYPE = "DATASTAX_APOLLO"; private static final AtomicBoolean LOGGED_ORPHAN_WARNING = new AtomicBoolean(); + private static final AtomicBoolean LOGGED_HANDLER_WARNING = new AtomicBoolean(); /** * A value for {@link #productType} that indicates that the server does not report any product @@ -90,6 +109,13 @@ public class ChannelFactory { */ private static final String UNKNOWN_PRODUCT_TYPE = "UNKNOWN"; + /** + * How many names {@link #rotationOffsets} tracks before it starts evicting. Generous next to the + * handful of names a session actually expands, and an evicted counter only costs that name a + * rotation restart. + */ + @VisibleForTesting static final int MAX_ROTATION_OFFSETS = 256; + // The names of the handlers on the pipeline: public static final String SSL_HANDLER_NAME = "ssl"; public static final String INBOUND_TRAFFIC_METER_NAME = "inboundTrafficMeter"; @@ -107,6 +133,32 @@ public class ChannelFactory { private final String logPrefix; protected final InternalDriverContext context; + /** + * Round-robin counters used by {@link #rotate} to vary which of a name's addresses a connection + * tries first, one counter per name. {@code SniEndPoint} used to hold an equivalent (single) + * counter of its own, before resolution moved here. + * + *

Per name rather than one global counter, because names whose expansions interleave in + * lockstep -- say two hostname contact points tried in sequence on every reconnection round -- + * would each only ever see one offset parity, pinning every name with an even record count to a + * fixed starting address. (The same failure mode once collapsed {@code SniEndPoint}'s rotation, + * when SSL engine setup shared its counter.) + * + *

Per factory, i.e. per session, and bounded on top of that: the names that reach here -- + * contact points, the SNI proxy name, client-route hostnames -- are not fixed for the lifetime of + * a JVM, or even of a session, since client routes can hand out different hostnames on every + * refresh. Spreading connections is only ever needed among the names a session is currently + * using, so nothing is lost by letting the rest go. + */ + @VisibleForTesting + final LoadingCache rotationOffsets = + CacheBuilder.newBuilder() + .maximumSize(MAX_ROTATION_OFFSETS) + .build(CacheLoader.from(name -> new AtomicInteger())); + + /** Fallback rotation counter for the odd original address that is not name-based. */ + private final AtomicInteger fallbackRotationOffset = new AtomicInteger(); + /** either set from the configuration, or null and will be negotiated */ @VisibleForTesting volatile ProtocolVersion protocolVersion; @@ -125,6 +177,7 @@ public ChannelFactory(InternalDriverContext context) { this.context = context; DriverExecutionProfile defaultConfig = context.getConfig().getDefaultProfile(); + if (defaultConfig.isDefined(DefaultDriverOption.PROTOCOL_VERSION)) { String versionName = defaultConfig.getString(DefaultDriverOption.PROTOCOL_VERSION); this.protocolVersion = context.getProtocolVersionRegistry().fromName(versionName); @@ -161,7 +214,7 @@ public CompletionStage connect(Node node, DriverChannelOptions op } else { nodeMetricUpdater = NoopNodeMetricUpdater.INSTANCE; } - return connect(node.getEndPoint(), null, null, options, nodeMetricUpdater); + return connect(node.getEndPoint(), null, null, options, nodeMetricUpdater, isIdentified(node)); } public CompletionStage connect( @@ -172,7 +225,24 @@ public CompletionStage connect( } else { nodeMetricUpdater = NoopNodeMetricUpdater.INSTANCE; } - return connect(node.getEndPoint(), node.getShardingInfo(), shardId, options, nodeMetricUpdater); + return connect( + node.getEndPoint(), + node.getShardingInfo(), + shardId, + options, + nodeMetricUpdater, + isIdentified(node)); + } + + /** + * Whether we know which node we are connecting to, as opposed to merely which address to + * try. {@link Node#getHostId()} is null only for an initial contact point, until the driver has + * read host ids from {@code system.local} and {@code system.peers} for the first time; every node + * discovered from those rows has one. {@link #tryNextCandidate} needs the distinction because an + * unidentified contact-point name may expand to addresses of different nodes. + */ + private static boolean isIdentified(Node node) { + return node.getHostId() != null; } @VisibleForTesting @@ -182,11 +252,23 @@ CompletionStage connect( Integer shardId, DriverChannelOptions options, NodeMetricUpdater nodeMetricUpdater) { + // A bare endpoint carries no host id, so this matches the contact-point case (see + // isIdentified()). + return connect(endPoint, shardingInfo, shardId, options, nodeMetricUpdater, false); + } + + @VisibleForTesting + CompletionStage connect( + EndPoint endPoint, + NodeShardingInfo shardingInfo, + Integer shardId, + DriverChannelOptions options, + NodeMetricUpdater nodeMetricUpdater, + boolean nodeIsIdentified) { CompletableFuture resultFuture = new CompletableFuture<>(); ProtocolVersion currentVersion; boolean isNegotiating; - List attemptedVersions = new CopyOnWriteArrayList<>(); if (this.protocolVersion != null) { currentVersion = protocolVersion; isNegotiating = false; @@ -203,7 +285,7 @@ CompletionStage connect( nodeMetricUpdater, currentVersion, isNegotiating, - attemptedVersions, + nodeIsIdentified, resultFuture); return resultFuture; } @@ -216,119 +298,633 @@ private void connect( NodeMetricUpdater nodeMetricUpdater, ProtocolVersion currentVersion, boolean isNegotiating, - List attemptedVersions, + boolean nodeIsIdentified, CompletableFuture resultFuture) { - SocketAddress resolvedAddress; + // Built once per connect() rather than once per candidate: it is the only handle on the Netty + // AddressResolverGroup (see resolveCandidates()), and it means the user's + // afterBootstrapInitialized() hook runs once per logical connection instead of once per address + // attempt. Each attempt gets its own clone() with its own handler. + // + // The event loop is likewise picked once per connect() and shared by name resolution and the + // channel itself (the per-attempt clones are bound to it, see connectToAddress()). Advancing + // the group's round-robin chooser exactly once per connect keeps channels evenly distributed: + // taking one loop for resolution and letting Bootstrap.connect() take another would advance + // the chooser twice per connect, parking all channels on half the loops with the default + // power-of-two chooser. It also mirrors what Netty itself does with an unresolved address: + // Bootstrap resolves on the connecting channel's own event loop. + Bootstrap baseBootstrap; + EventLoop eventLoop; try { - resolvedAddress = endPoint.resolve(); + baseBootstrap = newBootstrap(); + eventLoop = context.getNettyOptions().ioEventLoopGroup().next(); } catch (Exception e) { resultFuture.completeExceptionally(e); return; } - NettyOptions nettyOptions = context.getNettyOptions(); + // EndPoint.resolve() is contractually non-blocking and performs no name resolution, so it is + // safe to call here even though connect() runs on the admin event loop for control-connection + // reconnects. Everything a name needs to become connectable happens in resolveCandidates(). + SocketAddress address; + try { + address = endPoint.resolve(); + } catch (Exception e) { + resultFuture.completeExceptionally(e); + return; + } + if (address == null) { + // EndPoint.resolve() is contractually non-null; fail fast instead of NPE-ing inside an + // event-loop task later, which would leave resultFuture hanging (see resolveCandidates()). + resultFuture.completeExceptionally( + new IllegalArgumentException("EndPoint.resolve() returned null: " + endPoint)); + return; + } + + resolveCandidates(baseBootstrap, address, eventLoop) + .whenComplete( + (candidates, error) -> { + if (error != null) { + Throwable cause = + (error instanceof CompletionException && error.getCause() != null) + ? error.getCause() + : error; + resultFuture.completeExceptionally(cause); + return; + } + tryNextCandidate( + baseBootstrap, + eventLoop, + endPoint, + shardingInfo, + shardId, + options, + nodeMetricUpdater, + currentVersion, + isNegotiating, + nodeIsIdentified, + resultFuture, + candidates, + 0, + new ArrayList<>()); + }); + } + /** + * Builds the {@link Bootstrap} shared by every connection attempt of a single {@code connect()} + * call, including the user's {@link NettyOptions#afterBootstrapInitialized(Bootstrap)} hook. Per + * attempt, {@link #connectToAddress} takes a {@link + * Bootstrap#clone(io.netty.channel.EventLoopGroup)} of it bound to the event loop the connect + * picked, and installs its own handler; the copy carries the resolver configuration over. The + * base bootstrap itself keeps the full I/O group, so the hook observes the same group as always. + */ + private Bootstrap newBootstrap() { + NettyOptions nettyOptions = context.getNettyOptions(); Bootstrap bootstrap = new Bootstrap() .group(nettyOptions.ioEventLoopGroup()) .channel(nettyOptions.channelClass()) - .option(ChannelOption.ALLOCATOR, nettyOptions.allocator()) - .handler( - initializer(endPoint, currentVersion, options, nodeMetricUpdater, resultFuture)); - + .option(ChannelOption.ALLOCATOR, nettyOptions.allocator()); nettyOptions.afterBootstrapInitialized(bootstrap); + if (bootstrap.config().handler() != null && LOGGED_HANDLER_WARNING.compareAndSet(false, true)) { + LOG.warn( + "[{}] NettyOptions.afterBootstrapInitialized() installed a channel handler on the" + + " bootstrap; it will be replaced by the driver's own handler. Use" + + " NettyOptions.afterChannelInitialized() to customize the pipeline instead.", + logPrefix); + } + return bootstrap; + } - ChannelFuture connectFuture; - if (shardId == null || shardingInfo == null) { - if (shardId != null) { - LOG.debug( - "Requested connection to shard {} but shardingInfo is currently missing for Node at endpoint {}. Falling back to arbitrary local port.", - shardId, - endPoint); - } - connectFuture = bootstrap.connect(resolvedAddress); - } else { - int localPort = - PortAllocator.getNextAvailablePort(shardingInfo.getShardsCount(), shardId, context); - if (localPort == -1) { - LOG.warn( - "Could not find free port for shard {} at {}. Falling back to arbitrary local port.", - shardId, - endPoint); - connectFuture = bootstrap.connect(resolvedAddress); - } else { - connectFuture = bootstrap.connect(resolvedAddress, new InetSocketAddress(localPort)); - } + /** + * Turns the address an {@link EndPoint} denotes into the concrete, connectable addresses to try, + * expanding it to all the addresses it maps to when it is a name. + * + *

Expansion goes through the bootstrap's Netty {@link AddressResolverGroup} rather than a + * direct {@code InetAddress.getAllByName()} call, so a custom resolver installed via {@link + * NettyOptions#afterBootstrapInitialized(Bootstrap)} is honoured — that is the resolver an + * unresolved address would have reached had it been handed straight to {@code + * Bootstrap.connect()}, as it was before multi-address support. This is also why endpoints are + * forbidden from resolving names themselves (see {@link EndPoint#resolve()}): doing it here is + * the only way to keep that configuration point working, and the only way to keep {@code + * resolve()} non-blocking. + * + *

Whether an address needs resolving at all is the resolver's decision, not ours: exactly as + * in {@code Bootstrap#doResolveAndConnect0}, the address is passed through untouched only when + * the resolver says it does not {@linkplain AddressResolver#isSupported support} it (e.g. {@link + * io.netty.channel.local.LocalAddress}) or that it {@linkplain AddressResolver#isResolved is + * already resolved}. Both are overridable, and a custom resolver may well report an + * already-resolved address as unresolved in order to redirect it — Netty consulted it either way, + * so a pre-check here on {@code InetSocketAddress#isUnresolved()} would silently take that + * configuration point away for every connect to an already-resolved node, which is to say for + * almost every connect. A null group means the user called {@link Bootstrap#disableResolver()}, + * which is likewise respected. + * + *

Note that with Netty's default resolver the lookup blocks the event loop it runs on, + * because {@code DefaultNameResolver} performs {@code InetAddress.getAllByName()} inline. That is + * the pre-existing behaviour of handing an unresolved address to {@code Bootstrap.connect()}, and + * it is an I/O loop, never the admin loop that {@code connect()} is called from. Deployments that + * need non-blocking resolution can now install {@code DnsAddressResolverGroup} and have it take + * effect. + */ + private CompletionStage> resolveCandidates( + Bootstrap bootstrap, SocketAddress address, EventLoop eventLoop) { + + AddressResolverGroup resolverGroup = bootstrap.config().resolver(); + if (resolverGroup == null) { + // Bootstrap.disableResolver(): the user wants the address passed through as-is. + return CompletableFuture.completedFuture(Collections.singletonList(address)); } - connectFuture.addListener( - cf -> { - if (connectFuture.isSuccess()) { - Channel channel = connectFuture.channel(); - DriverChannel driverChannel = - new DriverChannel(endPoint, channel, context.getWriteCoalescer(), currentVersion); - // If this is the first successful connection, remember the protocol version and - // cluster name for future connections. - if (isNegotiating) { - ChannelFactory.this.protocolVersion = currentVersion; - } - if (ChannelFactory.this.clusterName == null) { - ChannelFactory.this.clusterName = driverChannel.getClusterName(); - } - Map> supportedOptions = driverChannel.getOptions(); - if (ChannelFactory.this.productType == null && supportedOptions != null) { - List productTypes = supportedOptions.get("PRODUCT_TYPE"); - String productType = - productTypes != null && !productTypes.isEmpty() - ? productTypes.get(0) - : UNKNOWN_PRODUCT_TYPE; - ChannelFactory.this.productType = productType; - DriverConfig driverConfig = context.getConfig(); - if (driverConfig instanceof TypesafeDriverConfig - && productType.equals(DATASTAX_CLOUD_PRODUCT_TYPE)) { - ((TypesafeDriverConfig) driverConfig) - .overrideDefaults( - ImmutableMap.of( - DefaultDriverOption.REQUEST_CONSISTENCY, - ConsistencyLevel.LOCAL_QUORUM.name())); + // The supplied event loop is the same one the channel will be registered on (see connect()), + // which is what Netty itself does with an unresolved address: Bootstrap resolves on the + // connecting channel's own event loop. Its transport also matches the channel class, which + // matters because DnsAddressResolverGroup registers a datagram channel on the executor it + // resolves for. + CompletableFuture> result = new CompletableFuture<>(); + // Every path below must complete `result`: nothing at this stage has a timeout, so a task or + // listener that dies with the future still pending (Netty swallows their throwables, it only + // logs them) would hang the connect attempt -- and with it control-connection init or a pool + // reconnect -- forever. Hence the blanket catches around the task body, the listener body, and + // the execute() call itself (which throws RejectedExecutionException while shutting down). + try { + eventLoop.execute( + () -> { + try { + AddressResolver resolver = + resolverGroup.getResolver(eventLoop); + if (!resolver.isSupported(address) || resolver.isResolved(address)) { + // Nothing for the resolver to do; same short-circuit as + // Bootstrap#doResolveAndConnect0. + result.complete(Collections.singletonList(address)); + return; } + resolver + .resolveAll(address) + .addListener( + (Future> future) -> { + try { + if (!future.isSuccess()) { + result.completeExceptionally(future.cause()); + return; + } + @SuppressWarnings("unchecked") + List addresses = + (List) future.getNow(); + if (addresses == null || addresses.isEmpty()) { + result.completeExceptionally( + new IllegalStateException( + "Resolver returned no address for " + address)); + return; + } + result.complete(rotate(address, reattachHostnames(address, addresses))); + } catch (Throwable t) { + result.completeExceptionally(t); + } + }); + } catch (Throwable t) { + result.completeExceptionally(t); } - resultFuture.complete(driverChannel); - } else { - Throwable error = connectFuture.cause(); - if (error instanceof UnsupportedProtocolVersionException && isNegotiating) { - attemptedVersions.add(currentVersion); - Optional downgraded = - context.getProtocolVersionRegistry().downgrade(currentVersion); - if (downgraded.isPresent()) { + }); + } catch (Throwable t) { + result.completeExceptionally(t); + } + return result; + } + + /** Applies {@link #reattachHostname} to every expanded candidate. */ + private static List reattachHostnames( + SocketAddress original, List candidates) { + List result = new ArrayList<>(candidates.size()); + for (SocketAddress candidate : candidates) { + result.add(reattachHostname(original, candidate)); + } + return result; + } + + /** + * Re-attaches the {@code original} address's host name to one of the resolved candidates it + * expanded to, whatever name that candidate carries. + * + *

The JDK and Netty-DNS resolvers already attach the queried name to the {@link InetAddress}es + * they return, so this is a no-op for them. A custom resolver, however, may build its results + * from raw address bytes, or label them with a canonical/CNAME name of its own. The channel's + * pinned endpoint is built from the candidate (see {@link PinnableEndPoint}), and it is what + * {@code DefaultSslEngineFactory} and {@code SniSslEngineFactory} derive the SSL peer host from, + * inside the channel initializer. So whatever name the candidate carries is the name TLS hostname + * verification checks the server certificate against, and the only name that may be is the one + * the user configured: with a nameless address, {@code InetSocketAddress#getHostName()} + * additionally triggers a blocking reverse-DNS lookup on the event loop and validation falls back + * to the IP or the PTR record, and with a resolver-supplied label it validates a name the + * operator never chose. Hence the queried name always wins here; before multi-address support the + * initializer kept the original endpoint and Netty resolved only the TCP destination, which had + * the same effect. + * + *

Re-attaching changes nothing else: {@code InetAddress.getByAddress(host, bytes)} performs no + * lookup, the TCP connect target is the same IP, and a resolved {@link InetSocketAddress}'s + * equality ignores host names, so pinning and the pin-equality shortcuts are unaffected. It also + * makes {@link #rotate} more deterministic, not less: with one uniform name across an expansion, + * its {@code toString()} sort depends only on the IP and port. A scoped IPv6 candidate keeps its + * scope, since {@link Inet6Address} has {@code getByAddress} overloads that carry one. + * + *

An original that carries no name of its own is left alone (see {@link + * AddressUtils#carriesName}): a resolver is free to redirect it to a different IP, and labelling + * that IP with the literal form of the one we asked for would invent a name that resolves to + * something else. + */ + @VisibleForTesting + static SocketAddress reattachHostname(SocketAddress original, SocketAddress candidate) { + if (!(original instanceof InetSocketAddress) || !(candidate instanceof InetSocketAddress)) { + return candidate; + } + InetSocketAddress originalInet = (InetSocketAddress) original; + InetSocketAddress candidateInet = (InetSocketAddress) candidate; + InetAddress candidateIp = candidateInet.getAddress(); + if (!AddressUtils.carriesName(originalInet) + || candidateIp == null + // Nothing to change: the candidate already carries the queried name, which is the common + // case (the JDK and Netty-DNS resolvers attach it themselves). getHostString() never looks + // anything up -- for a nameless address it falls back to the IP literal. + || candidateInet.getHostString().equals(originalInet.getHostString())) { + return candidate; + } + try { + return new InetSocketAddress( + withHostName(originalInet.getHostString(), candidateIp), candidateInet.getPort()); + } catch (UnknownHostException impossible) { + // getByAddress only rejects illegal byte lengths, and these bytes come from a real + // InetAddress; keep the raw candidate rather than failing the connect over a cosmetic step. + return candidate; + } + } + + /** + * Returns a copy of {@code ip} labelled with {@code hostName}, preserving an IPv6 scope if there + * is one. + * + *

{@link InetAddress#getByAddress(String, byte[])} cannot carry a scope, and dropping one + * would change where the address actually points — a link-local address is only meaningful + * together with its zone. {@link Inet6Address#getByAddress(String, byte[], int)} carries the zone + * as its numeric id, which is what the connect itself goes on; a scope id of 0 means "unscoped" + * and is accepted, so this needs no special case for a plain IPv6 address. + * + *

The sibling overload taking a {@link NetworkInterface} is deliberately not used: it + * re-derives the numeric scope by searching that interface for an address of the same local type, + * and throws {@code UnknownHostException("no scope_id found")} when it finds none — so it can + * fail for an address that was legitimately built from an interface in the first place. All that + * is lost by going numeric is the interface name, which surfaces in {@code toString()} and + * nowhere else. + */ + private static InetAddress withHostName(String hostName, InetAddress ip) + throws UnknownHostException { + return ip instanceof Inet6Address + ? Inet6Address.getByAddress(hostName, ip.getAddress(), ((Inet6Address) ip).getScopeId()) + : InetAddress.getByAddress(hostName, ip.getAddress()); + } + + /** + * Rotates the expanded address list so that successive connections to the same name do not all + * start at the same address. + * + *

Without this, every connection would try the resolver's first address first and healthy + * connections would pile onto one IP; the whole point of a multi-record name is usually to spread + * them. All addresses are still returned, and in the same cyclic order, so a single attempt can + * still fall back across every one of them. + * + *

The list is sorted first so that a given rotation offset maps to the same address on every + * call, regardless of the order the resolver happened to return. Only that determinism matters + * here, not the ordering itself, so a plain string comparison is enough. + * + *

The offset is tracked per name -- {@code original} is the address the expansion was queried + * for -- so that different names rotate independently (see {@link #rotationOffsets}). + */ + @VisibleForTesting + List rotate(SocketAddress original, List addresses) { + int size = addresses.size(); + if (size == 1) { + // Nothing to rotate, and don't burn a rotation offset (or create a counter) for it. + return new ArrayList<>(addresses); + } + List sorted = new ArrayList<>(addresses); + sorted.sort(Comparator.comparing(SocketAddress::toString)); + int start = Math.floorMod(rotationOffsetFor(original).getAndIncrement(), size); + List result = new ArrayList<>(size); + for (int i = 0; i < size; i++) { + result.add(sorted.get((start + i) % size)); + } + return result; + } + + private AtomicInteger rotationOffsetFor(SocketAddress original) { + if (original instanceof InetSocketAddress) { + // DNS names are case-insensitive; normalize so the same name shares one counter. + String name = ((InetSocketAddress) original).getHostString().toLowerCase(Locale.ROOT); + return rotationOffsets.getUnchecked(name); + } + return fallbackRotationOffset; + } + + /** + * Iterates through the candidate addresses produced by {@link #resolveCandidates}. Tries each one + * in sequence; when an address fails, the next candidate is tried, and only when all candidates + * are exhausted is the overall {@code resultFuture} failed. + * + *

The one exception is a {@link UnsupportedProtocolVersionException} against an + * identified node ({@code nodeIsIdentified}, see {@link #isIdentified(Node)}): a + * protocol-version rejection -- whether negotiation exhausted every downgrade or the server + * refused a forced version -- is a property of the node, and every address of a node we have + * already identified is that same node, so the attempt fails immediately instead of replaying the + * whole negotiation ladder against every remaining IP (worst case {@code N × versions × + * connect-timeout} for nothing). This matches the pre-multi-address behaviour of a single-address + * connect. The corner it deliberately does not rescue: a heterogeneous rolling upgrade where + * different IPs of one identified node genuinely support different protocol versions. + * + *

For an unidentified endpoint -- a contact point, before the driver has read host ids + * -- the addresses a name expands to may well belong to different nodes, so a rejection by the + * first of them says nothing about the rest and the loop keeps going. That also preserves the + * behaviour this PR would otherwise have removed: with {@code advanced.resolve-contact-points = + * true} each resolved address used to be a separate {@code Node}, and {@code ControlConnection} + * advances to the next node in its query plan on any error, including this one. + * + *

Other failures -- TCP, init, authentication -- always advance to the next candidate, since + * with a multi-record name they may well be address-specific. + * + *

Timeout note: addresses are tried serially, so the worst-case time before failure is + * {@code N × connect-timeout} where N is the number of candidates. This is an intentional + * tradeoff: failing immediately on the first unreachable IP would prevent fallback to healthy + * ones. In practice DNS entries have only a small number of records. + * + *

When every candidate fails, the last candidate's error is propagated with each earlier + * candidate's failure attached as a {@linkplain Throwable#addSuppressed(Throwable) suppressed} + * exception, so the full set of per-address causes is visible for diagnosis (they are otherwise + * only logged at DEBUG). + */ + private void tryNextCandidate( + Bootstrap baseBootstrap, + EventLoop eventLoop, + EndPoint endPoint, + NodeShardingInfo shardingInfo, + Integer shardId, + DriverChannelOptions options, + NodeMetricUpdater nodeMetricUpdater, + ProtocolVersion currentVersion, + boolean isNegotiating, + boolean nodeIsIdentified, + CompletableFuture resultFuture, + List candidates, + int index, + List priorErrors) { + + // Invariant: this method always (eventually) completes resultFuture. It is invoked from + // CompletionStage and Netty callbacks that swallow throwables, so a synchronous throw -- a + // custom PinnableEndPoint.pinTo() for instance -- would otherwise leave the connect attempt + // hanging forever. Double completion is harmless: completeExceptionally() on an already + // completed future is a no-op. + try { + SocketAddress candidate = candidates.get(index); + // Everything downstream of here -- the channel, its pipeline (SSL engine, authenticator) and + // the DriverChannel handed to the caller -- sees an endpoint bound to this one address + // instead of the multi-address original. See PinnableEndPoint for why that matters. + EndPoint pinnedEndPoint = pin(endPoint, candidate); + CompletableFuture perAddressFuture = new CompletableFuture<>(); + // Fresh per candidate address: connectToAddress()'s downgrade retries stay on this one + // address, so the final UnsupportedProtocolVersionException (if negotiation is what dooms + // this candidate) only reports versions actually tried against it, not earlier candidates'. + List attemptedVersions = new CopyOnWriteArrayList<>(); + connectToAddress( + baseBootstrap, + eventLoop, + pinnedEndPoint, + shardingInfo, + shardId, + options, + nodeMetricUpdater, + currentVersion, + isNegotiating, + attemptedVersions, + perAddressFuture, + candidate); + + perAddressFuture.whenComplete( + (channel, error) -> { + try { + if (error == null) { + resultFuture.complete(channel); + } else if (!isNodeWideFailure(error, nodeIsIdentified) + && index + 1 < candidates.size()) { LOG.debug( - "[{}] Failed to connect with protocol {}, retrying with {}", + "[{}] Failed to connect to {} ({}), trying next address", logPrefix, - currentVersion, - downgraded.get()); - connect( + candidate, + error.getMessage()); + priorErrors.add(error); + tryNextCandidate( + baseBootstrap, + eventLoop, + // Deliberately the original, not the pinned copy: the next candidate must be + // pinned from the unpinned endpoint. endPoint, shardingInfo, shardId, options, nodeMetricUpdater, - downgraded.get(), - true, - attemptedVersions, - resultFuture); + currentVersion, + isNegotiating, + nodeIsIdentified, + resultFuture, + candidates, + index + 1, + priorErrors); } else { - resultFuture.completeExceptionally( - UnsupportedProtocolVersionException.forNegotiation( - endPoint, attemptedVersions)); + if (index + 1 < candidates.size()) { + // Only reachable for a node-wide failure (see the javadoc). + LOG.debug( + "[{}] Not trying the remaining addresses of {}: a protocol-version rejection" + + " is a property of the node, not of the address ({})", + logPrefix, + endPoint, + error.getMessage()); + } + // Surface the last error, carrying the earlier failures as suppressed exceptions + // so they are not lost (they were only logged at DEBUG above). + for (Throwable priorError : priorErrors) { + if (priorError != error) { + error.addSuppressed(priorError); + } + } + // Note: might be completed already if the failure happened in initializer() + resultFuture.completeExceptionally(error); } - } else { - // Note: might be completed already if the failure happened in initializer(), this is - // fine - resultFuture.completeExceptionally(error); + } catch (Throwable t) { + resultFuture.completeExceptionally(t); } - } - }); + }); + } catch (Throwable t) { + resultFuture.completeExceptionally(t); + } + } + + /** + * Whether {@code error} dooms every remaining address of the endpoint, making it pointless for + * {@link #tryNextCandidate} to try them. See its javadoc for why this is limited to a + * protocol-version rejection against an identified node. + */ + private static boolean isNodeWideFailure(Throwable error, boolean nodeIsIdentified) { + return nodeIsIdentified && error instanceof UnsupportedProtocolVersionException; + } + + /** + * Performs a Netty bootstrap connect to a single, already-resolved address. Handles + * protocol-version negotiation (downgrade retries) internally, staying on the same address. Uses + * {@code perAddressFuture} so {@link #tryNextCandidate} can distinguish a per-address TCP failure + * (try the next IP) from a successful protocol handshake. + */ + private void connectToAddress( + Bootstrap baseBootstrap, + EventLoop eventLoop, + EndPoint endPoint, + NodeShardingInfo shardingInfo, + Integer shardId, + DriverChannelOptions options, + NodeMetricUpdater nodeMetricUpdater, + ProtocolVersion currentVersion, + boolean isNegotiating, + List attemptedVersions, + CompletableFuture perAddressFuture, + SocketAddress resolvedAddress) { + + // Invariant, as in tryNextCandidate(): every path completes perAddressFuture. The synchronous + // section can throw from Bootstrap validation; the connect listener runs inside a Netty + // callback that swallows throwables and contains the downgrade recursion, the version-registry + // lookup and the config overrides, any of which throwing would otherwise hang the attempt. + try { + // clone(eventLoop) so each attempt gets its own handler while sharing the options and + // resolver configuration (including anything afterBootstrapInitialized() set), and is + // registered on the event loop the connect() picked -- the same one resolution ran on, so + // the group's chooser advances exactly once per logical connect (see connect()). + Bootstrap bootstrap = + baseBootstrap + .clone(eventLoop) + .handler( + initializer( + endPoint, currentVersion, options, nodeMetricUpdater, perAddressFuture)); + + ChannelFuture connectFuture; + if (shardId == null || shardingInfo == null) { + if (shardId != null) { + LOG.debug( + "Requested connection to shard {} but shardingInfo is currently missing for Node at endpoint {}. Falling back to arbitrary local port.", + shardId, + endPoint); + } + connectFuture = bootstrap.connect(resolvedAddress); + } else { + int localPort = + PortAllocator.getNextAvailablePort(shardingInfo.getShardsCount(), shardId, context); + if (localPort == -1) { + LOG.warn( + "Could not find free port for shard {} at {}. Falling back to arbitrary local port.", + shardId, + endPoint); + connectFuture = bootstrap.connect(resolvedAddress); + } else { + connectFuture = bootstrap.connect(resolvedAddress, new InetSocketAddress(localPort)); + } + } + + connectFuture.addListener( + cf -> { + try { + if (connectFuture.isSuccess()) { + Channel channel = connectFuture.channel(); + DriverChannel driverChannel = + new DriverChannel( + endPoint, channel, context.getWriteCoalescer(), currentVersion); + // If this is the first successful connection, remember the protocol version and + // cluster name for future connections. + if (isNegotiating) { + ChannelFactory.this.protocolVersion = currentVersion; + } + if (ChannelFactory.this.clusterName == null) { + ChannelFactory.this.clusterName = driverChannel.getClusterName(); + } + Map> supportedOptions = driverChannel.getOptions(); + if (ChannelFactory.this.productType == null && supportedOptions != null) { + List productTypes = supportedOptions.get("PRODUCT_TYPE"); + String productType = + productTypes != null && !productTypes.isEmpty() + ? productTypes.get(0) + : UNKNOWN_PRODUCT_TYPE; + ChannelFactory.this.productType = productType; + DriverConfig driverConfig = context.getConfig(); + if (driverConfig instanceof TypesafeDriverConfig + && productType.equals(DATASTAX_CLOUD_PRODUCT_TYPE)) { + ((TypesafeDriverConfig) driverConfig) + .overrideDefaults( + ImmutableMap.of( + DefaultDriverOption.REQUEST_CONSISTENCY, + ConsistencyLevel.LOCAL_QUORUM.name())); + } + } + perAddressFuture.complete(driverChannel); + } else { + Throwable error = connectFuture.cause(); + if (error instanceof UnsupportedProtocolVersionException && isNegotiating) { + attemptedVersions.add(currentVersion); + Optional downgraded = + context.getProtocolVersionRegistry().downgrade(currentVersion); + if (downgraded.isPresent()) { + LOG.debug( + "[{}] Failed to connect with protocol {}, retrying with {}", + logPrefix, + currentVersion, + downgraded.get()); + // Stay on the same address for protocol-version downgrade retries. + connectToAddress( + baseBootstrap, + eventLoop, + endPoint, + shardingInfo, + shardId, + options, + nodeMetricUpdater, + downgraded.get(), + true, + attemptedVersions, + perAddressFuture, + resolvedAddress); + } else { + perAddressFuture.completeExceptionally( + UnsupportedProtocolVersionException.forNegotiation( + endPoint, attemptedVersions)); + } + } else { + // Note: might be completed already if the failure happened in initializer(), this + // is fine + perAddressFuture.completeExceptionally(error); + } + } + } catch (Throwable t) { + perAddressFuture.completeExceptionally(t); + } + }); + } catch (Throwable t) { + perAddressFuture.completeExceptionally(t); + } + } + + /** + * Binds {@code endPoint} to the address a connection is being opened to, when the implementation + * supports it. + * + *

Third-party {@link EndPoint}s that do not implement {@link PinnableEndPoint} are returned + * unchanged, so they keep behaving exactly as they did before multi-address support: the channel + * carries the endpoint it was given. + */ + private static EndPoint pin(EndPoint endPoint, SocketAddress resolvedAddress) { + return endPoint instanceof PinnableEndPoint + ? ((PinnableEndPoint) endPoint).pinTo(resolvedAddress) + : endPoint; } @VisibleForTesting diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/context/NettyOptions.java b/core/src/main/java/com/datastax/oss/driver/internal/core/context/NettyOptions.java index 5b4ff4dcec8..b319186910d 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/context/NettyOptions.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/context/NettyOptions.java @@ -66,7 +66,21 @@ public interface NettyOptions { /** * A hook invoked each time the driver creates a client bootstrap in order to open a channel. This - * is a good place to configure any custom option on the bootstrap. + * is a good place to configure any custom option, attribute, or {@link + * Bootstrap#resolver(io.netty.resolver.AddressResolverGroup)} on the bootstrap. + * + *

The hook runs once per logical connection to a node. When a hostname expands to several IP + * addresses, the same bootstrap is shared by every per-address attempt (each attempt uses a + * {@link Bootstrap#clone(io.netty.channel.EventLoopGroup)} of it); likewise, protocol-version + * downgrade retries reuse it. Before multi-address support the hook ran once per attempt, + * including once per downgrade retry. + * + *

The bootstrap does not carry the driver's channel handler yet, and a handler + * installed by this hook is not honoured: the driver sets its own handler on each + * per-attempt copy afterwards (and logs a one-time warning if it overwrites one). To customize + * the pipeline, use {@link #afterChannelInitialized(Channel)} instead. (Before multi-address + * support the hook ran after the driver's handler was installed, so replacing it was technically + * possible; that was never a supported extension point.) */ void afterBootstrapInitialized(Bootstrap bootstrap); diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/helper/OptionalLocalDcHelper.java b/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/helper/OptionalLocalDcHelper.java index b93a16a6525..97aab92fff7 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/helper/OptionalLocalDcHelper.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/helper/OptionalLocalDcHelper.java @@ -26,7 +26,6 @@ import edu.umd.cs.findbugs.annotations.NonNull; import java.util.ArrayList; import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -68,73 +67,34 @@ public OptionalLocalDcHelper( @Override @NonNull public Optional discoverLocalDc(@NonNull Map nodes) { - String localDcStr = context.getLocalDatacenter(profile.getName()); - Optional localDc; - if (localDcStr != null) { - LOG.debug("[{}] Local DC set programmatically: {}", logPrefix, localDcStr); - localDc = Optional.of(localDcStr); + String localDc = context.getLocalDatacenter(profile.getName()); + if (localDc != null) { + LOG.debug("[{}] Local DC set programmatically: {}", logPrefix, localDc); } else if (profile.isDefined(DefaultDriverOption.LOAD_BALANCING_LOCAL_DATACENTER)) { - localDcStr = profile.getString(DefaultDriverOption.LOAD_BALANCING_LOCAL_DATACENTER); - LOG.debug("[{}] Local DC set from configuration: {}", logPrefix, localDcStr); - localDc = Optional.of(localDcStr); - } else { - localDc = Optional.empty(); - } - if (localDc.isPresent()) { - checkLocalDatacenterCompatibility( - localDc.get(), context.getMetadataManager().getContactPoints()); - // Also warn if the configured DC doesn't match any node in the cluster - if (!nodes.isEmpty()) { - boolean found = false; - for (Node node : nodes.values()) { - if (localDc.get().equals(node.getDatacenter())) { - found = true; - break; - } - } - if (!found) { - LOG.warn( - "[{}] Configured local DC '{}' does not match any node's datacenter" - + " (available DCs: {}); please verify your configuration", - logPrefix, - localDc.get(), - formatDcs(nodes.values())); - } - } + localDc = profile.getString(DefaultDriverOption.LOAD_BALANCING_LOCAL_DATACENTER); + LOG.debug("[{}] Local DC set from configuration: {}", logPrefix, localDc); } else { LOG.debug("[{}] Local DC not set, DC awareness will be disabled", logPrefix); + return Optional.empty(); } - return localDc; - } - - /** - * Checks if the contact points are compatible with the local datacenter specified either through - * configuration, or programmatically. - * - *

The default implementation logs a warning when a contact point reports a datacenter - * different from the local one, and only for the default profile. - * - * @param localDc The local datacenter, as specified in the config, or programmatically. - * @param contactPoints The contact points provided when creating the session. - */ - protected void checkLocalDatacenterCompatibility( - @NonNull String localDc, Set contactPoints) { - if (profile.getName().equals(DriverExecutionProfile.DEFAULT_NAME)) { - Set badContactPoints = new LinkedHashSet<>(); - for (Node node : contactPoints) { - if (!Objects.equals(localDc, node.getDatacenter())) { - badContactPoints.add(node); + if (!nodes.isEmpty()) { + boolean found = false; + for (Node node : nodes.values()) { + if (localDc.equals(node.getDatacenter())) { + found = true; + break; } } - if (!badContactPoints.isEmpty()) { + if (!found) { LOG.warn( - "[{}] You specified {} as the local DC, but some contact points are from a different DC: {}; " - + "please provide the correct local DC, or check your contact points", + "[{}] Configured local DC '{}' does not match any node's datacenter" + + " (available DCs: {}); please verify your configuration", logPrefix, localDc, - formatNodesAndDcs(badContactPoints)); + formatDcs(nodes.values())); } } + return Optional.of(localDc); } /** diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPoint.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPoint.java index 15d825b2efc..4d495ec5303 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPoint.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPoint.java @@ -20,19 +20,26 @@ import com.datastax.oss.driver.api.core.metadata.EndPoint; import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; -import java.io.IOException; -import java.io.UncheckedIOException; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.SocketAddress; import java.util.Objects; import java.util.UUID; -public class ClientRoutesEndPoint implements EndPoint { +public class ClientRoutesEndPoint implements PinnableEndPoint { private final UUID hostId; private final ClientRoutesTopologyMonitor topologyMonitor; private final String metricPrefix; @NonNull private final EndPoint fallbackEndPoint; + /** Kept only so that {@link #pinTo(SocketAddress)} can rebuild an identical copy. */ + @Nullable private final InetAddress broadcastInetAddress; + + /** + * The address this endpoint has been {@linkplain #pinTo(SocketAddress) pinned} to, or {@code + * null} if it is not pinned. Deliberately excluded from {@link #equals} and {@link #hashCode}, + * which key off the host id alone. + */ + @Nullable private final InetSocketAddress pinnedAddress; /** * @param topologyMonitor the topology monitor used to resolve the endpoint address on demand. @@ -49,12 +56,23 @@ public ClientRoutesEndPoint( @NonNull UUID hostId, @Nullable InetAddress broadcastInetAddress, @NonNull EndPoint fallbackEndPoint) { + this(topologyMonitor, hostId, broadcastInetAddress, fallbackEndPoint, null); + } + + private ClientRoutesEndPoint( + @NonNull ClientRoutesTopologyMonitor topologyMonitor, + @NonNull UUID hostId, + @Nullable InetAddress broadcastInetAddress, + @NonNull EndPoint fallbackEndPoint, + @Nullable InetSocketAddress pinnedAddress) { this.topologyMonitor = Objects.requireNonNull(topologyMonitor, "Topology monitor cannot be null"); this.hostId = Objects.requireNonNull(hostId, "HOST uuid cannot be null"); this.fallbackEndPoint = Objects.requireNonNull(fallbackEndPoint, "Fallback endpoint cannot be null"); this.metricPrefix = buildMetricPrefix(broadcastInetAddress, hostId); + this.broadcastInetAddress = broadcastInetAddress; + this.pinnedAddress = pinnedAddress; } @NonNull @@ -62,18 +80,48 @@ public UUID getHostId() { return hostId; } + /** + * Returns the address connections should be opened to. + * + *

The client route for this host id is an in-memory lookup over the cached {@code + * system.client_routes} contents, and it yields exactly one address by design, so this neither + * blocks nor expands to several candidates. The route's hostname is returned {@linkplain + * InetSocketAddress#isUnresolved() unresolved}: {@link + * com.datastax.oss.driver.internal.core.channel.ChannelFactory} resolves it through Netty's + * configured {@code AddressResolverGroup}, so a custom resolver is honoured and no DNS lookup + * runs on the caller (the admin event loop, for control-connection reconnects). + * + *

When the topology monitor has no route for this host id — i.e. the node is not reached + * through a cloud private endpoint — this delegates to the fallback endpoint. + * + *

Once {@linkplain #pinTo(SocketAddress) pinned} the pinned address is returned directly. + */ @NonNull @Override public SocketAddress resolve() { - try { - InetSocketAddress address = topologyMonitor.resolve(hostId); - if (address != null) { - return address; - } - } catch (IOException e) { - throw new UncheckedIOException("DNS resolution failed for host_id=" + hostId, e); + if (pinnedAddress != null) { + return pinnedAddress; + } + InetSocketAddress address = topologyMonitor.resolve(hostId); + return address != null ? address : fallbackEndPoint.resolve(); + } + + @NonNull + @Override + public EndPoint pinTo(@NonNull SocketAddress resolvedAddress) { + Objects.requireNonNull(resolvedAddress, "resolvedAddress cannot be null"); + // Mirror DefaultEndPoint: an address we cannot hold in an InetSocketAddress field skips + // pinning rather than failing the connection. + if (!(resolvedAddress instanceof InetSocketAddress) + || resolvedAddress.equals(this.pinnedAddress)) { + return this; } - return fallbackEndPoint.resolve(); + return new ClientRoutesEndPoint( + topologyMonitor, + hostId, + broadcastInetAddress, + fallbackEndPoint, + (InetSocketAddress) resolvedAddress); } @Override diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesTopologyMonitor.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesTopologyMonitor.java index 1ffc35fd9f4..31e476d866d 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesTopologyMonitor.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesTopologyMonitor.java @@ -21,6 +21,7 @@ import com.datastax.oss.driver.api.core.config.ClientRoutesConfig; import com.datastax.oss.driver.api.core.config.DefaultDriverOption; import com.datastax.oss.driver.api.core.metadata.EndPoint; +import com.datastax.oss.driver.api.core.metadata.Node; import com.datastax.oss.driver.internal.core.adminrequest.AdminRequestHandler; import com.datastax.oss.driver.internal.core.adminrequest.AdminResult; import com.datastax.oss.driver.internal.core.adminrequest.AdminRow; @@ -32,7 +33,6 @@ import edu.umd.cs.findbugs.annotations.Nullable; import java.net.InetAddress; import java.net.InetSocketAddress; -import java.net.UnknownHostException; import java.time.Duration; import java.util.ArrayList; import java.util.Collections; @@ -195,9 +195,18 @@ void setResolvedRoutes(Map routes) { resolvedRoutesCache.set(Collections.unmodifiableMap(new HashMap<>(routes))); } + /** + * Returns the client route for {@code hostId} as an {@linkplain InetSocketAddress#isUnresolved() + * unresolved} address, or {@code null} if this node has no route. + * + *

The route's hostname is deliberately left unresolved: {@link + * com.datastax.oss.driver.internal.core.channel.ChannelFactory} resolves it through Netty's + * configured {@code AddressResolverGroup} at connection time. That keeps this method a pure + * in-memory cache lookup, so it is safe to call from an event loop, and it means a custom + * resolver applies to client routes just like it does to contact points. + */ @Nullable - public InetSocketAddress resolve(@NonNull UUID hostId) - throws IllegalStateException, UnknownHostException { + public InetSocketAddress resolve(@NonNull UUID hostId) throws IllegalStateException { if (closed) { throw new IllegalStateException("Topology monitor is closed"); } @@ -206,7 +215,7 @@ public InetSocketAddress resolve(@NonNull UUID hostId) return null; // no client route for this node — caller falls back to default } - return new InetSocketAddress(resolveAddress(route.getHostname()), route.getPort()); + return InetSocketAddress.createUnresolved(route.getHostname(), route.getPort()); } /** @@ -480,6 +489,22 @@ protected EndPoint buildNodeEndPoint( return new ClientRoutesEndPoint(this, hostId, broadcastInetAddress, fallback); } + @Override + public boolean reresolvesNodeAddresses() { + // ClientRoutesEndPoint re-resolves via the client route hostname on every connection attempt, + // but only when a route exists for that host_id (see ClientRoutesEndPoint#resolve()) -- for + // mixed/incomplete route sets it falls back to a static, non-re-resolving endpoint instead. + // Only report true when every currently-known node actually has a live route; otherwise the + // contact-point reconnection fallback must stay available for the nodes stuck on the fallback. + Map routes = resolvedRoutesCache.get(); + for (Node node : context.getMetadataManager().getMetadata().getNodes().values()) { + if (!routes.containsKey(node.getHostId())) { + return false; + } + } + return true; + } + /** * Builds the CQL query to fetch client routes. * @@ -645,13 +670,4 @@ public CompletionStage closeAsync() { LOG.debug("[{}] ClientRoutesTopologyMonitor closed", logPrefix); return super.closeAsync(); } - - /** - * Resolves a hostname to an {@link InetAddress}. Extracted as a protected method so that unit - * tests can override it to return stubbed addresses without hitting the network. - */ - @NonNull - protected InetAddress resolveAddress(@NonNull String hostname) throws UnknownHostException { - return InetAddress.getByName(hostname); - } } diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/CloudTopologyMonitor.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/CloudTopologyMonitor.java index 021824a9b16..013027e139d 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/CloudTopologyMonitor.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/CloudTopologyMonitor.java @@ -44,4 +44,12 @@ protected EndPoint buildNodeEndPoint( UUID hostId = Objects.requireNonNull(row.getUuid("host_id")); return new SniEndPoint(cloudProxyAddress, hostId.toString()); } + + @Override + public boolean reresolvesNodeAddresses() { + // Nodes are reached through the cloud SNI proxy via SniEndPoint, which re-resolves the proxy + // hostname on every connection attempt. Appending the original contact points as a DNS + // re-resolution fallback is therefore unnecessary and could resurrect removed nodes. + return true; + } } diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.java index 7ffbee8e4bb..bff230917b6 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.java @@ -19,26 +19,71 @@ import com.datastax.oss.driver.api.core.metadata.EndPoint; import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; import java.io.Serializable; import java.net.InetSocketAddress; +import java.net.SocketAddress; import java.util.Objects; -public class DefaultEndPoint implements EndPoint, Serializable { +public class DefaultEndPoint implements PinnableEndPoint, Serializable { private static final long serialVersionUID = 1; private final InetSocketAddress address; private final String metricPrefix; + /** + * The address this endpoint has been {@linkplain #pinTo(SocketAddress) pinned} to, or {@code + * null} if it is not pinned. Deliberately excluded from {@link #equals}, {@link #hashCode} and + * {@link #asMetricPrefix()}: a pinned copy denotes the same node as the original. + */ + @Nullable private final InetSocketAddress pinnedAddress; + public DefaultEndPoint(InetSocketAddress address) { + this(address, null); + } + + private DefaultEndPoint(InetSocketAddress address, @Nullable InetSocketAddress pinnedAddress) { this.address = Objects.requireNonNull(address, "address can't be null"); this.metricPrefix = buildMetricPrefix(address); + this.pinnedAddress = pinnedAddress; } + /** + * Returns the address connections should be opened to: the {@linkplain #pinTo(SocketAddress) + * pinned} one if this is a pinned copy, otherwise the stored address as-is. + * + *

This performs no name resolution. If the stored address is a hostname (i.e. {@linkplain + * InetSocketAddress#isUnresolved() unresolved} — contact points are always kept unresolved, see + * {@link com.datastax.oss.driver.api.core.session.SessionBuilder#addContactPoint}) it is returned + * unresolved, and {@link com.datastax.oss.driver.internal.core.channel.ChannelFactory} expands it + * to every IP it maps to through Netty's configured {@code AddressResolverGroup}. Resolving there + * rather than here is deliberate: it keeps any custom resolver installed via {@link + * com.datastax.oss.driver.internal.core.context.NettyOptions#afterBootstrapInitialized} in the + * loop, which a direct {@code InetAddress.getAllByName()} call from here would bypass, and it + * keeps this method non-blocking so it is safe to call from an event loop. + */ @NonNull @Override public InetSocketAddress resolve() { - return address; + return pinnedAddress != null ? pinnedAddress : address; + } + + @NonNull + @Override + public EndPoint pinTo(@NonNull SocketAddress resolvedAddress) { + Objects.requireNonNull(resolvedAddress, "resolvedAddress can't be null"); + if (!(resolvedAddress instanceof InetSocketAddress) + || resolvedAddress.equals(this.pinnedAddress) + // The address we already hold: pinning to it changes nothing, since resolve() and + // toString() would keep yielding what they already do. Skipping the copy spares toString() + // a + // redundant "addr(addr)" suffix on every already-resolved endpoint -- which is all of them, + // once a node is discovered from the peers rows. + || resolvedAddress.equals(this.address)) { + return this; + } + return new DefaultEndPoint(address, (InetSocketAddress) resolvedAddress); } @Override @@ -68,6 +113,9 @@ public int hashCode() { @Override public String toString() { + // Deliberately identical for a pinned copy: see PinnableEndPoint. Which IP a given connection + // landed on is in the channel's own toString(), which Netty builds from the actual remote + // address. return address.toString(); } diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultNode.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultNode.java index 1b09c26ce16..15b943313bf 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultNode.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultNode.java @@ -102,8 +102,33 @@ public EndPoint getEndPoint() { } public void setEndPoint(@NonNull EndPoint newEndPoint, @NonNull InternalDriverContext context) { - if (!newEndPoint.equals(endPoint)) { - endPoint = newEndPoint; + // Metrics are registered under names derived from the endpoint, so they have to be + // re-registered + // whenever those names change -- which is not the same question as whether this is a different + // node. It is narrower in one direction: a PinnableEndPoint copy differs from the original only + // by the address it is pinned to, and both equals() and the metric identity ignore that by + // contract (see PinnableEndPoint). And it is wider in the other: an unresolved hostname and the + // resolved address it maps to compare *equal* (see DefaultEndPoint#equals) while their metric + // prefixes differ, which is exactly what happens when a contact-point node adopts the endpoint + // built from its system.local row. + // + // Both halves of that identity are compared, because both are in use: the default + // MetricIdGenerator names node metrics after asMetricPrefix(), the tagging one tags them with + // the endpoint's toString(). Comparing toString() does mean that an endpoint whose string form + // is not stable across equal instances re-registers this node's metrics on every topology + // refresh; that is the intended reading, since the alternative is reporting under a name the + // endpoint no longer answers to. + boolean differentMetricIdentity = + !newEndPoint.asMetricPrefix().equals(endPoint.asMetricPrefix()) + || !newEndPoint.toString().equals(endPoint.toString()); + // Adopt the newest instance even when it compares equal: a pinned copy carries the address + // every + // subsequent connection to this node will use, so refusing it would freeze the node on the + // first + // address it ever connected to, even after the control connection moved to another one and told + // us about it. + endPoint = newEndPoint; + if (differentMetricIdentity) { // metricUpdater is transient, so it can be null on deserialized nodes. NodeMetricUpdater previousMetricUpdater = metricUpdater; if (previousMetricUpdater != null diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.java index f3f3e4fe346..42382519601 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.java @@ -27,6 +27,8 @@ import com.datastax.oss.driver.api.core.session.Request; import com.datastax.oss.driver.api.core.session.Session; import com.datastax.oss.driver.internal.core.context.InternalDriverContext; +import com.datastax.oss.driver.internal.core.util.collection.CompositeQueryPlan; +import com.datastax.oss.driver.internal.core.util.collection.SimpleQueryPlan; import com.datastax.oss.driver.internal.core.util.concurrent.ReplayingEventFilter; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableSet; @@ -147,7 +149,9 @@ public Queue newQueryPlan( switch (stateRef.get()) { case BEFORE_INIT: case DURING_INIT: - // The contact points are not stored in the metadata yet: + // The contact points are not stored in the metadata yet. Each unresolved hostname is + // expanded to all its DNS IPs at connection time by ChannelFactory, so one entry per + // contact point is enough here. List nodes = new ArrayList<>(context.getMetadataManager().getContactPoints()); Collections.shuffle(nodes); return new ConcurrentLinkedQueue<>(nodes); @@ -164,20 +168,50 @@ public Queue newQueryPlan( @NonNull public Queue newControlReconnectionQueryPlan() { + // Read the state once, before building the regular plan. State transitions are monotonic + // (BEFORE_INIT -> DURING_INIT -> RUNNING -> ...), so this captured value is <= the value + // newQueryPlan() reads internally; that guarantees we never both build the plan from the + // contact points (pre-RUNNING branch of newQueryPlan) and append them again below. + // + // Note: this is still two separate reads of stateRef (this one, and newQueryPlan()'s own + // internal read a moment later), so a transition landing exactly between them is possible: if + // state flips BEFORE_INIT/DURING_INIT -> RUNNING in that window, newQueryPlan() takes the + // RUNNING branch (a real LBP-built plan) while the state captured here is still pre-RUNNING, + // so the contact-point fallback below is skipped for this one call even though + // regularQueryPlan didn't come from the contact-point branch. This is benign: no crash, no + // duplicate entries, and it self-corrects on the very next reconnection attempt. + State state = stateRef.get(); Queue regularQueryPlan = newQueryPlan(null, DriverExecutionProfile.DEFAULT_NAME, null); - if (context - .getConfig() - .getDefaultProfile() - .getBoolean(DefaultDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS)) { - Set originalNodes = context.getMetadataManager().getContactPoints(); + // Only append the contact points as an explicit fallback once the LBP is RUNNING: before that + // (BEFORE_INIT/DURING_INIT), newQueryPlan() above already built regularQueryPlan directly from + // the contact points, so appending them again here would just duplicate every entry. + // + // Skipped when the topology monitor re-resolves node addresses on its own (e.g. proxy-based + // monitors such as client routes or the cloud SNI proxy): those keep addresses fresh without + // this fallback, and appending raw contact points could resurrect nodes the monitor has + // authoritatively removed. The exception is an empty regular plan: with no live node to try, + // reconnection cannot recover on its own, so the contact-point fallback is kept even for those + // monitors. + if (state == State.RUNNING + && context + .getConfig() + .getDefaultProfile() + .getBoolean(DefaultDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS) + && (!context.getTopologyMonitor().reresolvesNodeAddresses() + || regularQueryPlan.isEmpty())) { + // Append the original (unresolved) contact points so every IP their hostname resolves to is + // tried as a fallback: ChannelFactory expands each one at connection time, instead of the + // driver being stuck with whatever single IP a metadata node happens to hold. List contactNodes = new ArrayList<>(); - for (DefaultNode node : originalNodes) { + for (DefaultNode node : context.getMetadataManager().getContactPoints()) { contactNodes.add(DefaultNode.newContactPoint(node.getEndPoint(), context)); } Collections.shuffle(contactNodes); - // Append contact points to the end of the regular query plan so they serve as a fallback - regularQueryPlan.addAll(contactNodes); + // Concatenate rather than mutate: the RUNNING-state regularQueryPlan is a built-in QueryPlan + // whose add()/addAll() throw UnsupportedOperationException (poll() is its only mutator). + // CompositeQueryPlan drains the regular plan first, then the contact-point fallback. + return new CompositeQueryPlan(regularQueryPlan, new SimpleQueryPlan(contactNodes.toArray())); } return regularQueryPlan; diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/MetadataManager.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/MetadataManager.java index cd765c818e6..d8671678306 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/MetadataManager.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/MetadataManager.java @@ -188,6 +188,12 @@ public boolean wasImplicitContactPoint() { * they are never added to metadata and never exposed to user-facing APIs (events, {@link * com.datastax.oss.driver.api.core.metadata.Metadata#getNodes()}, or {@link * com.datastax.oss.driver.api.core.metadata.NodeStateListener} callbacks). + * + *

The metadata node stores {@code nodeInfo.getEndPoint()} as-is and never re-resolves it on + * its own. Re-resolving the original contact-point hostname to pick up current DNS only happens + * through the original-contact-point reconnection fallback (see {@code + * advanced.control-connection.reconnection.fallback-to-original-contact-points}), which re-enters + * the contact points and lets {@code ChannelFactory} expand each hostname at connection time. */ public CompletionStage registerNode(NodeInfo nodeInfo) { Preconditions.checkNotNull(nodeInfo.getHostId(), "Cannot register node without hostId"); diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/PinnableEndPoint.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/PinnableEndPoint.java new file mode 100644 index 00000000000..aee8630e09f --- /dev/null +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/PinnableEndPoint.java @@ -0,0 +1,80 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.datastax.oss.driver.internal.core.metadata; + +import com.datastax.oss.driver.api.core.metadata.EndPoint; +import edu.umd.cs.findbugs.annotations.NonNull; +import java.net.SocketAddress; + +/** + * An {@link EndPoint} that can produce a copy of itself bound ("pinned") to one specific address. + * + *

An endpoint whose hostname maps to several IPs describes a set of candidate addresses, + * but a channel is always connected to exactly one of them. {@link + * com.datastax.oss.driver.internal.core.channel.ChannelFactory} pins the endpoint to the address it + * actually used, and hands the pinned copy to the channel. That matters for two reasons: + * + *

+ * + *

This is an internal extension point: {@code ChannelFactory} pins endpoints that implement it + * and leaves any other implementation untouched, so third-party {@link EndPoint}s keep working + * exactly as before. + * + *

Implementations must keep {@link Object#equals}, {@link Object#hashCode}, {@link + * EndPoint#asMetricPrefix()} and {@link Object#toString()} identical to the unpinned + * original: a pinned copy denotes the same node, and every one of those is part of how the node is + * identified from the outside. Metric names in particular must not change depending on which IP a + * connection happened to land on — and that includes {@code toString()}, which is what {@code + * TaggingMetricIdGenerator} tags node metrics with, and what any third-party {@code + * MetricIdGenerator} is equally free to use. Nodes do adopt pinned copies (see {@code + * DefaultNode#setEndPoint}), so an identity that varied with the pin would silently re-tag a node's + * metrics mid-session. Equality must also stay symmetric: {@code original.equals(pinned)} and + * {@code pinned.equals(original)} must agree, since endpoints are used as set and map keys. + * + *

The pinned address is therefore observable only through {@link EndPoint#resolve()}. That is no + * loss for diagnostics: the address a channel is actually connected to appears in the channel's own + * {@code toString()}, which Netty builds from its remote address, and {@code ChannelFactory} logs + * each candidate as it tries it. + */ +public interface PinnableEndPoint extends EndPoint { + + /** + * Returns a copy of this endpoint that resolves to exactly {@code resolvedAddress}. + * + *

Implementations may return {@code this} when pinning does not apply (for example when the + * address is not of a type they can hold on to), or when it would be a no-op because the endpoint + * already resolves to exactly that address. + * + * @param resolvedAddress the address a connection was successfully established to; must not be + * null and must already be resolved. + */ + @NonNull + EndPoint pinTo(@NonNull SocketAddress resolvedAddress); +} diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.java index d1ab8eec98d..69c0c1151e7 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.java @@ -18,61 +18,113 @@ package com.datastax.oss.driver.internal.core.metadata; import com.datastax.oss.driver.api.core.metadata.EndPoint; -import com.datastax.oss.driver.shaded.guava.common.primitives.UnsignedBytes; +import com.datastax.oss.driver.internal.core.util.AddressUtils; import edu.umd.cs.findbugs.annotations.NonNull; -import java.net.InetAddress; +import edu.umd.cs.findbugs.annotations.Nullable; import java.net.InetSocketAddress; -import java.net.UnknownHostException; -import java.util.Arrays; -import java.util.Comparator; +import java.net.SocketAddress; import java.util.Objects; -import java.util.concurrent.atomic.AtomicInteger; -public class SniEndPoint implements EndPoint { - private static final AtomicInteger OFFSET = new AtomicInteger(); +public class SniEndPoint implements PinnableEndPoint { private final InetSocketAddress proxyAddress; private final String serverName; /** - * @param proxyAddress the address of the proxy. If it is {@linkplain - * InetSocketAddress#isUnresolved() unresolved}, each call to {@link #resolve()} will - * re-resolve it, fetch all of its A-records, and if there are more than 1 pick one in a - * round-robin fashion. + * The proxy IP this endpoint has been {@linkplain #pinTo(SocketAddress) pinned} to, or {@code + * null} if it is not pinned. Deliberately excluded from {@link #equals} and {@link #hashCode}: a + * pinned copy denotes the same node as the original. + */ + @Nullable private final InetSocketAddress pinnedAddress; + + /** + * @param proxyAddress the address of the proxy. A proxy hostname is stored {@linkplain + * InetSocketAddress#isUnresolved() unresolved}, whether or not it was supplied that way, so + * that the driver expands it to all of the proxy's A-records at connection time and tries + * each of them — see {@link #keepHostnameUnresolved}. A proxy given as an IP address is + * stored as-is. * @param serverName the SNI server name. In the context of Cloud, this is the string * representation of the host id. */ public SniEndPoint(InetSocketAddress proxyAddress, String serverName) { - this.proxyAddress = Objects.requireNonNull(proxyAddress, "SNI address cannot be null"); + this(proxyAddress, serverName, null); + } + + private SniEndPoint( + InetSocketAddress proxyAddress, + String serverName, + @Nullable InetSocketAddress pinnedAddress) { + this.proxyAddress = + keepHostnameUnresolved(Objects.requireNonNull(proxyAddress, "SNI address cannot be null")); this.serverName = Objects.requireNonNull(serverName, "SNI Server name cannot be null"); + this.pinnedAddress = pinnedAddress; + } + + /** + * Turns a proxy address that names a host into an unresolved one, leaving anything else + * untouched. + * + *

{@link #resolve()} hands the stored address to the connection layer as-is, and only an + * unresolved one gets expanded and re-expanded there. A proxy hostname supplied already resolved + * would therefore stay bound to whichever single IP its lookup happened to return, for the life + * of the session: no spreading across the proxy's A-records, no fallback when that one IP stops + * answering, and no pick-up of a DNS change. That is a real possibility for a hostname handed to + * {@link + * com.datastax.oss.driver.api.core.session.SessionBuilder#withCloudProxyAddress(InetSocketAddress)}, + * because the ordinary {@code InetSocketAddress(String, int)} constructor resolves eagerly. + * ({@code CloudConfigFactory}, the usual path, already builds an unresolved address.) + * + *

Normalizing here rather than at the call site keeps every {@code SniEndPoint} built from the + * same proxy comparable — {@link #equals} keys on this field — and matches what this endpoint did + * before resolution moved to the connection layer, when it re-resolved the proxy hostname on + * every {@code resolve()} call. + */ + private static InetSocketAddress keepHostnameUnresolved(InetSocketAddress proxyAddress) { + return proxyAddress.isUnresolved() || !AddressUtils.carriesName(proxyAddress) + ? proxyAddress + : InetSocketAddress.createUnresolved(proxyAddress.getHostString(), proxyAddress.getPort()); } public String getServerName() { return serverName; } + /** + * Returns the proxy address connections should be opened to. + * + *

Unpinned, this is the stored proxy address as-is. For Cloud that is a hostname, kept + * unresolved (see {@link #keepHostnameUnresolved}), which {@link + * com.datastax.oss.driver.internal.core.channel.ChannelFactory} expands to every proxy A-record, + * trying each in turn — so a single unreachable proxy IP no longer fails the connection. + * Re-resolving here instead would block whichever event loop called us, and would bypass a custom + * Netty resolver. + * + *

Once {@linkplain #pinTo(SocketAddress) pinned} this returns that one proxy IP. That is what + * {@link com.datastax.oss.driver.internal.core.ssl.SniSslEngineFactory#newSslEngine} sees: it + * runs inside Netty's channel initializer, so it gets the exact IP the channel is connected to + * without a lookup on the event loop. + */ @NonNull @Override public InetSocketAddress resolve() { - try { - InetAddress[] aRecords = InetAddress.getAllByName(proxyAddress.getHostName()); - if (aRecords.length == 0) { - // Probably never happens, but the JDK docs don't explicitly say so - throw new IllegalArgumentException( - "Could not resolve proxy address " + proxyAddress.getHostName()); - } - // The order of the returned address is unspecified. Sort by IP to make sure we get a true - // round-robin - Arrays.sort(aRecords, IP_COMPARATOR); - int index = - (aRecords.length == 1) - ? 0 - : OFFSET.getAndUpdate(x -> x == Integer.MAX_VALUE ? 0 : x + 1) % aRecords.length; - return new InetSocketAddress(aRecords[index], proxyAddress.getPort()); - } catch (UnknownHostException e) { - throw new IllegalArgumentException( - "Could not resolve proxy address " + proxyAddress.getHostName(), e); + return pinnedAddress != null ? pinnedAddress : proxyAddress; + } + + @NonNull + @Override + public EndPoint pinTo(@NonNull SocketAddress resolvedAddress) { + Objects.requireNonNull(resolvedAddress, "resolvedAddress cannot be null"); + if (!(resolvedAddress instanceof InetSocketAddress) + || resolvedAddress.equals(this.pinnedAddress) + // The address we already hold: pinning to it changes nothing -- resolve() and toString() + // keep yielding what they already do -- so spare the copy (and its redundant + // "proxy(proxy)" toString suffix). Only reachable when the proxy was given as an IP + // address: a proxy hostname is stored unresolved, and a resolved pin never compares equal + // to that. + || resolvedAddress.equals(this.proxyAddress)) { + return this; } + return new SniEndPoint(proxyAddress, serverName, (InetSocketAddress) resolvedAddress); } @Override @@ -94,10 +146,10 @@ public int hashCode() { @Override public String toString() { - // Note that this uses the original proxy address, so if there are multiple A-records it won't - // show which one was selected. If that turns out to be a problem for debugging, we might need - // to store the result of resolve() in Connection and log that instead of the endpoint. - return proxyAddress.toString() + ":" + serverName; + // Deliberately identical for a pinned copy: see PinnableEndPoint. Which proxy IP a given + // connection landed on is in the channel's own toString(), which Netty builds from the actual + // remote address. + return proxyAddress + ":" + serverName; } @NonNull @@ -110,10 +162,4 @@ public String asMetricPrefix() { } return hostString.replace('.', '_') + ':' + proxyAddress.getPort() + '_' + serverName; } - - @SuppressWarnings("UnnecessaryLambda") - private static final Comparator IP_COMPARATOR = - (InetAddress address1, InetAddress address2) -> - UnsignedBytes.lexicographicalComparator() - .compare(address1.getAddress(), address2.getAddress()); } diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/TopologyMonitor.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/TopologyMonitor.java index 1bb8e343d96..9be8c4c94bd 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/TopologyMonitor.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/TopologyMonitor.java @@ -141,4 +141,31 @@ public interface TopologyMonitor extends AsyncAutoCloseable { * {@link DefaultTopologyMonitor}) should override this method. */ default void resetColumnCaches() {} + + /** + * Whether this monitor re-resolves node addresses dynamically on every connection attempt (for + * example by re-resolving a proxy hostname each time), rather than relying on an endpoint address + * captured once at node-registration time. + * + *

When this returns {@code true}, the control connection's reconnection query plan must not + * append the original contact points as a DNS re-resolution fallback (see {@code + * advanced.control-connection.reconnection.fallback-to-original-contact-points}): the monitor + * already keeps addresses fresh, and appending raw contact points could resurrect nodes that the + * monitor has authoritatively removed. + * + *

The default implementation returns {@code false}, which is correct for {@link + * DefaultTopologyMonitor}: the peer nodes it registers hold a {@code DefaultEndPoint} built from + * an already-resolved physical IP (from {@code system.peers}), which never needs re-resolving. + * + *

The connected node's own {@code EndPoint} is a different case again. It originates from the + * contact point the control connection used, and {@code ChannelFactory} binds it to the single + * address that connection reached (see {@code PinnableEndPoint}), so it does not re-expand + * on later connection attempts. Recovering from an address change for that node therefore depends + * on this flag being {@code false}, i.e. on the contact-point fallback described above. + * + *

Proxy-based monitors that re-resolve per call should override this to return {@code true}. + */ + default boolean reresolvesNodeAddresses() { + return false; + } } diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/util/AddressUtils.java b/core/src/main/java/com/datastax/oss/driver/internal/core/util/AddressUtils.java index 8905edb9192..bc599910ac7 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/util/AddressUtils.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/util/AddressUtils.java @@ -18,6 +18,7 @@ package com.datastax.oss.driver.internal.core.util; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableSet; +import com.datastax.oss.driver.shaded.guava.common.net.InetAddresses; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.UnknownHostException; @@ -56,4 +57,29 @@ public static Set extract(String address, boolean resolve) { return result; } } + + /** + * Whether {@code address} denotes a host name, as opposed to an IP address written out in + * literal form. + * + *

The distinction matters wherever a name is treated as something that can be resolved — and + * re-resolved — while a literal is taken as the final answer. Both forms can appear resolved or + * unresolved, so neither {@link InetSocketAddress#isUnresolved()} nor the presence of an {@link + * InetAddress} tells them apart. + * + *

Performs no lookup of any kind. + */ + public static boolean carriesName(InetSocketAddress address) { + String hostString = address.getHostString(); + if (hostString == null) { + return false; + } + // A resolved address is compared against the literal its own bytes produce, which is cheaper + // and + // stricter than parsing; an unresolved one has no bytes, so its string has to be parsed. + InetAddress ip = address.getAddress(); + return ip != null + ? !hostString.equals(ip.getHostAddress()) + : !InetAddresses.isInetAddress(hostString); + } } diff --git a/core/src/main/resources/reference.conf b/core/src/main/resources/reference.conf index 8a3a444319e..e94e9398e61 100644 --- a/core/src/main/resources/reference.conf +++ b/core/src/main/resources/reference.conf @@ -1224,27 +1224,25 @@ datastax-java-driver { } - # Whether to resolve the addresses passed to `basic.contact-points`. + # DEPRECATED: this option no longer has any effect and will be removed in a future release. # - # If this is true, addresses are created with `InetSocketAddress(String, int)`: the host name will - # be resolved the first time, and the driver will use the resolved IP address for all subsequent - # connection attempts. + # Contact points are now always kept as unresolved hostnames and expanded to all of their + # DNS-mapped IPs lazily at connection time. This means the driver tries every IP a hostname + # resolves to, and re-resolves the hostname on each new connection so DNS changes are picked up + # automatically. Previously this option selected between resolving a contact-point hostname once + # (true) and re-resolving it on every connection (false); that distinction no longer applies. # - # If this is false, addresses are created with `InetSocketAddress.createUnresolved()`: the host - # name will be resolved again every time the driver opens a new connection. This is useful for - # containerized environments where DNS records are more likely to change over time (note that the - # JVM and OS have their own DNS caching mechanisms, so you might need additional configuration - # beyond the driver). + # The lookup goes through Netty's configured AddressResolverGroup -- the same resolver an + # unresolved address would have reached had it been passed straight to Bootstrap.connect() -- so a + # custom resolver installed via NettyOptions.afterBootstrapInitialized() still applies. With + # Netty's default (JDK) resolver the lookup blocks the I/O event loop it runs on; install + # DnsAddressResolverGroup if you need it to be non-blocking. # - # This option only applies to the contact points specified in the configuration. It has no effect - # on: - # - programmatic contact points passed to SessionBuilder.addContactPoints: these addresses are - # built outside of the driver, so it is your responsibility to provide unresolved instances. - # - dynamically discovered peers: the driver relies on Cassandra system tables, which expose raw - # IP addresses. Use a custom address translator to convert them to unresolved addresses (if - # you're in a containerized environment, you probably already need address translation anyway). + # This option only ever applied to the contact points specified in the configuration -- never to + # programmatic contact points passed to SessionBuilder.addContactPoints, nor to dynamically + # discovered peers. # - # Required: no (defaults to false) + # Required: no # Modifiable at runtime: no # Overridable in a profile: no advanced.resolve-contact-points = false @@ -2338,14 +2336,20 @@ datastax-java-driver { } reconnection { - # 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. + # + # This is also the driver's DNS re-resolution path. Contact points are kept as unresolved + # hostnames and expanded to their current DNS IPs at connection time, through Netty's + # configured resolver. Metadata nodes, in contrast, store an already-resolved endpoint that + # is never re-resolved, so once DNS records change they would otherwise become stale. Keeping + # this enabled lets control-connection reconnects re-resolve the original hostnames and pick up + # the new IPs once the live-node plan is exhausted. # # Required: yes # Modifiable at runtime: yes, the new value will be used for checks issued after the change. # Overridable in a profile: no - fallback-to-original-contact-points = false + fallback-to-original-contact-points = true } } diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryBootstrapHookTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryBootstrapHookTest.java new file mode 100644 index 00000000000..dbe60ae4432 --- /dev/null +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryBootstrapHookTest.java @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.datastax.oss.driver.internal.core.channel; + +import static com.datastax.oss.driver.Assertions.assertThatStage; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.when; + +import com.datastax.oss.driver.api.core.DefaultProtocolVersion; +import com.datastax.oss.driver.api.core.config.DefaultDriverOption; +import com.datastax.oss.driver.internal.core.context.NettyOptions; +import com.datastax.oss.driver.internal.core.metrics.NoopNodeMetricUpdater; +import io.netty.bootstrap.Bootstrap; +import io.netty.channel.ChannelInboundHandlerAdapter; +import java.util.concurrent.CompletionStage; +import org.junit.Test; + +/** + * Verifies the {@link NettyOptions#afterBootstrapInitialized(Bootstrap)} contract: the hook runs on + * a handler-less bootstrap, and a handler it installs is replaced by the driver's own. + */ +public class ChannelFactoryBootstrapHookTest extends ChannelFactoryTestBase { + + @Test + public void should_replace_handler_installed_by_bootstrap_hook() { + // Given – a hook that (incorrectly) installs its own channel handler. The driver sets its own + // handler on each per-attempt copy afterwards, logging a one-time warning; if the dummy + // handler below survived instead, the protocol handshake would never happen and this connect + // would fail on the init timeout. + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + doAnswer( + invocation -> { + Bootstrap bootstrap = invocation.getArgument(0); + bootstrap.handler(new ChannelInboundHandlerAdapter()); + return null; + }) + .when(nettyOptions) + .afterBootstrapInitialized(any(Bootstrap.class)); + ChannelFactory factory = newChannelFactory(); + + // When + CompletionStage channelFuture = + factory.connect( + SERVER_ADDRESS, + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE); + completeSimpleChannelInit(); + + // Then + assertThatStage(channelFuture).isSuccess(); + } +} diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryMultiAddressTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryMultiAddressTest.java new file mode 100644 index 00000000000..fc6be0af645 --- /dev/null +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryMultiAddressTest.java @@ -0,0 +1,452 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.datastax.oss.driver.internal.core.channel; + +import static com.datastax.oss.driver.Assertions.assertThatStage; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assumptions.assumeThat; +import static org.mockito.Mockito.when; + +import com.datastax.oss.driver.api.core.DefaultProtocolVersion; +import com.datastax.oss.driver.api.core.config.DefaultDriverOption; +import com.datastax.oss.driver.api.core.metadata.EndPoint; +import com.datastax.oss.driver.internal.core.metadata.DefaultEndPoint; +import com.datastax.oss.driver.internal.core.metrics.NoopNodeMetricUpdater; +import com.datastax.oss.driver.internal.core.util.AddressUtils; +import edu.umd.cs.findbugs.annotations.NonNull; +import io.netty.channel.local.LocalAddress; +import java.net.Inet6Address; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.NetworkInterface; +import java.net.SocketAddress; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.TimeUnit; +import org.junit.Test; + +/** + * Verifies how {@link ChannelFactory#connect} treats the several addresses a name expands to: every + * one of them is tried in sequence, failures are aggregated rather than dropped, and the starting + * address is rotated so healthy connections do not all pile onto the same one. + * + *

The expansion itself is exercised in {@link ChannelFactoryNettyResolverTest}; here the + * resolver is only the mechanism for producing more than one address from a single endpoint. + */ +public class ChannelFactoryMultiAddressTest extends ChannelFactoryTestBase { + + // Local addresses that no server is bound to: connecting to them fails immediately. + private static final SocketAddress UNREACHABLE_1 = + new LocalAddress(ChannelFactoryMultiAddressTest.class.getSimpleName() + "-unreachable-1"); + private static final SocketAddress UNREACHABLE_2 = + new LocalAddress(ChannelFactoryMultiAddressTest.class.getSimpleName() + "-unreachable-2"); + + /** The name the endpoint reports, and that only the resolver knows how to expand. */ + private static final InetSocketAddress HOSTNAME = + InetSocketAddress.createUnresolved("test.cluster.fake", 9042); + + @Test + public void should_fail_with_suppressed_causes_when_all_addresses_are_unreachable() { + // Given – a name that expands to two dead addresses. + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + installResolver(new TestAddressResolverGroup(Arrays.asList(UNREACHABLE_1, UNREACHABLE_2))); + ChannelFactory factory = newChannelFactory(); + + // When + CompletionStage channelFuture = + factory.connect( + new DefaultEndPoint(HOSTNAME), + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE); + + // Then – the future fails, and the earlier address's failure is preserved as a suppressed + // exception on the last one's error rather than being silently dropped. + assertThatStage(channelFuture) + .isFailed( + e -> + assertThat(e.getSuppressed()) + .as("earlier address failures should be attached as suppressed exceptions") + .isNotEmpty()); + } + + @Test + public void should_rotate_the_starting_address_across_successive_expansions() { + // Every address must still be offered -- a single attempt has to be able to fall back across + // all of them -- but the one tried *first* has to move, otherwise every connection piles onto + // the resolver's first record and a multi-record name buys no spreading at all. + InetSocketAddress name = InetSocketAddress.createUnresolved("successive.rotate.fake", 9042); + List addresses = Arrays.asList(UNREACHABLE_1, UNREACHABLE_2); + ChannelFactory factory = newChannelFactory(); + + List first = factory.rotate(name, addresses); + List second = factory.rotate(name, addresses); + + assertThat(first).containsExactlyInAnyOrder(UNREACHABLE_1, UNREACHABLE_2); + assertThat(second).containsExactlyInAnyOrder(UNREACHABLE_1, UNREACHABLE_2); + assertThat(second.get(0)) + .as("successive expansions must not start at the same address") + .isNotEqualTo(first.get(0)); + } + + @Test + public void should_leave_a_single_address_alone() { + InetSocketAddress name = InetSocketAddress.createUnresolved("single.rotate.fake", 9042); + ChannelFactory factory = newChannelFactory(); + + // Nothing to spread... + assertThat(factory.rotate(name, Collections.singletonList(UNREACHABLE_1))) + .containsExactly(UNREACHABLE_1); + + // ...and no rotation offset may be burned for it either: the name's first multi-address + // expansion still starts at the toString-sorted first element ("...-1" sorts before "...-2"). + List next = factory.rotate(name, Arrays.asList(UNREACHABLE_2, UNREACHABLE_1)); + assertThat(next.get(0)).isEqualTo(UNREACHABLE_1); + } + + @Test + public void should_rotate_names_independently() { + InetSocketAddress nameA = InetSocketAddress.createUnresolved("a.independent.fake", 9042); + InetSocketAddress nameB = InetSocketAddress.createUnresolved("b.independent.fake", 9042); + List addresses = Arrays.asList(UNREACHABLE_1, UNREACHABLE_2); + ChannelFactory factory = newChannelFactory(); + + // Interleave the two names the way two hostname contact points are expanded in sequence on + // every reconnection round. With one global counter each name would only ever see one offset + // parity, pinning both to a fixed starting address forever. + List a1 = factory.rotate(nameA, addresses); + List b1 = factory.rotate(nameB, addresses); + List a2 = factory.rotate(nameA, addresses); + List b2 = factory.rotate(nameB, addresses); + + // Each name still rotates on its own... + assertThat(a2.get(0)) + .as("name A must rotate despite interleaved expansions of name B") + .isNotEqualTo(a1.get(0)); + assertThat(b2.get(0)) + .as("name B must rotate despite interleaved expansions of name A") + .isNotEqualTo(b1.get(0)); + // ...and is not perturbed by the other: both fresh names start at the same (sorted-first) + // element instead of B starting wherever A's expansions left a shared counter. + assertThat(b1.get(0)).isEqualTo(a1.get(0)); + } + + @Test + public void should_not_share_rotation_offsets_across_factories() { + // The counters belong to the factory, i.e. to the session: a name an earlier session expanded + // must not leave an offset behind for the next one, and nothing outlives the session. + InetSocketAddress name = InetSocketAddress.createUnresolved("per.session.rotate.fake", 9042); + List addresses = Arrays.asList(UNREACHABLE_1, UNREACHABLE_2); + + List first = newChannelFactory().rotate(name, addresses); + List other = newChannelFactory().rotate(name, addresses); + + assertThat(other.get(0)) + .as("a fresh factory rotates from its own start, not from where another one left off") + .isEqualTo(first.get(0)); + } + + @Test + public void should_bound_the_number_of_tracked_names() { + // Client routes can hand out different hostnames on every refresh, so even within one session + // the set of names is not bounded by the configuration or the topology. + ChannelFactory factory = newChannelFactory(); + List addresses = Arrays.asList(UNREACHABLE_1, UNREACHABLE_2); + + for (int i = 0; i < ChannelFactory.MAX_ROTATION_OFFSETS * 4; i++) { + factory.rotate(InetSocketAddress.createUnresolved("churn-" + i + ".fake", 9042), addresses); + } + + assertThat(factory.rotationOffsets.size()) + .as("stale names must be evicted rather than retained for the session's lifetime") + .isLessThanOrEqualTo(ChannelFactory.MAX_ROTATION_OFFSETS); + } + + // ---- reattachHostname() --------------------------------------------------- + + @Test + public void should_reattach_queried_hostname_to_nameless_resolved_address() throws Exception { + // A custom resolver may build its results from raw address bytes; the queried name must be + // re-attached so TLS hostname validation checks the configured name (not the IP or a PTR + // record) and reading the host name never triggers a reverse lookup on the event loop. + InetSocketAddress candidate = + new InetSocketAddress(InetAddress.getByAddress(new byte[] {10, 0, 0, 1}), 9999); + + InetSocketAddress result = + (InetSocketAddress) ChannelFactory.reattachHostname(HOSTNAME, candidate); + + assertThat(result.isUnresolved()).isFalse(); + // getHostString() never looks anything up; getHostName() reverse-resolves a *nameless* + // address, so it returning the queried name proves the name is embedded, not looked up. + assertThat(result.getHostString()).isEqualTo("test.cluster.fake"); + assertThat(result.getHostName()).isEqualTo("test.cluster.fake"); + assertThat(result.getAddress().getHostAddress()).isEqualTo("10.0.0.1"); + // The candidate's port wins over the original's: a resolver may remap ports too. + assertThat(result.getPort()).isEqualTo(9999); + // Equality is unchanged (a resolved InetSocketAddress compares IP bytes + port only), so + // pinning and the pin-equality shortcuts behave exactly as with the raw candidate. + assertThat(result).isEqualTo(candidate); + } + + @Test + public void should_override_resolver_provided_hostname_with_queried_name() throws Exception { + // A resolver may label its results with a canonical/CNAME name of its own. That name would end + // up on the pinned endpoint and hence be the one TLS hostname verification checks the server + // certificate against, so the name the user configured has to win over it. + InetSocketAddress candidate = + new InetSocketAddress( + InetAddress.getByAddress("cname.example.fake", new byte[] {10, 0, 0, 1}), 9042); + + InetSocketAddress result = + (InetSocketAddress) ChannelFactory.reattachHostname(HOSTNAME, candidate); + + assertThat(result.getHostString()).isEqualTo("test.cluster.fake"); + assertThat(result.getAddress().getHostAddress()).isEqualTo("10.0.0.1"); + assertThat(result.getPort()).isEqualTo(9042); + } + + @Test + public void should_pass_candidate_through_when_it_already_carries_the_queried_name() + throws Exception { + // The common case: the JDK and Netty-DNS resolvers attach the queried name themselves, so + // there is nothing to rebuild. + InetSocketAddress candidate = + new InetSocketAddress( + InetAddress.getByAddress("test.cluster.fake", new byte[] {10, 0, 0, 1}), 9042); + + assertThat(ChannelFactory.reattachHostname(HOSTNAME, candidate)).isSameAs(candidate); + } + + @Test + public void should_pass_non_inet_candidate_through() { + // The local-transport addresses these unit tests connect over must never be touched. + assertThat(ChannelFactory.reattachHostname(HOSTNAME, UNREACHABLE_1)).isSameAs(UNREACHABLE_1); + } + + @Test + public void should_pass_candidate_through_when_original_carries_no_name() throws Exception { + // An original written as an IP literal has no name to carry over, and inventing one from the + // literal would be worse than leaving the candidate alone: a resolver is free to redirect it to + // a different IP, which would then be labelled with the literal form of a *different* address. + InetSocketAddress original = new InetSocketAddress("127.0.0.1", 9042); + InetSocketAddress candidate = + new InetSocketAddress(InetAddress.getByAddress(new byte[] {10, 0, 0, 1}), 9042); + + assertThat(ChannelFactory.reattachHostname(original, candidate)).isSameAs(candidate); + assertThat(AddressUtils.carriesName(original)).isFalse(); + assertThat(AddressUtils.carriesName(InetSocketAddress.createUnresolved("10.0.0.1", 9042))) + .isFalse(); + } + + @Test + public void should_reattach_name_of_a_resolved_original() throws Exception { + // A resolved original still reaches the resolver -- whether an address needs resolving is the + // resolver's call, and a custom one may redirect it. Its name is the one the operator + // configured, so it must survive onto whatever the resolver substitutes, exactly as it did when + // Netty resolved the TCP destination and the channel kept the original endpoint for TLS. + InetSocketAddress original = new InetSocketAddress("localhost", 9042); + InetSocketAddress candidate = + new InetSocketAddress(InetAddress.getByAddress(new byte[] {10, 0, 0, 1}), 9042); + + InetSocketAddress result = + (InetSocketAddress) ChannelFactory.reattachHostname(original, candidate); + + assertThat(AddressUtils.carriesName(original)).isTrue(); + assertThat(result.getHostString()).isEqualTo("localhost"); + assertThat(result.getAddress().getHostAddress()).isEqualTo("10.0.0.1"); + } + + @Test + public void should_reattach_hostname_to_nameless_ipv6_address() throws Exception { + byte[] loopback = new byte[16]; + loopback[15] = 1; // ::1 + InetSocketAddress candidate = new InetSocketAddress(InetAddress.getByAddress(loopback), 9042); + + InetSocketAddress result = + (InetSocketAddress) ChannelFactory.reattachHostname(HOSTNAME, candidate); + + assertThat(result.getHostString()).isEqualTo("test.cluster.fake"); + assertThat(result.getAddress()).isEqualTo(candidate.getAddress()); + assertThat(result.getPort()).isEqualTo(9042); + } + + @Test + public void should_keep_the_scope_when_reattaching_to_a_scoped_ipv6_address() throws Exception { + // A link-local address only points anywhere together with its zone, so the queried name has to + // be re-attached without dropping the scope. InetAddress.getByAddress(host, bytes) cannot carry + // one, but Inet6Address.getByAddress(host, bytes, scopeId) can. + byte[] linkLocal = new byte[16]; + linkLocal[0] = (byte) 0xfe; + linkLocal[1] = (byte) 0x80; + linkLocal[15] = 1; + InetSocketAddress candidate = + new InetSocketAddress(Inet6Address.getByAddress(null, linkLocal, 3), 9042); + + InetSocketAddress result = + (InetSocketAddress) ChannelFactory.reattachHostname(HOSTNAME, candidate); + + assertThat(result.getHostString()).isEqualTo("test.cluster.fake"); + assertThat(result.getAddress()).isInstanceOf(Inet6Address.class); + assertThat(((Inet6Address) result.getAddress()).getScopeId()).isEqualTo(3); + assertThat(result.getAddress().getAddress()).isEqualTo(linkLocal); + assertThat(result.getPort()).isEqualTo(9042); + } + + @Test + public void should_keep_the_zone_of_an_interface_scoped_ipv6_address() throws Exception { + // An address built from a NetworkInterface rather than from an index must keep pointing into + // the + // same zone. The numeric scope the JDK derived at construction is what the connect goes on, so + // carrying that over is enough; only the interface name, a toString() detail, is not. + Inet6Address linkLocal = firstInterfaceScopedIpv6Address(); + assumeThat(linkLocal).as("no interface-scoped IPv6 address on this host").isNotNull(); + InetSocketAddress candidate = new InetSocketAddress(linkLocal, 9042); + + InetSocketAddress result = + (InetSocketAddress) ChannelFactory.reattachHostname(HOSTNAME, candidate); + + assertThat(result.getHostString()).isEqualTo("test.cluster.fake"); + assertThat(((Inet6Address) result.getAddress()).getScopeId()).isEqualTo(linkLocal.getScopeId()); + assertThat(result.getAddress().getAddress()).isEqualTo(linkLocal.getAddress()); + } + + /** An interface-scoped IPv6 address of this host, or null if it has none. */ + private static Inet6Address firstInterfaceScopedIpv6Address() throws Exception { + for (NetworkInterface nif : Collections.list(NetworkInterface.getNetworkInterfaces())) { + for (InetAddress address : Collections.list(nif.getInetAddresses())) { + if (address instanceof Inet6Address + && ((Inet6Address) address).getScopedInterface() != null) { + return (Inet6Address) address; + } + } + } + return null; + } + + @Test + public void should_fail_future_when_endpoint_resolve_throws() { + // ChannelFactory calls EndPoint.resolve() directly on the caller thread, so a third-party + // implementation that throws must surface as a failed future rather than an escaping exception. + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + ChannelFactory factory = newChannelFactory(); + IllegalStateException failure = new IllegalStateException("resolve() blew up"); + + CompletionStage channelFuture = + factory.connect( + new ThrowingEndPoint(failure), + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE); + + assertThatStage(channelFuture).isFailed(e -> assertThat(e).isSameAs(failure)); + } + + @Test + public void should_fail_future_when_endpoint_resolve_returns_null() { + // EndPoint.resolve() is contractually non-null, but a broken third-party implementation must + // fail fast rather than NPE later inside an event-loop task, which would leave the future + // hanging. + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + ChannelFactory factory = newChannelFactory(); + + CompletionStage channelFuture = + factory.connect( + new NullResolvingEndPoint(), + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE); + + assertThatStage(channelFuture) + .isFailed( + e -> + assertThat(e) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("returned null")); + } + + @Test + public void should_fail_future_when_event_loop_group_is_rejecting_tasks() + throws InterruptedException { + // Resolution is dispatched to an I/O event loop; if the group is already shutting down, that + // dispatch is rejected synchronously. The rejection must fail the future rather than escape to + // the caller (connect() never used to throw) or leave the future hanging. + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + ChannelFactory factory = newChannelFactory(); + clientGroup.shutdownGracefully(0, 0, TimeUnit.MILLISECONDS).sync(); + + CompletionStage channelFuture = + factory.connect( + new DefaultEndPoint(HOSTNAME), + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE); + + assertThatStage(channelFuture) + .isFailed(e -> assertThat(e).isInstanceOf(RejectedExecutionException.class)); + } + + /** An endpoint whose {@link EndPoint#resolve()} throws, standing in for a broken third party. */ + private static class ThrowingEndPoint implements EndPoint { + + private final RuntimeException failure; + + ThrowingEndPoint(RuntimeException failure) { + this.failure = failure; + } + + @NonNull + @Override + public SocketAddress resolve() { + throw failure; + } + + @NonNull + @Override + public String asMetricPrefix() { + return "test"; + } + } + + /** A broken third-party endpoint that violates {@code resolve()}'s non-null contract. */ + private static class NullResolvingEndPoint implements EndPoint { + + @NonNull + @Override + @SuppressWarnings("NullAway") // deliberately broken, that is the point of the test + public SocketAddress resolve() { + return null; + } + + @NonNull + @Override + public String asMetricPrefix() { + return "test"; + } + } +} diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryNettyResolverTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryNettyResolverTest.java new file mode 100644 index 00000000000..92d092ee107 --- /dev/null +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryNettyResolverTest.java @@ -0,0 +1,339 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.datastax.oss.driver.internal.core.channel; + +import static com.datastax.oss.driver.Assertions.assertThatStage; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.when; + +import com.datastax.oss.driver.api.core.DefaultProtocolVersion; +import com.datastax.oss.driver.api.core.config.DefaultDriverOption; +import com.datastax.oss.driver.internal.core.metadata.DefaultEndPoint; +import com.datastax.oss.driver.internal.core.metrics.NoopNodeMetricUpdater; +import io.netty.bootstrap.Bootstrap; +import io.netty.channel.DefaultEventLoopGroup; +import io.netty.channel.local.LocalAddress; +import io.netty.resolver.AddressResolver; +import io.netty.resolver.AddressResolverGroup; +import io.netty.util.concurrent.EventExecutor; +import io.netty.util.concurrent.Future; +import io.netty.util.concurrent.Promise; +import java.net.InetSocketAddress; +import java.net.SocketAddress; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.TimeUnit; +import org.junit.Test; + +/** + * Verifies that {@link ChannelFactory} expands unresolved candidate addresses through Netty's + * configured {@link AddressResolverGroup}, rather than doing its own JVM DNS lookup. + * + *

This is what keeps a custom resolver installed via {@link + * com.datastax.oss.driver.internal.core.context.NettyOptions#afterBootstrapInitialized(Bootstrap)} + * effective: before multi-address support, an unresolved address was handed straight to {@code + * Bootstrap.connect()} and Netty's resolver expanded it, so resolving anywhere else would silently + * bypass the user's configuration. + */ +public class ChannelFactoryNettyResolverTest extends ChannelFactoryTestBase { + + // A local address that no server is bound to: connecting to it fails immediately. + private static final SocketAddress UNREACHABLE = + new LocalAddress(ChannelFactoryNettyResolverTest.class.getSimpleName() + "-unreachable"); + + /** The hostname the endpoint reports, and that only the custom resolver knows how to expand. */ + private static final InetSocketAddress HOSTNAME = + InetSocketAddress.createUnresolved("test.cluster.fake", 9042); + + @Test + public void should_expand_unresolved_address_through_the_custom_netty_resolver() { + // Given – a resolver that maps the hostname to an unreachable address followed by the running + // local server, mimicking a DNS round-robin entry whose first record is dead. + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + TestAddressResolverGroup resolverGroup = + new TestAddressResolverGroup(Arrays.asList(UNREACHABLE, SERVER_ADDRESS.resolve())); + installResolver(resolverGroup); + ChannelFactory factory = newChannelFactory(); + + // When – the endpoint itself performs no resolution at all; it just yields the hostname. + CompletionStage channelFuture = + factory.connect( + new DefaultEndPoint(HOSTNAME), + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE); + // The handshake only happens once we fall back to the reachable second address. + completeSimpleChannelInit(); + + // Then – the custom resolver was consulted for the hostname, and *all* the addresses it + // returned + // were tried, so the connection survived the dead first record. + assertThatStage(channelFuture).isSuccess(); + assertThat(resolverGroup.queried) + .as("the custom Netty resolver must be the one expanding the hostname") + .containsExactly(HOSTNAME); + } + + @Test + public void should_fail_when_the_custom_resolver_cannot_resolve_the_only_candidate() { + // Given – a resolver that fails every lookup. + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + TestAddressResolverGroup resolverGroup = new TestAddressResolverGroup(null); + installResolver(resolverGroup); + ChannelFactory factory = newChannelFactory(); + + // When + CompletionStage channelFuture = + factory.connect( + new DefaultEndPoint(HOSTNAME), + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE); + + // Then – no candidate survived resolution, so the connect fails with the resolver's own cause + // rather than, say, an empty-candidate-list error. + assertThatStage(channelFuture) + .isFailed(e -> assertThat(e).hasMessageContaining("mock resolver failure")); + } + + @Test + public void should_not_resolve_at_all_when_the_user_disabled_the_resolver() { + // Given – Bootstrap.disableResolver() means config().resolver() is null. ChannelFactory must + // treat that as "pass the candidates through" instead of dereferencing the missing group. + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + TestAddressResolverGroup resolverGroup = + new TestAddressResolverGroup(Collections.singletonList(UNREACHABLE)); + doAnswer( + invocation -> { + Bootstrap bootstrap = invocation.getArgument(0); + bootstrap.resolver(resolverGroup).disableResolver(); + return null; + }) + .when(nettyOptions) + .afterBootstrapInitialized(any(Bootstrap.class)); + ChannelFactory factory = newChannelFactory(); + + // When – the endpoint yields an already-usable address. + CompletionStage channelFuture = + factory.connect( + SERVER_ADDRESS, + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE); + completeSimpleChannelInit(); + + // Then – connection succeeds and the resolver was never even instantiated, let alone consulted. + assertThatStage(channelFuture).isSuccess(); + assertThat(resolverGroup.resolverRequested).isFalse(); + assertThat(resolverGroup.queried).isEmpty(); + } + + @Test + public void should_pass_already_resolved_address_through_untouched() { + // Given – an endpoint whose address is already resolved, which is the common case: metadata + // nodes hold resolved addresses from the peers rows, so this is every pool refill and every + // reconnect. A resolver with the usual semantics reports it as resolved and there is nothing + // to expand. + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + TestAddressResolverGroup resolverGroup = + new TestAddressResolverGroup(Collections.singletonList(UNREACHABLE)); + installResolver(resolverGroup); + ChannelFactory factory = newChannelFactory(); + + // When + CompletionStage channelFuture = + factory.connect( + SERVER_ADDRESS, + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE); + completeSimpleChannelInit(); + + // Then – no lookup was performed: had one been, it would have redirected us to UNREACHABLE and + // the connection would have failed. The decision was the resolver's own, though -- see + // should_let_the_resolver_redirect_an_already_resolved_address. + assertThatStage(channelFuture).isSuccess(); + assertThat(resolverGroup.queried).isEmpty(); + assertThat(resolverGroup.resolverRequested) + .as("whether an address needs resolving must be the resolver's decision") + .isTrue(); + } + + @Test + public void should_let_the_resolver_redirect_an_already_resolved_address() { + // Given – a resolver that reports even an address carrying an IP as still needing resolution, + // and redirects it. Netty consulted the resolver for every connect, resolved address or not + // (Bootstrap#doResolveAndConnect0 calls isSupported()/isResolved() on it rather than testing + // the address itself), so short-circuiting on InetSocketAddress#isUnresolved() here would take + // that away for every connect to an already-resolved node -- which is nearly all of them. + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + TestAddressResolverGroup resolverGroup = + new TestAddressResolverGroup( + Collections.singletonList(SERVER_ADDRESS.resolve()), + /* claimNothingIsResolved = */ true); + installResolver(resolverGroup); + ChannelFactory factory = newChannelFactory(); + + // When – the endpoint holds a resolved address that nothing is listening on. + InetSocketAddress resolved = new InetSocketAddress("127.0.0.1", 9042); + CompletionStage channelFuture = + factory.connect( + new DefaultEndPoint(resolved), + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE); + completeSimpleChannelInit(); + + // Then – the connect landed on the address the resolver substituted, which it could only do by + // having been asked about an address that already carried an IP. (Netty then asks again about + // the substitute, from Bootstrap.connect(); that second lookup is its own business.) + assertThatStage(channelFuture).isSuccess(); + assertThat(resolverGroup.queried) + .as("the resolver must get a say on an address that already carries an IP") + .startsWith(resolved); + } + + @Test + public void should_resolve_and_connect_on_the_same_event_loop() throws InterruptedException { + // Resolution and channel registration must share the loop picked once per connect. Taking one + // loop for resolution and letting the registration pick another would advance the group's + // round-robin chooser twice per connect, parking every channel on half the loops with the + // default power-of-two chooser. The base's single-thread group would make this assertion + // vacuous, so use two loops -- on which the split behavior was deterministic. + DefaultEventLoopGroup twoLoops = new DefaultEventLoopGroup(2); + try { + when(nettyOptions.ioEventLoopGroup()).thenReturn(twoLoops); + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + TestAddressResolverGroup resolverGroup = + new TestAddressResolverGroup(Collections.singletonList(SERVER_ADDRESS.resolve())); + installResolver(resolverGroup); + ChannelFactory factory = newChannelFactory(); + + CompletionStage channelFuture = + factory.connect( + new DefaultEndPoint(HOSTNAME), + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE); + completeSimpleChannelInit(); + + assertThatStage(channelFuture) + .isSuccess( + channel -> + assertThat((Object) channel.eventLoop()) + .as("the channel must be registered on the loop resolution ran on") + .isSameAs(resolverGroup.resolverExecutor)); + } finally { + twoLoops.shutdownGracefully(0, 100, TimeUnit.MILLISECONDS).sync(); + } + } + + @Test + public void should_fail_future_when_resolver_throws_synchronously() { + // Given – a broken custom resolver that throws instead of returning a failed future. The throw + // happens inside an event-loop task, where nothing else would ever complete the connect future: + // nothing at this stage has a timeout, so before the blanket catch in resolveCandidates() this + // hung the connect attempt (and with it control-connection init) forever. + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + RuntimeException failure = new IllegalStateException("broken resolver"); + installResolver(new ThrowingAddressResolverGroup(failure)); + ChannelFactory factory = newChannelFactory(); + + // When + CompletionStage channelFuture = + factory.connect( + new DefaultEndPoint(HOSTNAME), + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE); + + // Then + assertThatStage(channelFuture).isFailed(e -> assertThat(e).isSameAs(failure)); + } + + /** A resolver whose every method throws, standing in for a broken third-party implementation. */ + private static class ThrowingAddressResolverGroup extends AddressResolverGroup { + + private final RuntimeException failure; + + ThrowingAddressResolverGroup(RuntimeException failure) { + this.failure = failure; + } + + @Override + protected AddressResolver newResolver(EventExecutor executor) { + return new AddressResolver() { + + @Override + public boolean isSupported(SocketAddress address) { + throw failure; + } + + @Override + public boolean isResolved(SocketAddress address) { + throw failure; + } + + @Override + public Future resolve(SocketAddress address) { + throw failure; + } + + @Override + public Future resolve( + SocketAddress address, Promise promise) { + throw failure; + } + + @Override + public Future> resolveAll(SocketAddress address) { + throw failure; + } + + @Override + public Future> resolveAll( + SocketAddress address, Promise> promise) { + throw failure; + } + + @Override + public void close() { + // nothing to do + } + }; + } + } +} diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryPinnedEndPointTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryPinnedEndPointTest.java new file mode 100644 index 00000000000..4bf21472a79 --- /dev/null +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryPinnedEndPointTest.java @@ -0,0 +1,189 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.datastax.oss.driver.internal.core.channel; + +import static com.datastax.oss.driver.Assertions.assertThatStage; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +import com.datastax.oss.driver.api.core.DefaultProtocolVersion; +import com.datastax.oss.driver.api.core.config.DefaultDriverOption; +import com.datastax.oss.driver.api.core.metadata.EndPoint; +import com.datastax.oss.driver.internal.core.metadata.PinnableEndPoint; +import com.datastax.oss.driver.internal.core.metrics.NoopNodeMetricUpdater; +import edu.umd.cs.findbugs.annotations.NonNull; +import io.netty.channel.local.LocalAddress; +import java.net.InetSocketAddress; +import java.net.SocketAddress; +import java.util.Arrays; +import java.util.Collections; +import java.util.Objects; +import java.util.concurrent.CompletionStage; +import org.junit.Test; + +/** + * Verifies that a successfully connected {@link DriverChannel} carries an endpoint bound to the + * address the connection actually used, not the multi-address original. + * + *

Without this, a hostname shared by several nodes would let a later reconnect land on a + * different node while still being treated as the original {@code host_id}: {@code + * DefaultTopologyMonitor#buildNodeEndPoint} stores the channel's endpoint for the control node, and + * {@code ControlConnection} skips identity re-resolution for nodes that already have a host id. See + * {@link PinnableEndPoint}. + */ +public class ChannelFactoryPinnedEndPointTest extends ChannelFactoryTestBase { + + // A local address that no server is bound to: connecting to it fails immediately. + private static final SocketAddress UNREACHABLE = + new LocalAddress(ChannelFactoryPinnedEndPointTest.class.getSimpleName() + "-unreachable"); + + /** The name the endpoint reports, and that only the resolver knows how to expand. */ + private static final InetSocketAddress HOSTNAME = + InetSocketAddress.createUnresolved("test.cluster.fake", 9042); + + @Test + public void should_pin_channel_endpoint_to_the_address_that_connected() { + // Given – an endpoint reporting a name, which the resolver expands to a dead address and the + // running local server. Whichever of the two the connection ends up on, the channel must carry + // that one. + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + SocketAddress reachable = SERVER_ADDRESS.resolve(); + installResolver(new TestAddressResolverGroup(Arrays.asList(UNREACHABLE, reachable))); + ChannelFactory factory = newChannelFactory(); + TestPinnableEndPoint endPoint = new TestPinnableEndPoint(HOSTNAME); + + // When + CompletionStage channelFuture = + factory.connect( + endPoint, null, null, DriverChannelOptions.DEFAULT, NoopNodeMetricUpdater.INSTANCE); + completeSimpleChannelInit(); + + // Then + assertThatStage(channelFuture) + .isSuccess( + channel -> { + EndPoint channelEndPoint = channel.getEndPoint(); + // The channel resolves to the address it is actually connected to -- the name it was + // built from is gone from resolve(), which is what SSL engines and authenticators + // need. + assertThat(channelEndPoint.resolve()).isEqualTo(reachable); + // ...while still denoting the same node, so node lookups and metric names are stable. + assertThat(channelEndPoint).isEqualTo(endPoint); + assertThat(channelEndPoint.asMetricPrefix()).isEqualTo(endPoint.asMetricPrefix()); + }); + } + + @Test + public void should_leave_non_pinnable_endpoints_untouched() { + // A third-party EndPoint that does not implement PinnableEndPoint must reach the channel + // exactly + // as it was given, so existing implementations keep working. + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + ChannelFactory factory = newChannelFactory(); + + CompletionStage channelFuture = + factory.connect( + SERVER_ADDRESS, + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE); + completeSimpleChannelInit(); + + assertThatStage(channelFuture) + .isSuccess(channel -> assertThat(channel.getEndPoint()).isSameAs(SERVER_ADDRESS)); + } + + @Test + public void should_fail_future_when_pin_to_throws() { + // pinTo() runs in the continuation after resolution, whose exceptions CompletionStage + // swallows; a throwing implementation must fail the connect future rather than hang it. + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + SocketAddress reachable = SERVER_ADDRESS.resolve(); + installResolver(new TestAddressResolverGroup(Collections.singletonList(reachable))); + ChannelFactory factory = newChannelFactory(); + RuntimeException failure = new IllegalStateException("pinTo blew up"); + TestPinnableEndPoint endPoint = + new TestPinnableEndPoint(HOSTNAME) { + @NonNull + @Override + public EndPoint pinTo(@NonNull SocketAddress resolvedAddress) { + throw failure; + } + }; + + CompletionStage channelFuture = + factory.connect( + endPoint, null, null, DriverChannelOptions.DEFAULT, NoopNodeMetricUpdater.INSTANCE); + + assertThatStage(channelFuture).isFailed(e -> assertThat(e).isSameAs(failure)); + } + + /** + * A {@link PinnableEndPoint} that can hold a pin to any {@link SocketAddress}, including the + * local-transport addresses these tests connect over (which {@code DefaultEndPoint} cannot). + * Identity is the unpinned address, so a pinned copy stays equal to the original — the contract + * {@link PinnableEndPoint} requires. + */ + private static class TestPinnableEndPoint implements PinnableEndPoint { + + private final SocketAddress address; + private final SocketAddress pinnedAddress; + + TestPinnableEndPoint(SocketAddress address) { + this(address, null); + } + + private TestPinnableEndPoint(SocketAddress address, SocketAddress pinnedAddress) { + this.address = address; + this.pinnedAddress = pinnedAddress; + } + + @NonNull + @Override + public SocketAddress resolve() { + return pinnedAddress != null ? pinnedAddress : address; + } + + @NonNull + @Override + public EndPoint pinTo(@NonNull SocketAddress resolvedAddress) { + return new TestPinnableEndPoint(address, resolvedAddress); + } + + @NonNull + @Override + public String asMetricPrefix() { + return "test"; + } + + @Override + public boolean equals(Object other) { + return (other instanceof TestPinnableEndPoint) + && address.equals(((TestPinnableEndPoint) other).address); + } + + @Override + public int hashCode() { + return Objects.hash(address); + } + } +} diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryProtocolNegotiationTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryProtocolNegotiationTest.java index fceb8777904..6e868e5cc0b 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryProtocolNegotiationTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryProtocolNegotiationTest.java @@ -25,6 +25,7 @@ import com.datastax.oss.driver.api.core.UnsupportedProtocolVersionException; import com.datastax.oss.driver.api.core.config.DefaultDriverOption; import com.datastax.oss.driver.internal.core.TestResponses; +import com.datastax.oss.driver.internal.core.metadata.DefaultEndPoint; import com.datastax.oss.driver.internal.core.metrics.NoopNodeMetricUpdater; import com.datastax.oss.protocol.internal.Frame; import com.datastax.oss.protocol.internal.ProtocolConstants; @@ -33,6 +34,9 @@ import com.datastax.oss.protocol.internal.response.Ready; import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.UseDataProvider; +import java.net.InetSocketAddress; +import java.net.SocketAddress; +import java.util.Arrays; import java.util.Optional; import java.util.concurrent.CompletionStage; import org.junit.Test; @@ -280,6 +284,168 @@ public void should_fail_if_negotiation_finds_no_matching_version(int errorCode) }); } + @Test + public void should_not_try_next_address_of_identified_node_when_negotiation_exhausts_versions() { + // Given – an *identified* node (its host id is known, so every address its name expands to is + // that same node) whose name expands to two candidates: the same live server twice, so + // whichever the rotation picks first is irrelevant. The server rejects every protocol version. + mockNegotiationLadderDownToV3(); + SocketAddress serverAddress = SERVER_ADDRESS.resolve(); + installResolver(new TestAddressResolverGroup(Arrays.asList(serverAddress, serverAddress))); + ChannelFactory factory = newChannelFactory(); + + // When + CompletionStage channelFuture = + factory.connect( + new DefaultEndPoint(InetSocketAddress.createUnresolved("test.cluster.fake", 9042)), + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE, + true); + + exhaustNegotiationLadder(); + + // Then – the second candidate must not be attempted: for a node we have already identified, a + // protocol-version rejection is a property of the node, not of the address, so replaying the + // negotiation ladder against the remaining IPs would buy nothing. Checked before the future + // assertion so that on regression the stray frame is drained; leaving it unread would block the + // server's exchanger and hang the whole suite in tearDown() instead of failing this test. + assertThat(tryReadOutboundFrame(200)) + .as("second candidate must not be attempted after negotiation exhaustion") + .isNull(); + assertThatStage(channelFuture) + .isFailed( + e -> { + assertThat(e).isInstanceOf(UnsupportedProtocolVersionException.class); + assertThat(((UnsupportedProtocolVersionException) e).getAttemptedVersions()) + .containsExactly(DefaultProtocolVersion.V4, DefaultProtocolVersion.V3); + assertThat(e.getSuppressed()) + .as("no other candidate should have been tried, so nothing to suppress") + .isEmpty(); + }); + } + + @Test + public void + should_try_next_address_of_unidentified_endpoint_when_negotiation_exhausts_versions() { + // Given – the same setup, but for an endpoint the driver has not identified yet: a contact + // point, before host ids have been read. Its name may well expand to addresses of *different* + // nodes, so a version rejection by the first says nothing about the second. + mockNegotiationLadderDownToV3(); + SocketAddress serverAddress = SERVER_ADDRESS.resolve(); + installResolver(new TestAddressResolverGroup(Arrays.asList(serverAddress, serverAddress))); + ChannelFactory factory = newChannelFactory(); + + // When + CompletionStage channelFuture = + factory.connect( + new DefaultEndPoint(InetSocketAddress.createUnresolved("test.cluster.fake", 9042)), + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE, + false); + + // The first candidate exhausts the ladder... + exhaustNegotiationLadder(); + // ...and the second is tried all the same, replaying the ladder from the top. Before this, + // resolve-contact-points=true made each address a separate node and ControlConnection advanced + // to the next one on exactly this error; collapsing a name into one node must not lose that. + exhaustNegotiationLadder(); + + // Then + assertThat(tryReadOutboundFrame(200)) + .as("the name expands to two addresses, so there is no third attempt") + .isNull(); + assertThatStage(channelFuture) + .isFailed( + e -> { + assertThat(e).isInstanceOf(UnsupportedProtocolVersionException.class); + assertThat(((UnsupportedProtocolVersionException) e).getAttemptedVersions()) + .as("each candidate negotiates on its own, so this is the last one's ladder") + .containsExactly(DefaultProtocolVersion.V4, DefaultProtocolVersion.V3); + assertThat(e.getSuppressed()) + .as("the first candidate's failure must still be reported") + .hasSize(1); + assertThat(e.getSuppressed()[0]) + .isInstanceOf(UnsupportedProtocolVersionException.class); + }); + } + + /** Negotiation starts at V4 and has exactly one downgrade available, to V3. */ + private void mockNegotiationLadderDownToV3() { + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + when(protocolVersionRegistry.downgrade(DefaultProtocolVersion.V4)) + .thenReturn(Optional.of(DefaultProtocolVersion.V3)); + when(protocolVersionRegistry.downgrade(DefaultProtocolVersion.V3)).thenReturn(Optional.empty()); + } + + /** + * Plays the server side of a full negotiation ladder against one candidate address: V4 rejected, + * downgrade retry with V3 rejected, i.e. no version left to try on that address. + */ + private void exhaustNegotiationLadder() { + Frame requestFrame = readOutboundFrame(); + assertThat(requestFrame.message).isInstanceOf(Options.class); + writeInboundFrame(requestFrame, TestResponses.supportedResponse("mock_key", "mock_value")); + + requestFrame = readOutboundFrame(); + assertThat(requestFrame.protocolVersion).isEqualTo(DefaultProtocolVersion.V4.getCode()); + writeInboundFrame( + requestFrame, + new Error( + ProtocolConstants.ErrorCode.PROTOCOL_ERROR, "Invalid or unsupported protocol version")); + + requestFrame = readOutboundFrame(); + assertThat(requestFrame.message).isInstanceOf(Options.class); + writeInboundFrame(requestFrame, TestResponses.supportedResponse("mock_key", "mock_value")); + + requestFrame = readOutboundFrame(); + assertThat(requestFrame.protocolVersion).isEqualTo(DefaultProtocolVersion.V3.getCode()); + writeInboundFrame( + requestFrame, + new Error( + ProtocolConstants.ErrorCode.PROTOCOL_ERROR, "Invalid or unsupported protocol version")); + } + + @Test + public void should_fail_future_when_downgrade_lookup_throws_in_connect_listener() { + // Given – a version registry that throws when the factory looks up the downgrade. The lookup + // runs inside the Netty connect listener, which swallows throwables: without the blanket catch + // in connectToAddress() the connect future would never complete and the attempt would hang. + when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false); + when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4); + RuntimeException failure = new IllegalStateException("registry broken"); + when(protocolVersionRegistry.downgrade(DefaultProtocolVersion.V4)).thenThrow(failure); + ChannelFactory factory = newChannelFactory(); + + // When + CompletionStage channelFuture = + factory.connect( + SERVER_ADDRESS, + null, + null, + DriverChannelOptions.DEFAULT, + NoopNodeMetricUpdater.INSTANCE); + + Frame requestFrame = readOutboundFrame(); + assertThat(requestFrame.message).isInstanceOf(Options.class); + writeInboundFrame(requestFrame, TestResponses.supportedResponse("mock_key", "mock_value")); + + requestFrame = readOutboundFrame(); + assertThat(requestFrame.protocolVersion).isEqualTo(DefaultProtocolVersion.V4.getCode()); + // Server does not support v4, which is what sends the factory to the downgrade lookup + writeInboundFrame( + requestFrame, + new Error( + ProtocolConstants.ErrorCode.PROTOCOL_ERROR, "Invalid or unsupported protocol version")); + + // Then + assertThatStage(channelFuture).isFailed(e -> assertThat(e).isSameAs(failure)); + } + /** * Depending on the Cassandra version, an "unsupported protocol" response can use different error * codes, so we test all of them. diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryTestBase.java b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryTestBase.java index 2780d5bdec9..12ab96c59b9 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryTestBase.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryTestBase.java @@ -20,6 +20,8 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.when; import com.datastax.oss.driver.api.core.ProtocolVersion; @@ -42,6 +44,7 @@ import com.datastax.oss.protocol.internal.request.Startup; import com.datastax.oss.protocol.internal.response.Ready; import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import io.netty.bootstrap.Bootstrap; import io.netty.bootstrap.ServerBootstrap; import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufAllocator; @@ -53,6 +56,8 @@ import io.netty.channel.DefaultEventLoopGroup; import io.netty.channel.local.LocalChannel; import io.netty.channel.local.LocalServerChannel; +import io.netty.resolver.AddressResolverGroup; +import java.net.SocketAddress; import java.time.Duration; import java.util.Collections; import java.util.Optional; @@ -188,6 +193,38 @@ protected Frame readOutboundFrame() { return null; // never reached } + /** + * Like {@link #readOutboundFrame()}, but returns {@code null} instead of failing the test when no + * frame arrives within {@code timeoutMillis}. + * + *

Use this to assert that the client did not send another request. Unlike asserting via + * a failing read, it also drains a frame that does arrive: the server-side exchange in {@link + * ServerInitializer} has no timeout, so a stray unread frame would block the server event loop + * and hang the whole suite in {@link #tearDown()} instead of failing just the test. + */ + protected Frame tryReadOutboundFrame(long timeoutMillis) { + try { + return requestFrameExchanger.exchange(null, timeoutMillis, MILLISECONDS); + } catch (InterruptedException e) { + fail("unexpected interruption while waiting for outbound frame", e); + return null; // never reached + } catch (TimeoutException e) { + return null; + } + } + + /** Installs {@code group} the way a user would, through the {@code NettyOptions} hook. */ + protected void installResolver(AddressResolverGroup group) { + doAnswer( + invocation -> { + Bootstrap bootstrap = invocation.getArgument(0); + bootstrap.resolver(group); + return null; + }) + .when(nettyOptions) + .afterBootstrapInitialized(any(Bootstrap.class)); + } + protected void writeInboundFrame(Frame requestFrame, Message response) { writeInboundFrame(requestFrame, response, requestFrame.protocolVersion); } diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/channel/TestAddressResolverGroup.java b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/TestAddressResolverGroup.java new file mode 100644 index 00000000000..06f5f636cba --- /dev/null +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/channel/TestAddressResolverGroup.java @@ -0,0 +1,130 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.datastax.oss.driver.internal.core.channel; + +import edu.umd.cs.findbugs.annotations.Nullable; +import io.netty.bootstrap.Bootstrap; +import io.netty.channel.local.LocalAddress; +import io.netty.resolver.AddressResolver; +import io.netty.resolver.AddressResolverGroup; +import io.netty.util.concurrent.EventExecutor; +import io.netty.util.concurrent.Future; +import io.netty.util.concurrent.Promise; +import java.net.InetSocketAddress; +import java.net.SocketAddress; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; + +/** + * A stand-in for a user-supplied {@code AddressResolverGroup} (e.g. Netty's {@code + * DnsAddressResolverGroup}), installed the way a user would install one: through {@link + * com.datastax.oss.driver.internal.core.context.NettyOptions#afterBootstrapInitialized(Bootstrap)}. + * + *

Records what it was asked to resolve, and answers with a fixed list of addresses so tests can + * assert that every one of them is tried, and in what order. + * + *

Implements {@link AddressResolver} directly rather than extending {@code + * AbstractAddressResolver} so it can hand back {@link LocalAddress}es — the unit tests connect over + * Netty's local transport, which is not reachable through an {@link InetSocketAddress}. + */ +class TestAddressResolverGroup extends AddressResolverGroup { + + /** Every address this group was asked to resolve, in order. */ + final List queried = new CopyOnWriteArrayList<>(); + + /** Whether a resolver was ever obtained from this group at all. */ + volatile boolean resolverRequested; + + /** The executor the last resolver was created for, i.e. the loop resolution runs on. */ + @Nullable volatile EventExecutor resolverExecutor; + + /** The addresses to answer with, or {@code null} to fail every lookup. */ + @Nullable private final List answer; + + /** + * Whether to claim that every address still needs resolving, even one that already carries an IP. + * A real resolver may do this to redirect traffic, and Netty honours it: {@code + * Bootstrap#doResolveAndConnect0} asks the resolver rather than testing the address itself. + */ + private final boolean claimNothingIsResolved; + + TestAddressResolverGroup(@Nullable List answer) { + this(answer, false); + } + + TestAddressResolverGroup(@Nullable List answer, boolean claimNothingIsResolved) { + this.answer = answer; + this.claimNothingIsResolved = claimNothingIsResolved; + } + + @Override + protected AddressResolver newResolver(EventExecutor executor) { + resolverRequested = true; + resolverExecutor = executor; + return new AddressResolver() { + + @Override + public boolean isSupported(SocketAddress address) { + return true; + } + + @Override + public boolean isResolved(SocketAddress address) { + if (claimNothingIsResolved) { + return false; + } + // Only hostnames need resolving; anything else (including the local-transport addresses we + // hand back) is already usable. + return !(address instanceof InetSocketAddress) + || !((InetSocketAddress) address).isUnresolved(); + } + + @Override + public Future resolve(SocketAddress address) { + return resolve(address, executor.newPromise()); + } + + @Override + public Future resolve(SocketAddress address, Promise promise) { + queried.add(address); + return answer == null + ? promise.setFailure(new IllegalStateException("mock resolver failure")) + : promise.setSuccess(answer.get(0)); + } + + @Override + public Future> resolveAll(SocketAddress address) { + return resolveAll(address, executor.newPromise()); + } + + @Override + public Future> resolveAll( + SocketAddress address, Promise> promise) { + queried.add(address); + return answer == null + ? promise.setFailure(new IllegalStateException("mock resolver failure")) + : promise.setSuccess(answer); + } + + @Override + public void close() { + // nothing to do + } + }; + } +} diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicyInitTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicyInitTest.java index b5e843d77d2..9c36cbebfee 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicyInitTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicyInitTest.java @@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -29,9 +30,12 @@ import com.datastax.oss.driver.api.core.config.DriverExecutionProfile; import com.datastax.oss.driver.api.core.loadbalancing.NodeDistance; import com.datastax.oss.driver.api.core.metadata.NodeState; +import com.datastax.oss.driver.internal.core.metadata.DefaultEndPoint; +import com.datastax.oss.driver.internal.core.metadata.DefaultNode; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableSet; import edu.umd.cs.findbugs.annotations.NonNull; +import java.net.InetSocketAddress; import java.util.UUID; import org.junit.Test; import org.junit.runner.RunWith; @@ -210,6 +214,38 @@ public void should_warn_if_configured_dc_matches_no_node() { .isTrue(); } + @Test + public void should_not_warn_about_dc_mismatch_when_the_only_real_node_matches_configured_dc() { + // Given — CUSTOMER-588. A contact point given as a hostname is represented, before the control + // connection resolves it, by an ephemeral placeholder Node (built by + // MetadataManager#addContactPoints via DefaultNode#newContactPoint) whose datacenter is always + // null: it is never populated, because real topology is attached to a *different* Node object + // matched by hostId (see MetadataManager#registerNode). + // + // The removed OptionalLocalDcHelper#checkLocalDatacenterCompatibility compared the configured + // local DC against *those* placeholders, so it warned unconditionally whenever a local DC was + // configured, no matter where the contact points actually were. Here the only node carrying + // real, resolved metadata (node1) genuinely is in the configured local DC ("dc1", per base + // setup). + DefaultNode ephemeralContactPointNode = + DefaultNode.newContactPoint( + new DefaultEndPoint(new InetSocketAddress("127.0.0.9", 9042)), context); + when(metadataManager.getContactPoints()).thenReturn(ImmutableSet.of(ephemeralContactPointNode)); + DefaultLoadBalancingPolicy policy = createPolicy(); + + // When + policy.init(ImmutableMap.of(UUID.randomUUID(), node1), distanceReporter); + + // Then — no WARN at all. The retained check inspects the resolved node map, where node1 + // matches. + // Asserting that nothing is warned, rather than that one particular message is absent, also + // catches a regression that brings the false positive back under different wording. + // should_warn_if_configured_dc_matches_no_node is the positive control for this same appender, + // so a silent capture failure cannot make this pass by accident. + verify(appender, never()).doAppend(argThat(event -> event.getLevel() == Level.WARN)); + assertThat(policy.getLocalDatacenter()).isEqualTo("dc1"); + } + @NonNull protected DefaultLoadBalancingPolicy createPolicy() { return new DefaultLoadBalancingPolicy(context, DriverExecutionProfile.DEFAULT_NAME); diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPointTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPointTest.java index f31dd2861ed..296390c53e7 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPointTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPointTest.java @@ -18,11 +18,12 @@ package com.datastax.oss.driver.internal.core.metadata; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.datastax.oss.driver.api.core.metadata.EndPoint; -import java.io.UncheckedIOException; +import io.netty.channel.local.LocalAddress; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.UnknownHostException; @@ -66,20 +67,23 @@ public void should_fallback_when_resolve_returns_null() throws UnknownHostExcept } @Test - public void should_wrap_io_exceptions_in_unchecked_io_exception() throws UnknownHostException { + public void should_return_the_route_address_unresolved() { + // The route hostname is handed over unresolved on purpose: ChannelFactory resolves it through + // Netty's AddressResolverGroup, so a custom resolver applies to client routes too and no DNS + // lookup runs on the admin event loop that connect() is called from. UUID hostId = UUID.randomUUID(); - when(topologyMonitor.resolve(hostId)).thenThrow(new UnknownHostException("no-such-host")); + InetSocketAddress route = InetSocketAddress.createUnresolved("route.example.com", 9042); + when(topologyMonitor.resolve(hostId)).thenReturn(route); ClientRoutesEndPoint ep = new ClientRoutesEndPoint(topologyMonitor, hostId, null, fallbackEndPoint); - assertThatThrownBy(ep::resolve) - .isInstanceOf(UncheckedIOException.class) - .hasCauseInstanceOf(UnknownHostException.class); + assertThat(ep.resolve()).isSameAs(route); + assertThat(((InetSocketAddress) ep.resolve()).isUnresolved()).isTrue(); } @Test - public void should_reflect_route_changes_on_subsequent_resolve() throws UnknownHostException { + public void should_reflect_route_changes_on_subsequent_resolve() { UUID hostId = UUID.randomUUID(); InetSocketAddress addr1 = new InetSocketAddress("127.0.0.1", 9042); InetSocketAddress addr2 = new InetSocketAddress("10.0.0.1", 9043); @@ -96,6 +100,48 @@ public void should_reflect_route_changes_on_subsequent_resolve() throws UnknownH assertThat(ep.resolve()).isEqualTo(addr2); } + // ---- pinTo() ------------------------------------------------------------ + + @Test + public void pin_to_should_stop_consulting_the_topology_monitor() { + UUID hostId = UUID.randomUUID(); + InetSocketAddress pinnedTo = new InetSocketAddress("127.0.0.1", 9042); + + ClientRoutesEndPoint original = + new ClientRoutesEndPoint(topologyMonitor, hostId, null, fallbackEndPoint); + EndPoint pinned = original.pinTo(pinnedTo); + + assertThat(pinned.resolve()).isEqualTo(pinnedTo); + // No lookup at all -- that is the point: DefaultTopologyMonitor#savePort and the SSL factories + // read the channel's endpoint, and must not trigger a blocking re-resolution there. + verify(topologyMonitor, never()).resolve(hostId); + // Identity is keyed off the host id, so the pinned copy is still the same node. + assertThat(pinned).isEqualTo(original); + assertThat(original).isEqualTo(pinned); + assertThat(pinned.asMetricPrefix()).isEqualTo(original.asMetricPrefix()); + } + + @Test + public void pin_to_should_return_same_instance_when_already_pinned_to_that_address() { + ClientRoutesEndPoint original = + new ClientRoutesEndPoint(topologyMonitor, UUID.randomUUID(), null, fallbackEndPoint); + InetSocketAddress pinnedTo = new InetSocketAddress("127.0.0.1", 9042); + + EndPoint pinned = original.pinTo(pinnedTo); + + assertThat(((ClientRoutesEndPoint) pinned).pinTo(pinnedTo)).isSameAs(pinned); + } + + @Test + public void pin_to_should_be_a_no_op_for_a_non_inet_address() { + // Mirror DefaultEndPoint: an address that cannot be held in an InetSocketAddress field (e.g. + // the local transport used by unit tests) skips pinning rather than failing the connection. + ClientRoutesEndPoint endPoint = + new ClientRoutesEndPoint(topologyMonitor, UUID.randomUUID(), null, fallbackEndPoint); + + assertThat(endPoint.pinTo(new LocalAddress("some-id"))).isSameAs(endPoint); + } + // ---- equals / hashCode -------------------------------------------------- @Test diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesTopologyMonitorTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesTopologyMonitorTest.java index a1ba4617ef5..e6ab0895034 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesTopologyMonitorTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesTopologyMonitorTest.java @@ -28,6 +28,8 @@ import com.datastax.oss.driver.api.core.config.DriverConfig; import com.datastax.oss.driver.api.core.config.DriverExecutionProfile; import com.datastax.oss.driver.api.core.metadata.EndPoint; +import com.datastax.oss.driver.api.core.metadata.Metadata; +import com.datastax.oss.driver.api.core.metadata.Node; import com.datastax.oss.driver.internal.core.adminrequest.AdminResult; import com.datastax.oss.driver.internal.core.adminrequest.AdminRow; import com.datastax.oss.driver.internal.core.channel.DriverChannel; @@ -66,6 +68,8 @@ public class ClientRoutesTopologyMonitorTest { @Mock private ControlConnection controlConnection; @Mock private DriverConfig driverConfig; @Mock private DriverExecutionProfile defaultProfile; + @Mock private MetadataManager metadataManager; + @Mock private Metadata metadata; private TestableClientRoutesTopologyMonitor handler; @@ -194,14 +198,21 @@ public void should_throw_after_close() { } @Test - public void should_throw_for_unresolvable_hostname() { + public void should_not_look_up_the_route_hostname() { UUID hostId = UUID.randomUUID(); - // Use a hostname guaranteed not to resolve + // A hostname guaranteed not to resolve: this must still succeed, because resolve() is a pure + // in-memory cache lookup that hands the name over unresolved. ChannelFactory resolves it later + // through Netty's AddressResolverGroup, so a custom resolver applies to client routes too and + // nothing blocks the admin event loop here. handler.setRoutes( ImmutableMap.of( hostId, new ClientRouteRecord(hostId, "this.host.does.not.exist.invalid", 9042))); - assertThatThrownBy(() -> handler.resolve(hostId)).isInstanceOf(UnknownHostException.class); + InetSocketAddress result = handler.resolve(hostId); + + assertThat(result.isUnresolved()).isTrue(); + assertThat(result.getHostString()).isEqualTo("this.host.does.not.exist.invalid"); + assertThat(result.getPort()).isEqualTo(9042); } @Test @@ -220,6 +231,57 @@ public void should_refresh_updates_routes() throws UnknownHostException { assertThat(handler.resolve(hostId2)).isNotNull(); } + // ---- reresolvesNodeAddresses() ------------------------------------------- + + @Test + public void should_reresolve_when_all_known_nodes_have_client_routes() { + UUID hostId1 = UUID.randomUUID(); + UUID hostId2 = UUID.randomUUID(); + Node node1 = Mockito.mock(Node.class); + when(node1.getHostId()).thenReturn(hostId1); + Node node2 = Mockito.mock(Node.class); + when(node2.getHostId()).thenReturn(hostId2); + + when(context.getMetadataManager()).thenReturn(metadataManager); + when(metadataManager.getMetadata()).thenReturn(metadata); + when(metadata.getNodes()).thenReturn(ImmutableMap.of(hostId1, node1, hostId2, node2)); + + handler.setRoutes( + ImmutableMap.of( + hostId1, new ClientRouteRecord(hostId1, "127.0.0.1", 9042), + hostId2, new ClientRouteRecord(hostId2, "127.0.0.2", 9042))); + + assertThat(handler.reresolvesNodeAddresses()).isTrue(); + } + + @Test + public void should_not_reresolve_when_a_known_node_has_no_client_route() { + UUID hostId1 = UUID.randomUUID(); + UUID hostId2 = UUID.randomUUID(); + Node node1 = Mockito.mock(Node.class); + when(node1.getHostId()).thenReturn(hostId1); + Node node2 = Mockito.mock(Node.class); + when(node2.getHostId()).thenReturn(hostId2); + + when(context.getMetadataManager()).thenReturn(metadataManager); + when(metadataManager.getMetadata()).thenReturn(metadata); + when(metadata.getNodes()).thenReturn(ImmutableMap.of(hostId1, node1, hostId2, node2)); + + // Only node1 has a live client route; node2 would fall back to a static endpoint. + handler.setRoutes(ImmutableMap.of(hostId1, new ClientRouteRecord(hostId1, "127.0.0.1", 9042))); + + assertThat(handler.reresolvesNodeAddresses()).isFalse(); + } + + @Test + public void should_reresolve_when_no_nodes_known_yet() { + when(context.getMetadataManager()).thenReturn(metadataManager); + when(metadataManager.getMetadata()).thenReturn(metadata); + when(metadata.getNodes()).thenReturn(Collections.emptyMap()); + + assertThat(handler.reresolvesNodeAddresses()).isTrue(); + } + // ---- Merge behavior tests ----------------------------------------------- @Test diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.java index 7da8fb39415..c92c7b8e20c 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.java @@ -20,6 +20,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import com.datastax.oss.driver.api.core.metadata.EndPoint; +import io.netty.channel.local.LocalAddress; import java.net.InetSocketAddress; import org.junit.Test; @@ -57,4 +59,107 @@ public void should_reject_null_address() { .isInstanceOf(NullPointerException.class) .hasMessage("address can't be null"); } + + @Test + public void resolve_returns_already_resolved_address_as_is() { + DefaultEndPoint endPoint = new DefaultEndPoint(new InetSocketAddress("127.0.0.1", 9042)); + InetSocketAddress resolved = endPoint.resolve(); + assertThat(resolved.isUnresolved()).isFalse(); + assertThat(resolved.getHostString()).isEqualTo("127.0.0.1"); + } + + @Test + public void resolve_passes_unresolved_hostname_through_without_looking_it_up() { + // This endpoint does NOT resolve hostnames itself. It hands the unresolved address to + // ChannelFactory, which expands it through Netty's AddressResolverGroup so that a custom + // resolver installed via NettyOptions#afterBootstrapInitialized still applies -- a direct + // InetAddress.getAllByName() call here would bypass it, and would block the admin event loop + // that connect() runs on. "localhost" would resolve fine, so this assertion is only meaningful + // because we check the address comes back *unresolved*. + DefaultEndPoint endPoint = + new DefaultEndPoint(InetSocketAddress.createUnresolved("localhost", 9042)); + + InetSocketAddress resolved = endPoint.resolve(); + + assertThat(resolved.isUnresolved()).isTrue(); + assertThat(resolved.getHostString()).isEqualTo("localhost"); + assertThat(resolved.getPort()).isEqualTo(9042); + } + + @Test + public void resolve_does_not_throw_for_unresolvable_hostname() { + // No lookup happens, so an unresolvable name is not an error at this level: the connect attempt + // fails later with a descriptive error instead. + DefaultEndPoint endPoint = + new DefaultEndPoint( + InetSocketAddress.createUnresolved("this-host-does-not-exist.invalid", 9042)); + + assertThat(endPoint.resolve().getHostString()).isEqualTo("this-host-does-not-exist.invalid"); + } + + @Test + public void pin_to_should_override_resolution_but_preserve_identity() { + InetSocketAddress hostname = InetSocketAddress.createUnresolved("test.com", 9042); + DefaultEndPoint original = new DefaultEndPoint(hostname); + InetSocketAddress pinnedTo = new InetSocketAddress("127.0.0.1", 9042); + + EndPoint pinned = original.pinTo(pinnedTo); + + // Resolution now yields exactly the pinned address... + assertThat(pinned.resolve()).isEqualTo(pinnedTo); + // ...but the copy still denotes the same node, and metric names must not change depending on + // which IP a connection happened to land on -- including through toString(), which is what + // TaggingMetricIdGenerator tags node metrics with, and nodes do adopt pinned copies. + assertThat(pinned.asMetricPrefix()).isEqualTo(original.asMetricPrefix()); + assertThat(pinned.toString()).isEqualTo(original.toString()); + assertThat(pinned).isEqualTo(original); + assertThat(pinned.hashCode()).isEqualTo(original.hashCode()); + // Equality has to hold in both directions: endpoints are used as set and map keys. + assertThat(original).isEqualTo(pinned); + // The original is untouched. + assertThat(original.resolve()).isEqualTo(hostname); + } + + @Test + public void pin_to_should_return_same_instance_when_already_pinned_to_that_address() { + DefaultEndPoint original = + new DefaultEndPoint(InetSocketAddress.createUnresolved("test.com", 9042)); + InetSocketAddress pinnedTo = new InetSocketAddress("127.0.0.1", 9042); + + EndPoint pinned = original.pinTo(pinnedTo); + + assertThat(((DefaultEndPoint) pinned).pinTo(pinnedTo)).isSameAs(pinned); + } + + @Test + public void pin_to_should_return_same_instance_when_address_is_already_the_endpoints_own() { + // An already-resolved endpoint expands to exactly one candidate -- itself -- so ChannelFactory + // pins it to the address it already holds. That copy would be indistinguishable from the + // original in every respect, so there is no point allocating it. Every node discovered from the + // peers rows takes this path. + InetSocketAddress resolved = new InetSocketAddress("127.0.0.1", 9042); + DefaultEndPoint endPoint = new DefaultEndPoint(resolved); + + assertThat(endPoint.pinTo(new InetSocketAddress("127.0.0.1", 9042))).isSameAs(endPoint); + assertThat(endPoint.toString()).isEqualTo(resolved.toString()); + } + + @Test + public void pin_to_should_reject_null_address() { + DefaultEndPoint endPoint = new DefaultEndPoint(new InetSocketAddress("127.0.0.1", 9042)); + assertThatThrownBy(() -> endPoint.pinTo(null)) + .isInstanceOf(NullPointerException.class) + .hasMessage("resolvedAddress can't be null"); + } + + @Test + public void pin_to_should_be_a_no_op_for_a_non_inet_address() { + // ChannelFactory pins whatever address it connected to; a non-Inet one (e.g. the local + // transport + // used by unit tests) cannot be held in an InetSocketAddress field, so pinning is skipped + // rather + // than failing the connection. + DefaultEndPoint endPoint = new DefaultEndPoint(new InetSocketAddress("127.0.0.1", 9042)); + assertThat(endPoint.pinTo(new LocalAddress("some-id"))).isSameAs(endPoint); + } } diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultNodeTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultNodeTest.java index 6a53fe3e433..6f9f6916769 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultNodeTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultNodeTest.java @@ -18,9 +18,17 @@ package com.datastax.oss.driver.internal.core.metadata; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import com.datastax.oss.driver.api.core.metadata.EndPoint; +import com.datastax.oss.driver.internal.core.context.InternalDriverContext; import com.datastax.oss.driver.internal.core.context.MockedDriverContextFactory; +import com.datastax.oss.driver.internal.core.metrics.MetricsFactory; +import com.datastax.oss.driver.internal.core.metrics.NodeMetricUpdater; import java.net.InetSocketAddress; import java.util.UUID; import org.junit.Test; @@ -55,4 +63,85 @@ public void should_have_expected_string_representation_if_hostid_is_null() { "Node(endPoint=localhost/127.0.0.1:9042, hostId=null, hashCode=%x)", node.hashCode()); assertThat(node.toString()).isEqualTo(expected); } + + @Test + public void should_adopt_a_newer_endpoint_that_only_differs_by_its_pinned_address() { + // A PinnableEndPoint copy compares equal to the original -- pinnedAddress is excluded from + // equals() by contract, so that a pinned copy still denotes the same node. setEndPoint() must + // therefore not use equals() to decide whether to adopt it: the pinned address is the one every + // subsequent connection to this node will use, so refusing the newer instance would freeze the + // node on the first address it ever connected to, even after the control connection has moved + // and told us about it. + InternalDriverContext context = MockedDriverContextFactory.defaultDriverContext(); + DefaultNode node = new DefaultNode(endPoint, context); + + EndPoint pinnedToFirst = + ((PinnableEndPoint) endPoint).pinTo(new InetSocketAddress("127.0.0.2", 9042)); + node.setEndPoint(pinnedToFirst, context); + assertThat(node.getEndPoint()).isSameAs(pinnedToFirst); + + EndPoint pinnedToSecond = + ((PinnableEndPoint) endPoint).pinTo(new InetSocketAddress("127.0.0.3", 9042)); + // Same node by equals(), different pinned address. + assertThat(pinnedToSecond).isEqualTo(pinnedToFirst); + node.setEndPoint(pinnedToSecond, context); + + assertThat(node.getEndPoint()).isSameAs(pinnedToSecond); + assertThat(node.getEndPoint().resolve()).isEqualTo(new InetSocketAddress("127.0.0.3", 9042)); + } + + @Test + public void should_not_rebuild_the_metric_updater_for_a_pin_only_change() { + // A pinned copy is identified exactly like the original -- same asMetricPrefix(), same + // toString() -- so rebuilding would clear and re-register metrics under identical names, and + // reset their values along the way. + MetricsFactory metricsFactory = mock(MetricsFactory.class); + InternalDriverContext context = contextWith(metricsFactory); + NodeMetricUpdater first = mock(NodeMetricUpdater.class); + NodeMetricUpdater second = mock(NodeMetricUpdater.class); + when(metricsFactory.newNodeUpdater(any())).thenReturn(first, second); + DefaultNode node = new DefaultNode(endPoint, context); + assertThat(node.getMetricUpdater()).isSameAs(first); + + node.setEndPoint( + ((PinnableEndPoint) endPoint).pinTo(new InetSocketAddress("127.0.0.2", 9042)), context); + + assertThat(node.getMetricUpdater()).isSameAs(first); + verify(first, never()).clearMetrics(); + } + + @Test + public void should_rebuild_the_metric_updater_when_an_equal_endpoint_renames_the_metrics() { + // An unresolved hostname and the address it resolves to compare *equal* (see + // DefaultEndPoint#equals) but do not produce the same metric prefix. That is exactly what + // happens when a contact-point node adopts the endpoint built from its system.local row, so + // deciding on equals() alone would leave the node's metrics registered under the hostname while + // asMetricPrefix() had moved on to the IP. + MetricsFactory metricsFactory = mock(MetricsFactory.class); + InternalDriverContext context = contextWith(metricsFactory); + NodeMetricUpdater first = mock(NodeMetricUpdater.class); + NodeMetricUpdater second = mock(NodeMetricUpdater.class); + when(metricsFactory.newNodeUpdater(any())).thenReturn(first, second); + + EndPoint asHostname = + new DefaultEndPoint(InetSocketAddress.createUnresolved("localhost", 9042)); + EndPoint asAddress = new DefaultEndPoint(new InetSocketAddress("127.0.0.1", 9042)); + assertThat(asHostname).isEqualTo(asAddress); + assertThat(asHostname.asMetricPrefix()).isNotEqualTo(asAddress.asMetricPrefix()); + + DefaultNode node = new DefaultNode(asHostname, context); + assertThat(node.getMetricUpdater()).isSameAs(first); + + node.setEndPoint(asAddress, context); + + assertThat(node.getMetricUpdater()).isSameAs(second); + verify(first).clearMetrics(); + } + + /** A context whose only stubbed behaviour is the metrics factory {@code DefaultNode} asks for. */ + private static InternalDriverContext contextWith(MetricsFactory metricsFactory) { + InternalDriverContext context = mock(InternalDriverContext.class); + when(context.getMetricsFactory()).thenReturn(metricsFactory); + return context; + } } diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapperTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapperTest.java index 89b36b9ee09..8c635982eea 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapperTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapperTest.java @@ -40,10 +40,11 @@ import com.datastax.oss.driver.internal.core.context.EventBus; import com.datastax.oss.driver.internal.core.context.InternalDriverContext; import com.datastax.oss.driver.internal.core.metrics.MetricsFactory; +import com.datastax.oss.driver.internal.core.util.collection.QueryPlan; +import com.datastax.oss.driver.internal.core.util.collection.SimpleQueryPlan; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableList; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableMap; import com.datastax.oss.driver.shaded.guava.common.collect.ImmutableSet; -import com.datastax.oss.driver.shaded.guava.common.collect.Lists; import java.util.Map; import java.util.Objects; import java.util.Queue; @@ -79,6 +80,7 @@ public class LoadBalancingPolicyWrapperTest { private EventBus eventBus; @Mock private MetadataManager metadataManager; @Mock private Metadata metadata; + @Mock private TopologyMonitor topologyMonitor; @Mock protected MetricsFactory metricsFactory; @Captor private ArgumentCaptor> initNodesCaptor; @@ -102,13 +104,16 @@ public void setup() { when(metadata.getNodes()).thenReturn(allNodes); when(metadataManager.getContactPoints()).thenReturn(contactPoints); when(context.getMetadataManager()).thenReturn(metadataManager); + when(context.getTopologyMonitor()).thenReturn(topologyMonitor); when(context.getConfig()).thenReturn(config); when(config.getDefaultProfile()).thenReturn(defaultProfile); when(defaultProfile.getBoolean(DefaultDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS)) .thenReturn(false); - defaultPolicyQueryPlan = Lists.newLinkedList(ImmutableList.of(node3, node2, node1)); + // Use a real built-in QueryPlan (not a mutable LinkedList): its add()/addAll() throw, so the + // control-reconnection plan must compose rather than mutate it (see CompositeQueryPlan usage). + defaultPolicyQueryPlan = new SimpleQueryPlan(node3, node2, node1); when(policy1.newQueryPlan(null, null)).thenReturn(defaultPolicyQueryPlan); eventBus = spy(new EventBus("test")); @@ -130,26 +135,28 @@ public void setup() { @Test public void should_build_control_connection_query_plan_from_contact_points_before_init() { - // When + // When — before init, the control-reconnection plan is built straight from the contact points + // (bypassing the load balancing policies), so each hostname can be tried on the first connect. Queue queryPlan = wrapper.newControlReconnectionQueryPlan(); - // Then + // Then — query plan contains the contact points, and no policy was consulted for (LoadBalancingPolicy policy : ImmutableList.of(policy1, policy2, policy3)) { verify(policy, never()).newQueryPlan(null, null); } - assertThat(queryPlan).hasSameElementsAs(contactPoints); + assertThat(queryPlan).containsExactlyInAnyOrder(node1, node2); } @Test public void should_build_query_plan_from_contact_points_before_init() { - // When + // When — before init, the query plan is built straight from the contact points (bypassing the + // load balancing policies) Queue queryPlan = wrapper.newQueryPlan(null, DriverExecutionProfile.DEFAULT_NAME, null); - // Then + // Then — query plan contains the contact points, and no policy was consulted for (LoadBalancingPolicy policy : ImmutableList.of(policy1, policy2, policy3)) { verify(policy, never()).newQueryPlan(null, null); } - assertThat(queryPlan).hasSameElementsAs(contactPoints); + assertThat(queryPlan).containsExactlyInAnyOrder(node1, node2); } @Test @@ -204,8 +211,7 @@ public void should_fetch_control_connection_query_plan_from_policy_after_init() assertThat(queryPlan.poll()).isEqualTo(node3); assertThat(queryPlan.poll()).isEqualTo(node2); assertThat(queryPlan.poll()).isEqualTo(node1); - // Remaining nodes are contact points appended at the end. - // They are new DefaultNode instances created via newContactPoint, so compare by endpoint. + // Remaining nodes are the original contact points appended at the end. Set remainingEndpoints = new java.util.HashSet<>(); for (Node n : queryPlan) { remainingEndpoints.add(n.getEndPoint()); @@ -217,14 +223,92 @@ public void should_fetch_control_connection_query_plan_from_policy_after_init() assertThat(remainingEndpoints).isEqualTo(contactEndpoints); } + @Test + public void should_not_duplicate_contact_points_before_init() { + // Given — the wrapper hasn't been init()-ed yet (state=BEFORE_INIT), so newQueryPlan() already + // builds the regular plan directly from the contact points. The reconnect-contact-points flag + // doesn't matter here: newControlReconnectionQueryPlan() short-circuits on state before even + // reading it, since appending contact points again pre-init would just duplicate every entry in + // the plan. + + // When + Queue queryPlan = wrapper.newControlReconnectionQueryPlan(); + + // Then — the contact points are read only once (not once for the regular plan and again for a + // redundant "fallback" append), and the plan has no duplicate entries. + verify(metadataManager, times(1)).getContactPoints(); + assertThat(queryPlan).containsExactlyInAnyOrder(node1, node2); + } + + @Test + public void + should_not_append_contact_points_to_query_plan_when_reconnect_contact_points_is_disabled() { + // Given — the flag defaults to false in the test setup (see @Before) + wrapper.init(); + + // When + Queue queryPlan = wrapper.newControlReconnectionQueryPlan(); + + // Then + // Only the policy query plan is returned; no contact points are appended. + assertThat(queryPlan).isEqualTo(defaultPolicyQueryPlan); + } + + @Test + public void + should_not_append_contact_points_to_query_plan_when_topology_monitor_reresolves_addresses() { + // Given — the flag is enabled, but the topology monitor re-resolves node addresses on its own + // (e.g. a proxy-based monitor such as client routes or the cloud SNI proxy). + when(defaultProfile.getBoolean(DefaultDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS)) + .thenReturn(true); + when(topologyMonitor.reresolvesNodeAddresses()).thenReturn(true); + wrapper.init(); + + // When + Queue queryPlan = wrapper.newControlReconnectionQueryPlan(); + + // Then + // Contact points must not be appended: the monitor keeps addresses fresh, and appending raw + // contact points could resurrect nodes it has authoritatively removed. + assertThat(queryPlan).isEqualTo(defaultPolicyQueryPlan); + } + + @Test + public void + should_append_contact_points_when_query_plan_empty_even_if_topology_monitor_reresolves() { + // Given — the flag is enabled and the topology monitor re-resolves node addresses on its own, + // but the live-node query plan is empty. With no node to try, reconnection can only recover + // through the contact-point fallback, so it must be appended despite the re-resolving monitor. + when(defaultProfile.getBoolean(DefaultDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS)) + .thenReturn(true); + when(topologyMonitor.reresolvesNodeAddresses()).thenReturn(true); + wrapper.init(); + when(policy1.newQueryPlan(null, null)).thenReturn(QueryPlan.EMPTY); + + // When + Queue queryPlan = wrapper.newControlReconnectionQueryPlan(); + + // Then — the contact points are appended (compare by endpoint since they are new instances) + assertThat(queryPlan.size()).isEqualTo(contactPoints.size()); + Set resultEndpoints = new java.util.HashSet<>(); + for (Node n : queryPlan) { + resultEndpoints.add(n.getEndPoint()); + } + Set contactEndpoints = new java.util.HashSet<>(); + for (DefaultNode n : contactPoints) { + contactEndpoints.add(n.getEndPoint()); + } + assertThat(resultEndpoints).isEqualTo(contactEndpoints); + } + @Test public void should_return_contact_points_when_query_plan_empty_and_flag_enabled() { // Given when(defaultProfile.getBoolean(DefaultDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS)) .thenReturn(true); wrapper.init(); - // Make the policy return an empty query plan - when(policy1.newQueryPlan(null, null)).thenReturn(Lists.newLinkedList(ImmutableList.of())); + // Make the policy return an empty query plan (QueryPlan.EMPTY, as the real policies do) + when(policy1.newQueryPlan(null, null)).thenReturn(QueryPlan.EMPTY); // When Queue queryPlan = wrapper.newControlReconnectionQueryPlan(); diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/SniEndPointTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/SniEndPointTest.java new file mode 100644 index 00000000000..63484ec21d9 --- /dev/null +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/metadata/SniEndPointTest.java @@ -0,0 +1,139 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.datastax.oss.driver.internal.core.metadata; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.InetSocketAddress; +import org.junit.Test; + +public class SniEndPointTest { + + @Test + public void resolve_returns_the_proxy_address_as_is_without_looking_it_up() { + // The proxy address is a hostname (that is how CloudConfigFactory builds it) and this endpoint + // must not resolve it: ChannelFactory expands it through Netty's AddressResolverGroup, which is + // what makes a custom resolver apply to the SNI proxy too, and what keeps resolve() safe to + // call + // from an event loop -- SniSslEngineFactory#newSslEngine does exactly that. + InetSocketAddress proxy = InetSocketAddress.createUnresolved("proxy.example.com", 9042); + SniEndPoint endPoint = new SniEndPoint(proxy, "test-server-name"); + + assertThat(endPoint.resolve()).isSameAs(proxy); + assertThat(endPoint.resolve().isUnresolved()).isTrue(); + } + + @Test + public void should_keep_a_resolved_proxy_hostname_unresolved() { + // InetSocketAddress(String, int) resolves eagerly, so a hostname passed to + // withCloudProxyAddress() arrives here already bound to one of its IPs. Storing it that way + // would freeze every Cloud connection on that IP for the life of the session: resolve() hands + // the stored address straight to the connection layer, which only expands unresolved ones. + InetSocketAddress resolvedProxy = new InetSocketAddress("localhost", 9042); + assertThat(resolvedProxy.isUnresolved()).isFalse(); + + SniEndPoint endPoint = new SniEndPoint(resolvedProxy, "test-server-name"); + + assertThat(endPoint.resolve().isUnresolved()).isTrue(); + assertThat(endPoint.resolve().getHostString()).isEqualTo("localhost"); + assertThat(endPoint.resolve().getPort()).isEqualTo(9042); + // Normalization is unconditional, so endpoints built from either form of the same proxy still + // denote the same node -- equals() keys on the stored address. + assertThat(endPoint) + .isEqualTo( + new SniEndPoint( + InetSocketAddress.createUnresolved("localhost", 9042), "test-server-name")); + // The metric prefix is unaffected either way: it was already built from the host string. + assertThat(endPoint.asMetricPrefix()).isEqualTo("localhost:9042_test-server-name"); + } + + @Test + public void should_keep_a_proxy_given_as_an_ip_address_as_is() { + // Nothing to re-resolve: an IP address is the final answer, and turning it into an unresolved + // "hostname" would only send a literal through the resolver on every connect. + InetSocketAddress ipProxy = new InetSocketAddress("127.0.0.1", 9042); + + SniEndPoint endPoint = new SniEndPoint(ipProxy, "test-server-name"); + + assertThat(endPoint.resolve()).isSameAs(ipProxy); + assertThat(endPoint.resolve().isUnresolved()).isFalse(); + } + + @Test + public void resolve_does_not_throw_for_unresolvable_proxy_hostname() { + // No lookup happens here, so an unresolvable name only fails later, at connect time. + SniEndPoint endPoint = + new SniEndPoint( + InetSocketAddress.createUnresolved("this-host-does-not-exist.invalid", 9042), + "test-server-name"); + + assertThat(endPoint.resolve().getHostString()).isEqualTo("this-host-does-not-exist.invalid"); + } + + @Test + public void pin_to_should_make_resolve_return_the_connected_proxy_ip_and_preserve_identity() { + // Pinning is what lets SniSslEngineFactory#newSslEngine -- which runs inside Netty's channel + // initializer -- see the very proxy IP the channel is connected to, rather than the hostname or + // another A-record. + SniEndPoint original = + new SniEndPoint( + InetSocketAddress.createUnresolved("proxy.example.com", 9042), "test-server-name"); + InetSocketAddress pinnedTo = new InetSocketAddress("127.0.0.1", 9042); + + SniEndPoint pinned = (SniEndPoint) original.pinTo(pinnedTo); + + assertThat(pinned.resolve()).isEqualTo(pinnedTo); + assertThat(pinned.resolve().isUnresolved()).isFalse(); + // The original is untouched. + assertThat(original.resolve().isUnresolved()).isTrue(); + + // The pinned copy still denotes the same node, down to every string it is identified by: the + // tagging MetricIdGenerator tags node metrics with the endpoint's toString(), and nodes do + // adopt + // pinned copies. + assertThat(pinned.getServerName()).isEqualTo(original.getServerName()); + assertThat(pinned.asMetricPrefix()).isEqualTo(original.asMetricPrefix()); + assertThat(pinned.toString()).isEqualTo(original.toString()); + assertThat(pinned).isEqualTo(original); + assertThat(original).isEqualTo(pinned); + assertThat(pinned.hashCode()).isEqualTo(original.hashCode()); + } + + @Test + public void pin_to_should_return_same_instance_when_already_pinned_to_that_address() { + SniEndPoint original = + new SniEndPoint( + InetSocketAddress.createUnresolved("proxy.example.com", 9042), "test-server-name"); + InetSocketAddress pinnedTo = new InetSocketAddress("127.0.0.1", 9042); + + SniEndPoint pinned = (SniEndPoint) original.pinTo(pinnedTo); + + assertThat(pinned.pinTo(pinnedTo)).isSameAs(pinned); + } + + @Test + public void pin_to_should_return_same_instance_when_address_is_the_proxy_address_itself() { + // Only reachable when the proxy was given as an IP address (Cloud supplies a hostname, which is + // stored unresolved): pinning to the very address the endpoint holds is a no-op, so there is no + // point allocating a copy that would be indistinguishable from it. + InetSocketAddress resolvedProxy = new InetSocketAddress("127.0.0.1", 9042); + SniEndPoint endPoint = new SniEndPoint(resolvedProxy, "test-server-name"); + + assertThat(endPoint.pinTo(new InetSocketAddress("127.0.0.1", 9042))).isSameAs(endPoint); + } +} diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/util/AddressUtilsTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/util/AddressUtilsTest.java new file mode 100644 index 00000000000..f48d70ef838 --- /dev/null +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/util/AddressUtilsTest.java @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.datastax.oss.driver.internal.core.util; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.InetAddress; +import java.net.InetSocketAddress; +import org.junit.Test; + +public class AddressUtilsTest { + + @Test + public void should_recognize_a_hostname_whether_resolved_or_not() { + assertThat( + AddressUtils.carriesName(InetSocketAddress.createUnresolved("host.example.com", 9042))) + .isTrue(); + // Eagerly resolved by the constructor, but still a name. + assertThat(AddressUtils.carriesName(new InetSocketAddress("localhost", 9042))).isTrue(); + } + + @Test + public void should_not_mistake_an_ip_literal_for_a_hostname() throws Exception { + assertThat(AddressUtils.carriesName(new InetSocketAddress("127.0.0.1", 9042))).isFalse(); + assertThat(AddressUtils.carriesName(InetSocketAddress.createUnresolved("127.0.0.1", 9042))) + .isFalse(); + assertThat(AddressUtils.carriesName(InetSocketAddress.createUnresolved("::1", 9042))).isFalse(); + // Built from raw bytes, so it carries no name at all and getHostString() falls back to the + // literal -- without triggering the reverse lookup that getHostName() would. + assertThat( + AddressUtils.carriesName( + new InetSocketAddress(InetAddress.getByAddress(new byte[] {10, 0, 0, 1}), 9042))) + .isFalse(); + } + + @Test + public void should_report_an_explicitly_named_address_as_a_name() throws Exception { + // A resolver may label its results with a name of its own; that is still a name. + assertThat( + AddressUtils.carriesName( + new InetSocketAddress( + InetAddress.getByAddress("cname.example.com", new byte[] {10, 0, 0, 1}), 9042))) + .isTrue(); + } +} diff --git a/integration-tests/src/test/java/com/datastax/oss/driver/core/clientroutes/ClientRoutesIT.java b/integration-tests/src/test/java/com/datastax/oss/driver/core/clientroutes/ClientRoutesIT.java index 9a23f368546..e658f843360 100644 --- a/integration-tests/src/test/java/com/datastax/oss/driver/core/clientroutes/ClientRoutesIT.java +++ b/integration-tests/src/test/java/com/datastax/oss/driver/core/clientroutes/ClientRoutesIT.java @@ -186,12 +186,12 @@ private void requireSystemClientRoutesTable(CqlSession admin) { } private InetSocketAddress tryResolve(ClientRoutesTopologyMonitor handler, UUID hostId) { + // resolve() is an in-memory cache lookup that hands the route hostname over unresolved -- the + // connection layer resolves it -- so the only failure left is the monitor being closed. try { return handler.resolve(hostId); } catch (IllegalStateException e) { return null; - } catch (UnknownHostException e) { - throw new RuntimeException("DNS resolution failed for host_id=" + hostId, e); } } @@ -206,7 +206,9 @@ private NodeClassification classifyNodes(CqlSession session) { NodeClassification result = new NodeClassification(); for (Node node : session.getMetadata().getNodes().values()) { InetSocketAddress addr = (InetSocketAddress) node.getEndPoint().resolve(); - String ip = addr.getAddress().getHostAddress(); + // getHostString() rather than getAddress().getHostAddress(): a client route is handed over + // unresolved (the connection layer resolves it), so getAddress() is null for proxied nodes. + String ip = addr.getHostString(); UUID hostId = node.getHostId(); boolean connected = node.getOpenConnections() > 0; LOG.info( @@ -319,7 +321,8 @@ private Map collectHostIds(CcmBridge ccm, int nodeCount, String t .build()) { for (Node node : adminSession.getMetadata().getNodes().values()) { InetSocketAddress addr = (InetSocketAddress) node.getEndPoint().resolve(); - String ip = addr.getAddress().getHostAddress(); + // See classifyNodes(): a client route is unresolved, so getAddress() would be null. + String ip = addr.getHostString(); Integer nodeId = ipToNodeId.get(ip); if (nodeId != null && node.getHostId() != null) { hostIds.put(nodeId, node.getHostId()); @@ -540,7 +543,10 @@ public void should_refresh_routes_after_table_update() throws Exception { () -> { InetSocketAddress resolved = handler.resolve(hostId); assertThat(resolved).isNotNull(); - assertThat(resolved.getAddress().getHostAddress()).isEqualTo(nodeAddr); + // The route is returned unresolved on purpose -- ChannelFactory resolves it through + // Netty's AddressResolverGroup -- so assert on the host string, not getAddress(). + assertThat(resolved.isUnresolved()).isTrue(); + assertThat(resolved.getHostString()).isEqualTo(nodeAddr); assertThat(resolved.getPort()).isEqualTo(9042); }); } diff --git a/integration-tests/src/test/java/com/datastax/oss/driver/core/heartbeat/HeartbeatIT.java b/integration-tests/src/test/java/com/datastax/oss/driver/core/heartbeat/HeartbeatIT.java index 26658bd76d1..b8a32c68450 100644 --- a/integration-tests/src/test/java/com/datastax/oss/driver/core/heartbeat/HeartbeatIT.java +++ b/integration-tests/src/test/java/com/datastax/oss/driver/core/heartbeat/HeartbeatIT.java @@ -235,6 +235,10 @@ private CqlSession newSession(ProgrammaticDriverConfigLoaderBuilder loaderBuilde .withDuration(DefaultDriverOption.HEARTBEAT_TIMEOUT, Duration.ofMillis(500)) .withDuration(DefaultDriverOption.CONNECTION_INIT_QUERY_TIMEOUT, Duration.ofSeconds(2)) .withDuration(DefaultDriverOption.RECONNECTION_MAX_DELAY, Duration.ofSeconds(1)) + // These tests exercise heartbeat behavior only. Disable the contact-point + // reconnection fallback, which would otherwise send an extra OPTIONS message on + // init/reconnect and skew the heartbeat counts. + .withBoolean(DefaultDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS, false) .build(); return SessionUtils.newSession(SIMULACRON_RULE, loader); } diff --git a/integration-tests/src/test/java/com/datastax/oss/driver/core/resolver/MockResolverIT.java b/integration-tests/src/test/java/com/datastax/oss/driver/core/resolver/MockResolverIT.java index 4e9eefebf63..6ae767389eb 100644 --- a/integration-tests/src/test/java/com/datastax/oss/driver/core/resolver/MockResolverIT.java +++ b/integration-tests/src/test/java/com/datastax/oss/driver/core/resolver/MockResolverIT.java @@ -25,7 +25,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import com.datastax.oss.driver.api.core.CqlSession; @@ -84,7 +83,6 @@ public void should_connect_with_mocked_hostname() { DriverConfigLoader loader = new DefaultProgrammaticDriverConfigLoaderBuilder() - .withBoolean(TypedDriverOption.RESOLVE_CONTACT_POINTS.getRawOption(), false) .withBoolean(TypedDriverOption.RECONNECT_ON_INIT.getRawOption(), true) .withStringList( TypedDriverOption.CONTACT_POINTS.getRawOption(), @@ -107,9 +105,54 @@ public void should_connect_with_mocked_hostname() { .filter(x -> x.toString().contains("test.cluster.fake")) .collect(Collectors.toSet()); assertThat(filteredNodes).hasSize(1); - InetSocketAddress address = - (InetSocketAddress) filteredNodes.iterator().next().getEndPoint().resolve(); - assertTrue(address.isUnresolved()); + Node node = filteredNodes.iterator().next(); + InetSocketAddress address = (InetSocketAddress) node.getEndPoint().resolve(); + // ChannelFactory pins the control connection's endpoint to the address it actually reached, + // and DefaultTopologyMonitor#buildNodeEndPoint stores that copy for the control node, so + // resolution yields that concrete IP rather than the hostname. + assertFalse(address.isUnresolved()); + assertThat(address.getAddress().getHostAddress()).isEqualTo(ccmBridge.getNodeIpAddress(1)); + // The pinned copy still denotes the same node by hostname though -- it is what the filter + // above matched on -- so metric names do not depend on which IP a connection landed on. + assertThat(node.getEndPoint().asMetricPrefix()).isEqualTo("test_cluster_fake:9042"); + } + } + } + + @Test + public void should_connect_when_first_dns_entry_is_non_responsive() { + final int numberOfNodes = 2; + DriverConfigLoader loader = + new DefaultProgrammaticDriverConfigLoaderBuilder() + .withBoolean(TypedDriverOption.RECONNECT_ON_INIT.getRawOption(), true) + .withStringList( + TypedDriverOption.CONTACT_POINTS.getRawOption(), + Collections.singletonList("test.cluster.fake:9042")) + .build(); + + CqlSessionBuilder builder = new CqlSessionBuilder().withConfigLoader(loader); + try (CcmBridge ccmBridge = + CcmBridge.builder().withNodes(numberOfNodes).withIpPrefix("127.0.1.").build()) { + MultimapHostResolverProvider.removeResolverEntries("test.cluster.fake"); + // Register the dead IP first so it's the first entry InetAddress.getAllByName() returns for + // this hostname (MultimapHostResolver preserves insertion order). Node 11 is never started, + // so nothing listens on 127.0.1.11 in this subnet. + MultimapHostResolverProvider.addResolverEntry("test.cluster.fake", "127.0.1.11"); + MultimapHostResolverProvider.addResolverEntry( + "test.cluster.fake", ccmBridge.getNodeIpAddress(1)); + MultimapHostResolverProvider.addResolverEntry( + "test.cluster.fake", ccmBridge.getNodeIpAddress(2)); + ccmBridge.create(); + ccmBridge.start(); + + try (CqlSession session = builder.build()) { + waitForAllNodesUp(session, numberOfNodes); + ResultSet rs = session.execute("select * from system.local where key='local'"); + assertThat(rs).isNotNull(); + List rows = rs.all(); + assertThat(rows).hasSize(1); + Collection nodes = session.getMetadata().getNodes().values(); + assertThat(nodes).hasSize(numberOfNodes); } } } @@ -119,7 +162,6 @@ public void replace_cluster_test() { final int numberOfNodes = 3; DriverConfigLoader loader = new DefaultProgrammaticDriverConfigLoaderBuilder() - .withBoolean(TypedDriverOption.RESOLVE_CONTACT_POINTS.getRawOption(), false) .withBoolean(TypedDriverOption.RECONNECT_ON_INIT.getRawOption(), true) .withStringList( TypedDriverOption.CONTACT_POINTS.getRawOption(), @@ -206,7 +248,6 @@ public void run_replace_test_20_times() { public void cannot_reconnect_with_resolved_socket() { DriverConfigLoader loader = new DefaultProgrammaticDriverConfigLoaderBuilder() - .withBoolean(TypedDriverOption.RESOLVE_CONTACT_POINTS.getRawOption(), false) .withBoolean(TypedDriverOption.RECONNECT_ON_INIT.getRawOption(), true) .withStringList( TypedDriverOption.CONTACT_POINTS.getRawOption(), diff --git a/manual/core/address_resolution/README.md b/manual/core/address_resolution/README.md index ae44feea3ea..c692aa63336 100644 --- a/manual/core/address_resolution/README.md +++ b/manual/core/address_resolution/README.md @@ -183,13 +183,19 @@ datastax-java-driver { #### DNS resolution -DNS is resolved at connection time (not at route discovery time). The driver delegates to -`InetAddress.getByName()`, which is a blocking call that uses the JVM's built-in DNS cache -(30 s default TTL in the JDK). Because this runs on Netty I/O threads, slow or unresponsive -DNS can block connection establishment and impact driver throughput. To mitigate this, configure -the JVM DNS cache TTL via the `networkaddress.cache.ttl` security property (e.g. in -`$JAVA_HOME/conf/security/java.security` or programmatically with -`java.security.Security.setProperty("networkaddress.cache.ttl", "60")`). +DNS is resolved at connection time (not at route discovery time), and through the same mechanism as +every other address the driver connects to: the route's hostname is handed to the connection layer +unresolved, and Netty's configured `AddressResolverGroup` expands it. A custom resolver installed via +`NettyOptions.afterBootstrapInitialized()` therefore applies to client routes as well, and a hostname +that maps to several addresses has all of them tried in turn. + +With Netty's default (JDK) resolver the lookup is a blocking `InetAddress` call that uses the JVM's +built-in DNS cache (30 s default TTL in the JDK). It runs on a Netty I/O event loop — never on the +admin event loop that drives control-connection reconnects — so it delays the connection attempt +itself. It is therefore worth configuring the JVM DNS cache TTL via the `networkaddress.cache.ttl` +security property (e.g. in `$JAVA_HOME/conf/security/java.security` or programmatically with +`java.security.Security.setProperty("networkaddress.cache.ttl", "60")`), or installing +`DnsAddressResolverGroup` for non-blocking resolution. - **Route-map refresh** — the driver re-queries `system.client_routes` and atomically swaps the in-memory route map in two situations: diff --git a/upgrade_guide/README.md b/upgrade_guide/README.md index 214399dacc7..3e016fbb5fe 100644 --- a/upgrade_guide/README.md +++ b/upgrade_guide/README.md @@ -19,6 +19,51 @@ under the License. ## Upgrade guide +### 4.19.2.1 + +#### Contact points are expanded to all their DNS addresses at connection time + +Contact points backed by a hostname are now kept unresolved and expanded to **all** the IP +addresses the hostname maps to, at connection time. Previously only the first address returned by +DNS was tried, so a single non-responsive IP behind a multi-record hostname could fail the initial +connection (or a control-connection reconnect) even when the other addresses were healthy. No +configuration change is required to benefit from this. + +The expansion goes through Netty's configured `AddressResolverGroup`, which is the resolver an +unresolved address already reached when it was handed to `Bootstrap.connect()`. A custom resolver +installed through `NettyOptions.afterBootstrapInitialized()` therefore keeps working, and +`Bootstrap.disableResolver()` is still honored. As before, with Netty's default (JDK) resolver the +lookup blocks the I/O event loop it runs on — install `DnsAddressResolverGroup` for non-blocking +resolution. + +The same now applies to the two other address sources that used to perform their own JVM DNS lookups: +the Cloud/SNI proxy address and client-route hostnames are expanded by the connection layer too. A +custom Netty resolver applies to them for the first time, and a proxy hostname with several A-records +has all of them tried within a single connection attempt. + +There is **no public API change**: `EndPoint.resolve()` keeps its signature and is not deprecated. +Third-party `EndPoint` implementations keep working unchanged, with one new expectation — an +implementation should return its address as-is rather than looking names up itself, since resolution +now happens in the connection layer and `resolve()` is called from an event loop. + +As part of this change: + +- `advanced.resolve-contact-points` is deprecated and now has **no effect**. Contact points are + always kept as unresolved hostnames and expanded at connection time. An already-resolved + `InetSocketAddress` passed programmatically is still used as provided, with no further expansion. +- `advanced.control-connection.reconnection.fallback-to-original-contact-points` now defaults to + `true` (previously `false`). This is also the driver's DNS re-resolution path: metadata nodes + hold an already-resolved endpoint that is never re-resolved, so on control-connection reconnect + the driver falls back to the original contact points to pick up current DNS records once the + live-node plan is exhausted. Set it to `false` to restore the previous behavior. +- For advanced deployments that provide a custom `NettyOptions`: the + `afterBootstrapInitialized()` hook now runs once per logical connection to a node (previously + once per attempt, including protocol-version downgrade retries), and it receives the bootstrap + *before* the driver installs its channel handler — a handler set by the hook is replaced, and + the driver logs a one-time warning if it detects one. Use the hook for channel options, + attributes and `Bootstrap.resolver(...)`; use `afterChannelInitialized()` for pipeline + customization. + ### 4.19.0.7 #### Cloud private-endpoint support via client routes