Skip to content

Commit 1d0b540

Browse files
nikagraclaude
andcommitted
feat: fall back to contact points on control reconnect (DRIVER-201)
Default fallback-to-original-contact-points to true: once a control reconnection round has exhausted the live nodes, the retained, still-unresolved contact points are appended, so Netty resolves their names again and a cluster that moved to new addresses is found (#215). Every metadata node, the control node included, otherwise holds an address resolved once. Monitors that re-resolve on their own opt out through TopologyMonitor.reresolvesNodeAddresses(), unless the live-node plan is empty. HeartbeatIT turns the fallback off, since it counts every init OPTIONS as a heartbeat; MockResolverIT gains the recovery case. Fixes #215 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent f08b4a7 commit 1d0b540

18 files changed

Lines changed: 432 additions & 19 deletions

File tree

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

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -701,8 +701,21 @@ public enum DefaultDriverOption implements DriverOption {
701701
CONTROL_CONNECTION_AGREEMENT_WARN("advanced.control-connection.schema-agreement.warn-on-failure"),
702702

703703
/**
704-
* Whether to forcibly add original contact points held by MetadataManager to the reconnection
705-
* plan, in case there is no live nodes available according to LBP. Experimental.
704+
* Whether to append the original contact points held by MetadataManager to the control
705+
* connection's reconnection plan, after the live nodes reported by the load balancing policy.
706+
* Defaults to {@code true}.
707+
*
708+
* <p>This is the driver's DNS re-resolution path. A metadata node holds an address resolved once
709+
* and never re-resolved, the node the control connection reached included; a contact point given
710+
* as a hostname is kept unresolved (the default) and looked up again through Netty's resolver on
711+
* each connect, so once the live nodes are exhausted the contact points find a cluster that moved
712+
* to new addresses, as soon as the JVM's DNS cache ({@code networkaddress.cache.ttl}) has
713+
* expired. A resolved contact point ({@code advanced.resolve-contact-points = true}, or a
714+
* programmatic resolved address) is appended as it is and never re-resolved.
715+
*
716+
* <p>Skipped when the topology monitor re-resolves node addresses on its own ({@code
717+
* TopologyMonitor#reresolvesNodeAddresses()}: the Cloud SNI proxy, and client routes with full
718+
* route coverage), unless the live-node plan is empty.
706719
*
707720
* <p>Value-type: boolean
708721
*/

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ protected static void fillWithDriverDefaults(OptionsMap map) {
369369
map.put(TypedDriverOption.CONTROL_CONNECTION_AGREEMENT_INTERVAL, Duration.ofMillis(200));
370370
map.put(TypedDriverOption.CONTROL_CONNECTION_AGREEMENT_TIMEOUT, Duration.ofSeconds(10));
371371
map.put(TypedDriverOption.CONTROL_CONNECTION_AGREEMENT_WARN, true);
372-
map.put(TypedDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS, false);
372+
map.put(TypedDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS, true);
373373
map.put(TypedDriverOption.PREPARE_ON_ALL_NODES, true);
374374
map.put(TypedDriverOption.REPREPARE_ENABLED, true);
375375
map.put(TypedDriverOption.REPREPARE_CHECK_SYSTEM_TABLE, false);

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,15 @@ public String toString() {
600600
public static final TypedDriverOption<Boolean> CONTROL_CONNECTION_AGREEMENT_WARN =
601601
new TypedDriverOption<>(
602602
DefaultDriverOption.CONTROL_CONNECTION_AGREEMENT_WARN, GenericType.BOOLEAN);
603-
/** Whether to forcibly try original contacts if no live nodes are available */
603+
/**
604+
* Whether to append the original contact points to the control connection's reconnection plan,
605+
* after the live nodes reported by the load balancing policy (defaults to {@code true}).
606+
*
607+
* <p>A contact point given as a hostname is kept unresolved and looked up again on each connect,
608+
* so this is how the driver picks up changed DNS records once the live nodes are exhausted. It is
609+
* skipped for a topology monitor that re-resolves node addresses on its own, unless the live-node
610+
* plan is empty.
611+
*/
604612
public static final TypedDriverOption<Boolean> CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS =
605613
new TypedDriverOption<>(
606614
DefaultDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS, GenericType.BOOLEAN);

core/src/main/java/com/datastax/oss/driver/internal/core/control/ControlConnection.java

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@
6161
import java.util.List;
6262
import java.util.Map;
6363
import java.util.Map.Entry;
64-
import java.util.Objects;
6564
import java.util.Queue;
6665
import java.util.WeakHashMap;
6766
import java.util.concurrent.CompletableFuture;
@@ -694,12 +693,16 @@ private boolean isControlNode(Node eventNode) {
694693
&& eventNode.getHostId().equals(state.current.getHostId())) {
695694
return true;
696695
}
697-
if (state.current == null
698-
&& state.pending != null
699-
&& Objects.equals(eventNode.getEndPoint(), state.pending.getEndPoint())) {
700-
return true;
701-
}
702-
return false;
696+
// Reference identity, not endpoint equality: the reconnection plan now appends the
697+
// unresolved contact-point nodes (fallback-to-original-contact-points defaults to true), so a
698+
// mixed resolved/unresolved pair is routine here, and DefaultEndPoint.equals resolves the
699+
// unresolved side of one -- a blocking lookup on this admin executor, during the very DNS
700+
// outage the fallback exists for. Nodes carry identity semantics (no equals override, and
701+
// lastNodeState/lastNodeDistance already key on the instance) and connect() stores the plan's
702+
// own node as pending, so an event about that node carries the same object. A mixed pair that
703+
// is not the same object is a contact point we have not identified yet; the resolve step is
704+
// about to replace it with the registered node, after which the hostId branch above answers.
705+
return state.current == null && state.pending != null && eventNode == state.pending;
703706
}
704707

705708
private void onDistanceEvent(DistanceEvent event) {

core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesTopologyMonitor.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.datastax.oss.driver.api.core.config.ClientRoutesConfig;
2222
import com.datastax.oss.driver.api.core.config.DefaultDriverOption;
2323
import com.datastax.oss.driver.api.core.metadata.EndPoint;
24+
import com.datastax.oss.driver.api.core.metadata.Node;
2425
import com.datastax.oss.driver.internal.core.adminrequest.AdminRequestHandler;
2526
import com.datastax.oss.driver.internal.core.adminrequest.AdminResult;
2627
import com.datastax.oss.driver.internal.core.adminrequest.AdminRow;
@@ -480,6 +481,27 @@ protected EndPoint buildNodeEndPoint(
480481
return new ClientRoutesEndPoint(this, hostId, broadcastInetAddress, fallback);
481482
}
482483

484+
@Override
485+
public boolean reresolvesNodeAddresses() {
486+
// A ClientRoutesEndPoint looks its route hostname up on every connect, but only while a route
487+
// exists for that host id; without one it falls back to a static, already-resolved address. So
488+
// this answers true only while every known node has a live route, and "every" has to mean at
489+
// least one: an empty node set (before the first refresh, or after every node was removed) must
490+
// not suppress the contact-point fallback at the one moment it is the only way back. A closed
491+
// monitor re-resolves nothing at all, however complete its cache still looks.
492+
//
493+
// A route only reaches DNS if the address column it came from holds a name rather than an IP
494+
// literal, and nothing validates that today (scylladb/java-driver#1064). An all-literal
495+
// deployment therefore answers true here and suppresses the contact-point fallback; tightening
496+
// this waits on that column's contract.
497+
if (closed) {
498+
return false;
499+
}
500+
// getNodes() is already keyed by host id, so this is exactly "every known node has a route".
501+
Map<UUID, Node> nodes = context.getMetadataManager().getMetadata().getNodes();
502+
return !nodes.isEmpty() && resolvedRoutesCache.get().keySet().containsAll(nodes.keySet());
503+
}
504+
483505
/**
484506
* Builds the CQL query to fetch client routes.
485507
*

core/src/main/java/com/datastax/oss/driver/internal/core/metadata/CloudTopologyMonitor.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,12 @@ protected EndPoint buildNodeEndPoint(
4444
UUID hostId = Objects.requireNonNull(row.getUuid("host_id"));
4545
return new SniEndPoint(cloudProxyAddress, hostId.toString());
4646
}
47+
48+
@Override
49+
public boolean reresolvesNodeAddresses() {
50+
// Every node is reached through the SNI proxy, whose hostname SniEndPoint#resolve() looks up on
51+
// every call, so addresses stay current on their own: appending the contact points as a DNS
52+
// fallback would add nothing, and could resurrect nodes this monitor has removed.
53+
return true;
54+
}
4755
}

core/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,11 +193,19 @@ public Queue<Node> newControlReconnectionQueryPlan() {
193193
// is an immutable QueryPlan (add()/addAll() throw), so concatenate rather than mutate. The
194194
// nodes retained by MetadataManager are appended, not fresh copies: their identity stays stable
195195
// across reconnection rounds, and no throwaway node is minted per round.
196+
//
197+
// A monitor that re-resolves node addresses itself (the Cloud SNI proxy, client routes with
198+
// full coverage) keeps them fresh without this fallback, and appending raw contact points could
199+
// resurrect nodes it has removed -- unless the live-node plan is empty, in which case there is
200+
// nothing else to try. isEmpty() on a policy's plan is size() == 0 through AbstractCollection;
201+
// QueryPlan's contract names this call, since it makes "size() never throws" load-bearing.
196202
if (state == State.RUNNING
197203
&& context
198204
.getConfig()
199205
.getDefaultProfile()
200-
.getBoolean(DefaultDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS)) {
206+
.getBoolean(DefaultDriverOption.CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS)
207+
&& (!context.getTopologyMonitor().reresolvesNodeAddresses()
208+
|| regularQueryPlan.isEmpty())) {
201209
Object[] contactNodes = context.getMetadataManager().getContactPoints().toArray();
202210
ArrayUtils.shuffleHead(contactNodes, contactNodes.length);
203211
return new CompositeQueryPlan(regularQueryPlan, new SimpleQueryPlan(contactNodes));

core/src/main/java/com/datastax/oss/driver/internal/core/metadata/MetadataManager.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,11 @@ public boolean wasImplicitContactPoint() {
188188
* they are never added to metadata and never exposed to user-facing APIs (events, {@link
189189
* com.datastax.oss.driver.api.core.metadata.Metadata#getNodes()}, or {@link
190190
* com.datastax.oss.driver.api.core.metadata.NodeStateListener} callbacks).
191+
*
192+
* <p>The metadata node stores {@code nodeInfo.getEndPoint()} as-is and never re-resolves it. A
193+
* contact-point hostname is re-read only through the control connection's reconnection fallback
194+
* ({@code advanced.control-connection.reconnection.fallback-to-original-contact-points}), which
195+
* re-offers the retained, still-unresolved contact points to Netty's resolver on each connect.
191196
*/
192197
public CompletionStage<Node> registerNode(NodeInfo nodeInfo) {
193198
Preconditions.checkNotNull(nodeInfo.getHostId(), "Cannot register node without hostId");

core/src/main/java/com/datastax/oss/driver/internal/core/metadata/TopologyMonitor.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,4 +141,24 @@ public interface TopologyMonitor extends AsyncAutoCloseable {
141141
* {@link DefaultTopologyMonitor}) should override this method.
142142
*/
143143
default void resetColumnCaches() {}
144+
145+
/**
146+
* Whether this monitor re-resolves node addresses on every connection attempt (for example by
147+
* handing out a proxy hostname to be looked up at connect time), rather than registering an
148+
* address resolved once.
149+
*
150+
* <p>When {@code true}, the control connection's reconnection plan does not append the original
151+
* contact points as a DNS re-resolution fallback (see {@code
152+
* advanced.control-connection.reconnection.fallback-to-original-contact-points}) unless the
153+
* live-node plan is empty: the monitor keeps addresses fresh on its own, and appending raw
154+
* contact points could resurrect nodes it has removed.
155+
*
156+
* <p>The default is {@code false}, which is right for {@link DefaultTopologyMonitor}: peers hold
157+
* a resolved address from the peers table, and the node the control connection reached is
158+
* registered under the address it reached, so neither re-reads DNS. Proxy-based monitors override
159+
* this.
160+
*/
161+
default boolean reresolvesNodeAddresses() {
162+
return false;
163+
}
144164
}

core/src/main/java/com/datastax/oss/driver/internal/core/util/collection/QueryPlan.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,10 @@
3939
* methods throw.
4040
*
4141
* <p>Both {@link #size()} and {@link #iterator()} are supported and never throw, even if called
42-
* concurrently. These methods are implemented for reporting purposes only, the driver itself does
43-
* not use them.
42+
* concurrently. They exist mainly for reporting, and the request path does not use them; the one
43+
* driver caller is {@code LoadBalancingPolicyWrapper#newControlReconnectionQueryPlan}, which asks
44+
* {@code isEmpty()} (that is, {@code size() == 0}) of the plan a policy returned, so a custom
45+
* implementation that throws from {@code size()} breaks control-connection reconnection.
4446
*
4547
* <p>All built-in {@link QueryPlan} implementations can be safely reused for custom load balancing
4648
* policies; if you plan to do so, study the source code of {@link

0 commit comments

Comments
 (0)