fix: expand contact point hostnames to all DNS IPs at connection time (DRIVER-201) — Part 1/2#889
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to address DRIVER-201 by ensuring a hostname contact point can be tried via all DNS-resolved IPs (instead of only the first), and begins deprecating RESOLVE_CONTACT_POINTS as a no-op to preserve unresolved hostnames until connection time.
Changes:
- Deprecates
RESOLVE_CONTACT_POINTSinDefaultDriverOption/TypedDriverOptionand stops reading it inSessionBuilder(always merges contact points withresolve=false). - Adds
MetadataManager#getResolvedContactPoints()plus unit tests that validate DNS expansion / passthrough behavior. - Updates comments/tests around contact-point handling in load-balancing wrapper tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| core/src/main/java/com/datastax/oss/driver/internal/core/metadata/MetadataManager.java | Adds getResolvedContactPoints() that expands unresolved hostname contact points to all DNS IPs. |
| core/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.java | Adjusts comments around contact point handling (but currently still uses getContactPoints() in query plan construction). |
| core/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.java | Stops honoring RESOLVE_CONTACT_POINTS; always stores contact points as unresolved. |
| core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java | Marks RESOLVE_CONTACT_POINTS deprecated with updated Javadoc. |
| core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java | Marks RESOLVE_CONTACT_POINTS deprecated with updated Javadoc. |
| core/src/test/java/com/datastax/oss/driver/internal/core/metadata/MetadataManagerTest.java | Adds tests for getResolvedContactPoints() behavior (null state, passthrough, DNS expansion). |
| core/src/test/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapperTest.java | Updates an assertion comment related to appended contact points. |
Comments suppressed due to low confidence (1)
core/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.java:176
- The new comment says DNS expansion is handled by ChannelFactory at connection time, but ChannelFactory.connect() currently just calls endPoint.resolve() once and bootstrap.connect(resolvedAddress), which won’t iterate over all A/AAAA records for an unresolved InetSocketAddress. Either update this logic to actually use getResolvedContactPoints() here, or implement/point to the connection-time DNS expansion in ChannelFactory; as-is, the comment (and intended fallback behavior) is incorrect.
// Use the original (potentially unresolved) contact-point endpoints so that the control
// connection channel retains the hostname, preserving hostname-based node identity in
// metadata. DNS expansion to all IPs for each hostname is handled by ChannelFactory at
// actual connection time.
Set<DefaultNode> originalNodes = context.getMetadataManager().getContactPoints();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR deprecates the RESOLVE_CONTACT_POINTS option and defers DNS expansion of contact-point hostnames until query-plan time. It adds MetadataManager.getResolvedContactPoints(), stops address resolution during session initialization, updates LoadBalancingPolicyWrapper and other consumers to use resolved contact points, simplifies OptionalLocalDcHelper logic, and adjusts unit and integration tests to reflect DNS-expanded endpoints. Sequence DiagramsequenceDiagram
participant SessionBuilder
participant LoadBalancingPolicyWrapper
participant MetadataManager
participant DNS
SessionBuilder->>LoadBalancingPolicyWrapper: initialize with unresolved contact points
LoadBalancingPolicyWrapper->>MetadataManager: getResolvedContactPoints()
MetadataManager->>MetadataManager: iterate contactPoints
MetadataManager->>DNS: InetAddress.getAllByName(hostname)
DNS-->>MetadataManager: resolved IPs
MetadataManager-->>LoadBalancingPolicyWrapper: expanded list of nodes with resolved addresses
LoadBalancingPolicyWrapper->>LoadBalancingPolicyWrapper: build query plan with resolved nodes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
68e37e8 to
317c73c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
core/src/test/java/com/datastax/oss/driver/internal/core/metadata/MetadataManagerTest.java (3)
493-551: ⚡ Quick winConsider adding negative test cases for DNS resolution.
The current tests cover happy-path scenarios but don't verify error handling for:
- DNS resolution failures (e.g., unknown hostname triggering
UnknownHostException)- Null or empty hostname handling
- Non-
InetSocketAddressendpoint types (if supported by the API)Adding these cases would improve robustness validation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/src/test/java/com/datastax/oss/driver/internal/core/metadata/MetadataManagerTest.java` around lines 493 - 551, Add negative tests to MetadataManagerTest covering DNS failures and invalid endpoints: create a test that adds an unresolved hostname that is guaranteed to fail resolution (e.g., "nonexistent.invalid") via metadataManager.addContactPoints(new DefaultEndPoint(InetSocketAddress.createUnresolved(...))) and assert getResolvedContactPoints() either returns empty or handles the UnknownHostException path as the implementation expects; add a test that passes a null/empty hostname using InetSocketAddress.createUnresolved("", 9042) and assert proper handling; and add a test that supplies a non-InetSocketAddress EndPoint (if supported by EndPoint implementations) and assert getResolvedContactPoints() rejects or skips it. Ensure tests reference getResolvedContactPoints, metadataManager.addContactPoints, DefaultEndPoint, and Node to locate behavior under test.
531-551: 💤 Low valueTest assertion doesn't verify independent expansion.
The test verifies
size >= 2(line 543), which only confirms at least one node from each contact point. However, iflocalhostresolves to multiple IPs (e.g.,127.0.0.1and::1on IPv6 systems), the test doesn't verify that all those IPs are included.This means the "independently" aspect of the test name isn't fully validated—if expansion silently dropped some IPs, the test would still pass.
Consider asserting a more specific count based on known localhost resolution behavior, or document that the test only validates minimum expansion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/src/test/java/com/datastax/oss/driver/internal/core/metadata/MetadataManagerTest.java` around lines 531 - 551, The test should explicitly verify that the unresolved contact point (unresolvedEndPoint) is expanded into all of its resolved addresses rather than only asserting a minimum total size; update should_expand_multiple_contact_points_independently to resolve unresolvedEndPoint (via unresolvedEndPoint.resolve()), collect all resulting InetSocketAddress instances, and assert that for each resolved address there is a corresponding Node in metadataManager.getResolvedContactPoints() whose resolved EndPoint equals that InetSocketAddress, while still asserting that resolvedEndPoint is present (use the existing resolvedEndPoint, metadataManager.getResolvedContactPoints(), and node.getEndPoint()/resolve() to locate matches).
512-529: 💤 Low valueTest doesn't verify expansion to ALL IPs.
The test name claims
should_expand_unresolved_hostname_to_all_ips, but the assertion only checksisNotEmpty()(line 523). This doesn't verify thatInetAddress.getAllByName()actually returned multiple IPs or all available IPs for the hostname.Consider either:
- Using a hostname known to resolve to multiple IPs and asserting the exact count, or
- Renaming the test to
should_expand_unresolved_hostname_to_resolved_ipsto match what's actually verified.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/src/test/java/com/datastax/oss/driver/internal/core/metadata/MetadataManagerTest.java` around lines 512 - 529, The test method should either assert multiple resolved addresses or be renamed to match its current assertions: update the test in MetadataManagerTest.should_expand_unresolved_hostname_to_all_ips to either (a) use a hostname known to resolve to multiple IPs and assert the expected count returned by metadataManager.getResolvedContactPoints (ensuring each Node from getResolvedContactPoints has a resolved InetSocketAddress with the expected port), or (b) rename the method to should_expand_unresolved_hostname_to_resolved_ips and keep the current assertions that each resolved endpoint is not unresolved and uses port 9042; adjust any assertions or test data around metadataManager.addContactPoints and getResolvedContactPoints accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@core/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.java`:
- Around line 938-943: Update the Javadoc for the addContactPoints method in
SessionBuilder to reflect that contact points are now stored as unresolved
hostnames and expanded to all DNS IPs at query-plan time (via
MetadataManager.getResolvedContactPoints) instead of being manually expanded
with InetAddress#getAllByName or controlled by the deprecated
advanced.resolve-contact-points option; remove or rephrase the lines that
instruct users to call InetAddress#getAllByName or reference
RESOLVE_CONTACT_POINTS/advanced.resolve-contact-points and instead document the
new resolution behavior and where users can find configuration guidance.
---
Nitpick comments:
In
`@core/src/test/java/com/datastax/oss/driver/internal/core/metadata/MetadataManagerTest.java`:
- Around line 493-551: Add negative tests to MetadataManagerTest covering DNS
failures and invalid endpoints: create a test that adds an unresolved hostname
that is guaranteed to fail resolution (e.g., "nonexistent.invalid") via
metadataManager.addContactPoints(new
DefaultEndPoint(InetSocketAddress.createUnresolved(...))) and assert
getResolvedContactPoints() either returns empty or handles the
UnknownHostException path as the implementation expects; add a test that passes
a null/empty hostname using InetSocketAddress.createUnresolved("", 9042) and
assert proper handling; and add a test that supplies a non-InetSocketAddress
EndPoint (if supported by EndPoint implementations) and assert
getResolvedContactPoints() rejects or skips it. Ensure tests reference
getResolvedContactPoints, metadataManager.addContactPoints, DefaultEndPoint, and
Node to locate behavior under test.
- Around line 531-551: The test should explicitly verify that the unresolved
contact point (unresolvedEndPoint) is expanded into all of its resolved
addresses rather than only asserting a minimum total size; update
should_expand_multiple_contact_points_independently to resolve
unresolvedEndPoint (via unresolvedEndPoint.resolve()), collect all resulting
InetSocketAddress instances, and assert that for each resolved address there is
a corresponding Node in metadataManager.getResolvedContactPoints() whose
resolved EndPoint equals that InetSocketAddress, while still asserting that
resolvedEndPoint is present (use the existing resolvedEndPoint,
metadataManager.getResolvedContactPoints(), and node.getEndPoint()/resolve() to
locate matches).
- Around line 512-529: The test method should either assert multiple resolved
addresses or be renamed to match its current assertions: update the test in
MetadataManagerTest.should_expand_unresolved_hostname_to_all_ips to either (a)
use a hostname known to resolve to multiple IPs and assert the expected count
returned by metadataManager.getResolvedContactPoints (ensuring each Node from
getResolvedContactPoints has a resolved InetSocketAddress with the expected
port), or (b) rename the method to
should_expand_unresolved_hostname_to_resolved_ips and keep the current
assertions that each resolved endpoint is not unresolved and uses port 9042;
adjust any assertions or test data around metadataManager.addContactPoints and
getResolvedContactPoints accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1d3e1e1e-4557-4536-ab87-dac1f64c0d44
📒 Files selected for processing (7)
core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.javacore/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.javacore/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/MetadataManager.javacore/src/test/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapperTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/metadata/MetadataManagerTest.java
4f07446 to
36e7af3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/helper/OptionalLocalDcHelper.java`:
- Around line 85-86: The warning is triggered by comparing localDc in
OptionalLocalDcHelper.checkLocalDatacenterCompatibility against per-IP synthetic
nodes from MetadataManager.getResolvedContactPoints() whose
DefaultNode.newContactPoint() leaves datacenter null; update the compatibility
check to either iterate over MetadataManager.getContactPoints() instead of
getResolvedContactPoints(), or skip nodes where node.getDatacenter() == null
when comparing to localDc (and avoid logging those as mismatches). Locate
checkLocalDatacenterCompatibility and replace the resolved-contact-points loop
with one using getContactPoints(), or add a null-check on node.getDatacenter()
before comparing and logging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4f2c96b7-838d-4718-8fb1-41be6a1e3299
📒 Files selected for processing (7)
core/src/main/java/com/datastax/dse/driver/internal/core/insights/InsightsClient.javacore/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/helper/OptionalLocalDcHelper.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.javacore/src/main/java/com/datastax/oss/driver/internal/core/metadata/MetadataManager.javacore/src/test/java/com/datastax/dse/driver/internal/core/insights/InsightsClientTest.javacore/src/test/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapperTest.javaintegration-tests/src/test/java/com/datastax/oss/driver/core/resolver/MockResolverIT.java
🚧 Files skipped from review as they are similar to previous changes (2)
- core/src/main/java/com/datastax/oss/driver/internal/core/metadata/MetadataManager.java
- core/src/test/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapperTest.java
4a5d3a0 to
4b04062
Compare
4b04062 to
970ce4a
Compare
|
Given the metadata behavior introduced here, I think
Making the fallback default |
dkropachev
left a comment
dkropachev
left a comment
There was a problem hiding this comment.
One bit of intent is still not obvious from the PR description/code comments: this change does not only expand DNS contact points for the current attempt. A successful DNS-expanded candidate can become the persisted metadata endpoint.
The flow is: getResolvedContactPoints() creates synthetic nodes with resolved IP-backed DefaultEndPoints, the successful control channel keeps that endpoint, DefaultTopologyMonitor.buildNodeEndPoint() returns localEndPoint for system.local, and MetadataManager.registerNode() stores nodeInfo.getEndPoint() in the real metadata node. Since DefaultEndPoint.resolve() just returns the stored InetSocketAddress, later reconnects can keep using that IP unless the original-contact-point fallback path re-enters DNS.
Please make that behavior explicit. Suggested places:
- PR description: add a behavioral note that original contact points remain unresolved, but successful DNS-expanded endpoints may be stored in metadata as resolved IP endpoints.
MetadataManager.getResolvedContactPoints(): mention that returned synthetic nodes are IP-backed connection candidates, not just hostname wrappers.MetadataManager.registerNode()Javadoc or nearby code: mention that the metadata node storesnodeInfo.getEndPoint()as-is, so a control node resolved via a synthetic IP candidate persists that resolved endpoint.- If a follow-up touches
DefaultTopologyMonitor.buildNodeEndPoint(), thesystem.localbranch should say that returninglocalEndPointintentionally persists the endpoint used by the successful control channel.
This also reinforces why advanced.control-connection.reconnection.fallback-to-original-contact-points should default to true: once metadata contains resolved IP endpoints, fallback to the original unresolved contact points is the path that re-resolves DNS after the metadata/LBP plan is exhausted.
dkropachev
left a comment
dkropachev
left a comment
There was a problem hiding this comment.
Please also document the TLS/SNI implication of the DNS expansion in both the PR description and the getResolvedContactPoints() docstring.
The implementation creates each synthetic endpoint with new InetSocketAddress(ip, port), where ip comes from InetAddress.getAllByName(originalHostname). With the JDK, that usually preserves the original hostname in the InetAddress, so DefaultSslEngineFactory / ProgrammaticSslEngineFactory still pass the original contact point hostname to SSLContext.createSSLEngine(host, port) even though the TCP connect uses the selected IP. That matters for hostname verification and implicit SNI.
This is subtle and easy to regress if someone later changes the synthetic endpoint construction to use a raw IP string/address without the original hostname. Please state explicitly that DNS-expanded synthetic endpoints are resolved for TCP connection selection but must retain the original hostname for TLS peer host/SNI/hostname verification purposes.
Suggested PR-description note:
TLS note: DNS-expanded contact point candidates connect to a selected resolved IP, but the synthetic `InetSocketAddress` is built from the `InetAddress` returned by resolving the original hostname. This preserves the original hostname for the SSL engine peer host, so hostname verification and implicit SNI continue to use the configured contact point hostname rather than the numeric IP.And a similar sentence in MetadataManager.getResolvedContactPoints() would make the code intent clear.
|
@dkropachev thanks, addressed all three in 30a585f. 1. 2. Metadata persistence made explicit (your review) 3. TLS/SNI implication made explicit (your review) |
6040121 to
b88b71c
Compare
b88b71c to
30a585f
Compare
… (DRIVER-201) When RESOLVE_CONTACT_POINTS=false (the default) a hostname contact point was stored as a single unresolved InetSocketAddress, so the query plan tried only the first DNS IP. Keep contact points unresolved and expand each hostname to all its DNS IPs at query-plan time via MetadataManager.getResolvedContactPoints(), so the driver falls back to the next candidate when one IP is unreachable. Default advanced.control-connection.reconnection.fallback-to-original-contact-points to true (no longer Experimental): it is the DNS re-resolution path on reconnect. Metadata nodes hold an already-resolved endpoint that is never re-resolved, so falling back to the original unresolved contact points re-expands the hostname to its current DNS IPs. Document that DNS-expanded contact points are IP-backed connection candidates that may be persisted in metadata, and that each synthetic endpoint retains the original hostname (built from the resolved InetAddress) so TLS peer host / SNI / hostname verification keep using the configured hostname. Gate the control-connection reconnection contact-point fallback behind a new TopologyMonitor.reresolvesNodeAddresses() (default false; true for the proxy-based ClientRoutesTopologyMonitor and CloudTopologyMonitor). Those monitors reach nodes through endpoints that already re-resolve on every connection attempt and maintain an authoritative node set, so appending raw contact points to their reconnection plan is unnecessary and could resurrect removed nodes (PrivateLink/Cloud regression safety). Remove OptionalLocalDcHelper.checkLocalDatacenterCompatibility(): it warned when a contact point reported a different datacenter than the configured local DC. Since commit 12e6acb switched initial metadata refresh to hostId-only matching, contact-point nodes are never reused and their datacenter stays null; comparing a configured local DC against that null made the check fire as a false positive for every contact point whenever local-datacenter was set on the default profile, rather than surface a real mismatch. The node-based "configured local DC matches no node" warning (against discovered nodes whose datacenters are populated) is retained, so the only user-visible effect is that the spurious warning is no longer emitted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
30a585f to
a4ccfa6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
core/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.java:173
- This public API documentation is internally inconsistent: unresolved addresses passed programmatically are also wrapped in
DefaultEndPointand expanded bygetResolvedContactPoints(), so they are not used as-is and expansion is not limited to config-based contact points. Document the resolved-versus-unresolved distinction so callers understand the actual behavior.
* <p>Addresses passed here are used as-is. For config-based contact points specified as
* hostnames, the driver automatically expands each hostname to all its DNS-mapped IPs at query
* plan time (via {@code MetadataManager.getResolvedContactPoints()}), so passing a single
* hostname in the configuration is sufficient to try all its IPs on initial connect. The {@code
* advanced.resolve-contact-points} option is deprecated and has no effect.
| InetSocketAddress address = | ||
| (InetSocketAddress) filteredNodes.iterator().next().getEndPoint().resolve(); | ||
| assertTrue(address.isUnresolved()); | ||
| assertFalse(address.isUnresolved()); |
| // and only happen on init / control reconnect. #890 moves multi-address resolution into | ||
| // the EndPoint API / ChannelFactory (EndPoint.resolveAll()); revisit when that lands. | ||
| try { | ||
| InetAddress[] all = InetAddress.getAllByName(address.getHostString()); |
| // route hostname (keyed by host_id) on every connection attempt. Appending the original | ||
| // contact points as a DNS re-resolution fallback is therefore unnecessary and could resurrect | ||
| // nodes that this monitor has authoritatively removed. | ||
| return true; |
Problem
When
RESOLVE_CONTACT_POINTS=false(the default), a contact point hostname was stored as a single unresolvedInetSocketAddress. At connection time the load-balancing query plan contained exactly oneNodeper hostname, so only the first IP returned by DNS was ever tried. If that IP was non-responsive the driver raisedAllNodesFailedExceptionwith no fallback to other IPs the hostname might resolve to.This is particularly impactful in dynamic DNS environments where a hostname maps to multiple nodes and the first one may be temporarily unavailable.
Fixes DRIVER-201.
Changes
DefaultDriverOption/TypedDriverOptionRESOLVE_CONTACT_POINTSis now@Deprecated. Contact points are always kept as unresolved hostnames; the option is a no-op.SessionBuilderRESOLVE_CONTACT_POINTSread;ContactPoints.merge()is now always called withresolve=false, deferring all DNS expansion to connection time.MetadataManagergetResolvedContactPoints()method: for each contact point backed by an unresolved hostname, callsInetAddress.getAllByName()to obtain all IPs and creates a syntheticDefaultNodefor each. Already-resolved and non-InetSocketAddressendpoints are returned as-is.LoadBalancingPolicyWrappernewQueryPlan()(inBEFORE_INIT/DURING_INITstates) andnewControlReconnectionQueryPlan()now callgetResolvedContactPoints()instead ofgetContactPoints(), so the query plan contains one entry per resolved IP. The driver naturally falls back to the next candidate if one is unreachable.TopologyMonitor(new capability method)TopologyMonitor.reresolvesNodeAddresses(), a default method returningfalse.trueinClientRoutesTopologyMonitorandCloudTopologyMonitor. These proxy-based monitors reach nodes throughClientRoutesEndPoint/SniEndPoint, which re-resolve their hostname on every connection attempt, so they never rely on the contact-point fallback for fresh DNS.LoadBalancingPolicyWrapper.newControlReconnectionQueryPlan()now appends the original contact points only when the flag is enabled and the active topology monitor does not re-resolve addresses. This prevents a regression (see Regression safety below) where an authoritative proxy-based monitor could have nodes it removed resurrected by the contact-point fallback.InsightsClientgetContactPoints()→getResolvedContactPoints(), so the startup insights payload reports the resolved contact-point IPs, consistent with the new resolution model.OptionalLocalDcHelper(dead-code cleanup)protected checkLocalDatacenterCompatibility(localDc, contactPoints)method and its call. It readcontactPoint.getDatacenter()and warned when a contact point reported a datacenter different from the configured local DC.scylla-4.x, independent of this PR. The check only ever worked in the original DataStax model, whereInitialNodeListRefreshmatched discovered nodes to contact points by endpoint and mutated the same contact-pointDefaultNodeinstance with its real datacenter. Commit12e6acb90b("Resolve control node identity via system.local before initial metadata refresh", already merged toscylla-4.x) switched refresh matching to hostId-only. Contact-point nodes carry nohostId, so they are never reused and theirdatacenterfield staysnullforever — meaningcheckLocalDatacenterCompatibilitycould never fire.discoverLocalDc(), which iterates the discovered cluster nodes (whose DCs are populated) and warns with the available DCs. That check is unchanged and strictly more reliable than the removed contact-point check.protectedin aninternalpackage (no public-API/binary-compat impact); the in-tree subclassesInferringLocalDcHelper/MandatoryLocalDcHelperdo not override or call it. Removing it is cleanup consistent with deferred resolution (contact points are now unresolved hostnames with no DC), not a behavior change.Reconnection fallback default
advanced.control-connection.reconnection.fallback-to-original-contact-pointsnow defaults totrue(and is no longer marked Experimental). This is the DNS re-resolution path: metadata nodes hold an already-resolved endpoint that is never re-resolved, so on control-connection reconnect the driver must fall back to the original unresolved contact points, which re-entersgetResolvedContactPoints()and re-expands each hostname to its current DNS IPs.OptionsMapwas flipped totruein lockstep withreference.conf(kept in sync; enforced byMapBasedDriverConfigLoaderTest).Tests
MetadataManagerTestcases forgetResolvedContactPoints(): not-yet-set state, already-resolved passthrough, single-hostname DNS expansion, multi-endpoint expansion, and unresolvable-hostname skip.LoadBalancingPolicyWrapperTest: updated to stubgetResolvedContactPoints()/getTopologyMonitor(), plus 3 new cases — contact points appended when the flag is enabled, not appended when disabled, and not appended when the topology monitor re-resolves addresses (even with the flag on).HeartbeatIT: pinsCONTROL_CONNECTION_RECONNECT_CONTACT_POINTS=falsefor its sessions. These tests useSimulacronRule(→DefaultTopologyMonitor) and exercise heartbeat behavior, not contact-point reconnection; without the pin, the newly-enabled fallback sends an extraOPTIONSduring init and skews heartbeat counts.MockResolverIT.should_connect_when_first_dns_entry_is_non_responsive: 2-node CCM cluster on127.0.1.x; first DNS entry points to a non-existent IP (127.0.1.11); asserts the session opens successfully against the real nodes.Behavioral notes
Metadata persistence. The DNS-expanded synthetic contact points are IP-backed connection candidates, not mere hostname wrappers. If one becomes the control node, its resolved IP endpoint is stored as-is in metadata (
MetadataManager.registerNode()keepsnodeInfo.getEndPoint()verbatim). SinceDefaultEndPoint.resolve()returns that stored address without re-querying DNS, later reconnects keep using that IP until the original-contact-point fallback re-resolves the hostname — which is why that fallback now defaults totrue.TLS / SNI. Each synthetic endpoint is built from the
InetAddressreturned by resolving the original hostname (new InetSocketAddress(ip, port)), which retains that hostname. So the TCP connection uses the selected IP while the SSL engine still receives the original contact-point hostname for peer host, implicit SNI, and hostname verification. This is intentional and must be preserved: constructing the endpoint from a raw IP string instead would break hostname verification and implicit SNI.Regression safety (PrivateLink / Cloud). The reconnection fallback that re-expands contact points is meaningful only for the default topology monitor, whose
DefaultEndPoints cache their resolved address and never re-resolve. Proxy-based monitors —ClientRoutesTopologyMonitor(PrivateLink) andCloudTopologyMonitor— reach nodes through endpoints (ClientRoutesEndPoint/SniEndPoint) that already re-resolve on every connection attempt and maintain an authoritative node set. Appending raw contact points to their reconnection plan is both unnecessary and harmful: it can resurrect nodes the monitor has removed (e.g. after node replacement). The newTopologyMonitor.reresolvesNodeAddresses()gate skips the append for these monitors, preserving existing PrivateLink/Cloud behavior with no regression.