fix: expand contact point hostnames to all DNS IPs at connection time (DRIVER-201) — Part 1/2

[nikagra]

Problem

When RESOLVE_CONTACT_POINTS=false (the default), a contact point hostname was stored as a single unresolved InetSocketAddress. At connection time the load-balancing query plan contained exactly one Node per hostname, so only the first IP returned by DNS was ever tried. If that IP was non-responsive the driver raised AllNodesFailedException with 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.

Note: This is part 1 of 2, implementing the initial contact endpoints fix per @dkropachev's architectural direction. Part 2 will expand the EndPoint API with resolveAll() to address the broader connection path.

Changes

DefaultDriverOption / TypedDriverOption

• RESOLVE_CONTACT_POINTS is now @Deprecated. Contact points are always kept as unresolved hostnames; the option is a no-op.

SessionBuilder

• Removed the RESOLVE_CONTACT_POINTS read; ContactPoints.merge() is now always called with resolve=false, deferring all DNS expansion to connection time.

MetadataManager

• New getResolvedContactPoints() method: for each contact point backed by an unresolved hostname, resolves the hostname to obtain all its DNS IPs and creates a synthetic DefaultNode for each. Already-resolved addresses, and endpoints that aren't a DefaultEndPoint (e.g. SNI or client-routes endpoints), are returned as-is.
• Bounded, off-event-loop resolution. This method runs on the shared admin event loop (called from ControlConnection init/reconnect, which assert adminExecutor.inEventLoop()), where nothing should ever block. DNS resolution (InetAddress.getAllByName()) is inherently a blocking call, so it's offloaded to a small dedicated daemon-thread executor (contactPointResolverExecutor) and bounded by a 3s timeout (CONTACT_POINT_RESOLUTION_TIMEOUT): a slow or unreachable resolver can now only stall the admin loop for that long, instead of the resolver's own (effectively unbounded) timeout. On timeout, the hostname is skipped, same as an unresolvable one. This is a deliberate interim mitigation, not a full fix — the admin-loop thread still waits, just for a bounded amount of time — and is superseded by feat: multi-address DNS resolution for contact points and connections (DRIVER-201) #890's EndPoint.resolveAll() redesign, which will make resolution properly non-blocking.

LoadBalancingPolicyWrapper

• newQueryPlan() (in BEFORE_INIT/DURING_INIT states) and newControlReconnectionQueryPlan() now call getResolvedContactPoints() instead of getContactPoints(), so the query plan contains one entry per resolved IP. The driver naturally falls back to the next candidate if one is unreachable.
• newControlReconnectionQueryPlan() only appends the resolved-contact-point fallback once the LBP is RUNNING. Before that (BEFORE_INIT/DURING_INIT), newQueryPlan() already built the regular plan directly from getResolvedContactPoints(), so appending it again would duplicate every entry in the plan and re-trigger DNS resolution (and the bounded wait above) for no benefit.

TopologyMonitor (new capability method)

• Added TopologyMonitor.reresolvesNodeAddresses(), a default method returning false.
• Overridden to return true in ClientRoutesTopologyMonitor and CloudTopologyMonitor. These proxy-based monitors reach nodes through ClientRoutesEndPoint / 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.

InsightsClient

• getContactPoints() → getResolvedContactPoints(), so the startup insights payload reports the resolved contact-point IPs, consistent with the new resolution model.

OptionalLocalDcHelper (dead-code cleanup) ⚠️ reviewer FYI

• Removed the protected checkLocalDatacenterCompatibility(localDc, contactPoints) method and its call. It read contactPoint.getDatacenter() and warned when a contact point reported a datacenter different from the configured local DC.
• This check was broken on scylla-4.x, independent of this PR — it could only ever emit a false positive. It worked in the original DataStax model, where InitialNodeListRefresh matched discovered nodes to contact points by endpoint and mutated the same contact-point DefaultNode instance with its real datacenter. Commit 12e6acb90b ("Resolve control node identity via system.local before initial metadata refresh", already merged to scylla-4.x) switched refresh matching to hostId-only. Contact-point nodes carry no hostId, so they are never reused and their datacenter field stays null forever. Comparing that null against a configured local DC (!Objects.equals(localDc, null)) is always true, so the check would fire as a false positive for every contact point whenever local-datacenter was set on the default profile — never surfacing a real mismatch.
• No functional loss — removing it eliminates a spurious warning. The useful diagnostic — "configured local DC matches no node's datacenter" — is already provided by the pre-existing node-based check in 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.
• Reviewer FYI: the removed method was protected in an internal package (no public-API/binary-compat impact); the in-tree subclasses InferringLocalDcHelper / MandatoryLocalDcHelper do not override or call it. Removing it is cleanup consistent with deferred resolution (contact points are now unresolved hostnames with no DC); the only user-visible effect is that the spurious false-positive warning is no longer emitted.

Reconnection fallback default

• advanced.control-connection.reconnection.fallback-to-original-contact-points now defaults to true (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-enters getResolvedContactPoints() and re-expands each hostname to its current DNS IPs.
• The programmatic default in OptionsMap was flipped to true in lockstep with reference.conf (kept in sync; enforced by MapBasedDriverConfigLoaderTest).

Tests

• 6 new MetadataManagerTest cases for getResolvedContactPoints(): not-yet-set state, already-resolved passthrough, single-hostname DNS expansion, multi-endpoint expansion, unresolvable-hostname skip, and resolution-timeout skip (verifies the bounded executor actually bounds the wait instead of hanging for the full simulated delay).
• LoadBalancingPolicyWrapperTest: updated to stub getResolvedContactPoints() / getTopologyMonitor(), plus 4 new cases — contact points appended when the flag is enabled, not appended when disabled, not appended when the topology monitor re-resolves addresses (even with the flag on), and not double-resolved/duplicated before init.
• HeartbeatIT: pins CONTROL_CONNECTION_RECONNECT_CONTACT_POINTS=false for its sessions. These tests use SimulacronRule (→ DefaultTopologyMonitor) and exercise heartbeat behavior, not contact-point reconnection; without the pin, the newly-enabled fallback sends an extra OPTIONS during init and skews heartbeat counts.
• MockResolverIT.should_connect_when_first_dns_entry_is_non_responsive: 2-node CCM cluster on 127.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

Event-loop safety. getResolvedContactPoints() runs on the shared admin event loop (called from ControlConnection init/reconnect, which assert adminExecutor.inEventLoop()). DNS resolution is inherently blocking, so the actual InetAddress.getAllByName() call is offloaded to a dedicated single-thread executor and bounded by a 3s timeout rather than being left to block that thread for the resolver's own (effectively unbounded) timeout — a single dedicated thread is enough given the small, bounded contact-point set this is used for. This is a deliberate interim mitigation, not a full fix: the admin-loop thread still waits, just for a bounded amount of time. It's superseded by #890's EndPoint.resolveAll() redesign, which moves multi-address resolution into the EndPoint API / ChannelFactory for proper non-blocking resolution.

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() keeps nodeInfo.getEndPoint() verbatim). Since DefaultEndPoint.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 to true.

TLS / SNI. Each synthetic endpoint is built from the InetAddress returned 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) and CloudTopologyMonitor — 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 new TopologyMonitor.reresolvesNodeAddresses() gate skips the append for these monitors, preserving existing PrivateLink/Cloud behavior with no regression.

[Copilot]

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_POINTS in DefaultDriverOption / TypedDriverOption and stops reading it in SessionBuilder (always merges contact points with resolve=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.

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This 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 Diagram

sequenceDiagram
  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

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: expanding contact point hostnames to all DNS IPs at connection time.
Description check	✅ Passed	The description is directly related to the changeset and accurately outlines the problem, fix, and scope.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

core/src/test/java/com/datastax/oss/driver/internal/core/metadata/MetadataManagerTest.java (3)

493-551: ⚡ Quick win

Consider 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-InetSocketAddress endpoint 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 value

Test 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, if localhost resolves to multiple IPs (e.g., 127.0.0.1 and ::1 on 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 value

Test doesn't verify expansion to ALL IPs.

The test name claims should_expand_unresolved_hostname_to_all_ips, but the assertion only checks isNotEmpty() (line 523). This doesn't verify that InetAddress.getAllByName() actually returned multiple IPs or all available IPs for the hostname.

Consider either:

1. Using a hostname known to resolve to multiple IPs and asserting the exact count, or
2. Renaming the test to should_expand_unresolved_hostname_to_resolved_ips to 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

📥 Commits

Reviewing files that changed from the base of the PR and between 9149592 and 68e37e8.

📒 Files selected for processing (7)

• core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java
• core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java
• core/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.java
• 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
• core/src/test/java/com/datastax/oss/driver/internal/core/metadata/MetadataManagerTest.java

[coderabbitai]

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

📥 Commits

Reviewing files that changed from the base of the PR and between 36e7af3 and 9c9cba9.

📒 Files selected for processing (7)

• core/src/main/java/com/datastax/dse/driver/internal/core/insights/InsightsClient.java
• core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/helper/OptionalLocalDcHelper.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/MetadataManager.java
• core/src/test/java/com/datastax/dse/driver/internal/core/insights/InsightsClientTest.java
• core/src/test/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapperTest.java
• integration-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

[dkropachev]

Given the metadata behavior introduced here, I think advanced.control-connection.reconnection.fallback-to-original-contact-points should default to true.

getResolvedContactPoints() now expands an unresolved hostname contact point into IP-backed synthetic DefaultEndPoints. If one of those connects, the control node can be registered into metadata with that resolved IP endpoint. Since DefaultEndPoint.resolve() just returns the stored InetSocketAddress, later control reconnects may keep trying stale metadata IPs and never re-resolve the original contact point DNS.

Making the fallback default true gives reconnect a path back to the original unresolved contact points after the metadata/LBP plan is exhausted, preserving deferred-DNS behavior when DNS records change.

[dkropachev]

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 stores nodeInfo.getEndPoint() as-is, so a control node resolved via a synthetic IP candidate persists that resolved endpoint.
• If a follow-up touches DefaultTopologyMonitor.buildNodeEndPoint(), the system.local branch should say that returning localEndPoint intentionally 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]

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.

[nikagra]

@dkropachev thanks, addressed all three in 30a585f.

1. fallback-to-original-contact-points default → true (your comment)
Done, and dropped the "Experimental" note. This is precisely the DNS re-resolution path: newControlReconnectionQueryPlan() calls getResolvedContactPoints() fresh on every reconnect, so the original hostnames are re-resolved and new IPs picked up once the live-node plan is exhausted. Metadata nodes can't self-heal here — DefaultEndPoint.resolve() returns the stored InetSocketAddress verbatim and never re-queries DNS — so the fallback is the mechanism, not a band-aid. (Metadata-endpoint self-re-resolution belongs to Part 2 / resolveAll().)

2. Metadata persistence made explicit (your review)
Documented in the PR description and in Javadoc: getResolvedContactPoints() now states the returned synthetic nodes are IP-backed connection candidates that may be persisted; registerNode() states the metadata node stores nodeInfo.getEndPoint() as-is, so a control node reached via a synthetic IP candidate persists that resolved endpoint and is only re-resolved via the fallback.

3. TLS/SNI implication made explicit (your review)
Added an inline comment at the new InetSocketAddress(ip, port) site plus a Javadoc TLS note and a PR-description note: the endpoint is built from the resolved InetAddress (which retains the original hostname) on purpose, so TCP connects to the IP while the SSL engine keeps the hostname for peer host / SNI / hostname verification — and must not be simplified to a raw IP string.

[Copilot]

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 DefaultEndPoint and expanded by getResolvedContactPoints(), 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.

[dkropachev]

Current implementation bounds DNS blocking, but does not remove it. The admin event loop still waits on DNS through Future.get(...), which can stall reconnects, metadata refreshes, topology events, and close handling.

Please make DNS-expanded contact-point planning asynchronous end to end: resolve on contactPointResolverExecutor, return a CompletionStage, and resume control-connection connect logic on adminExecutor when resolution completes.

[dkropachev]

This still blocks the admin event loop. DNS runs on contactPointResolverExecutor, but the admin thread waits on resolutionFuture.get(...).

Please make contact-point DNS expansion fully async: return CompletionStage<List<Node>> from the resolver path and complete it from contactPointResolverExecutor. The admin thread should only resume via callback on adminExecutor; it should never wait synchronously for DNS.

[nikagra]

Good catch — this whole path is superseded in #890 (5e312d74), so the fix lands there rather than adding CompletionStage plumbing here only for #890 to remove it:

• getResolvedContactPoints(), contactPointResolverExecutor and the resolutionFuture.get(...) wait are deleted in feat: multi-address DNS resolution for contact points and connections (DRIVER-201) #890 — MetadataManager no longer resolves DNS at all, and LoadBalancingPolicyWrapper uses the in-memory getContactPoints().
• Multi-address expansion moves to EndPoint.resolveAll() at the connection layer. In feat: multi-address DNS resolution for contact points and connections (DRIVER-201) #890 that method now returns CompletionStage<SocketAddress[]> and takes an Executor: ChannelFactory owns a dedicated daemon resolver pool, offloads the blocking InetAddress.getAllByName() to it, and drives the connect from the resulting stage.
• ChannelFactory.connect() is exactly the call reached from the admin event loop (via ControlConnection reconnect); it no longer waits synchronously — the admin thread submits resolution and resumes via callback.

Net: the admin-loop blocking is eliminated in #890. In #889 standalone the resolve stays bounded by the 3s CONTACT_POINT_RESOLUTION_TIMEOUT. Happy to restructure the stack if you'd prefer the async resolution to be visible within #889 itself.

[dkropachev]

This method needs to become async because it now depends on DNS expansion. Please expose an async control-reconnection query-plan method and compose resolved contact points there.

Expected flow: ControlConnection asks for async reconnection plan -> DNS resolves off admin -> continuation runs on adminExecutor -> connect(nodes, ...).

This keeps the existing control-connection sequencing but removes admin-thread blocking.

[nikagra]

In #890 (5e312d74) this no longer needs to become async: newControlReconnectionQueryPlan() switches from getResolvedContactPoints() to the in-memory getContactPoints() (no DNS), and per-hostname expansion is deferred to EndPoint.resolveAll() at the connection layer — which #890 makes asynchronous (resolution offloaded to a dedicated executor, connect resumed via callback). So the control-reconnection plan stays synchronous and cheap, and no admin-thread DNS wait remains.

[dkropachev]

This synchronous query-plan path should not trigger DNS resolution. DNS-expanded contact points should be resolved through the async control-connection path, then passed into the plan once available.

Please avoid calling a blocking or async-waiting resolver from newQueryPlan().

[nikagra]

Agreed — and in #890 (5e312d74) it doesn't: this path switches to the in-memory getContactPoints(), so newQueryPlan() triggers no DNS. The per-hostname expansion is deferred to EndPoint.resolveAll() at connection time, which #890 performs asynchronously off the admin loop.

[dkropachev]

I see the split as: #890 owns the final EndPoint.resolveAll() / ChannelFactory connection-layer design, but #889 still needs to be safe if it lands independently.

Blockers for #889:

• LoadBalancingPolicyWrapper.newControlReconnectionQueryPlan() mutates regularQueryPlan with addAll(...). Built-in query plans are not mutable; QueryPlan.add() throws. Since this PR defaults the fallback on, normal post-init control reconnects can fail before trying any node. Please wrap/copy instead, e.g. CompositeQueryPlan(regularPlan, fallbackPlan), and test with a real query-plan implementation rather than a mutable LinkedList.

• The !topologyMonitor.reresolvesNodeAddresses() gate suppresses fallback even when the regular/live-node plan is empty. In that case there is nothing to try, so reconnect can’t recover. Please keep fallback available when regularQueryPlan.isEmpty(), even for monitors that normally re-resolve addresses, and add coverage for that case.

• MetadataManager.getResolvedContactPoints() resolves with InetAddress.getAllByName(), which bypasses custom Netty resolvers that previously saw unresolved contact points through Bootstrap.connect(). The proper multi-address resolver fix belongs in #890, where ChannelFactory has the initialized Bootstrap. For #889, please avoid enabling this JVM-DNS path by default, remove query-plan-time DNS expansion, or land it only together with #890 where the interim path is removed.

[nikagra]

Thanks @dkropachev — all three points addressed. Since #890 already owns the final EndPoint.resolveAll() / ChannelFactory design and removes this PR's interim resolution path, I've folded #889 into #890 and am closing this one. The fixes below are pushed to #890 (commit dbd37b3); #890's description is updated to reflect that it now delivers DRIVER-201 as a single change.

1. addAll(...) on an immutable query plan. Fixed. newControlReconnectionQueryPlan() no longer mutates the policy's plan — it returns new CompositeQueryPlan(regularQueryPlan, new SimpleQueryPlan(contactNodes.toArray())), the same primitive BasicLoadBalancingPolicy uses to concatenate plans. Root cause of the miss: the tests stubbed policy.newQueryPlan(...) with a mutable Guava LinkedList; they now use a real SimpleQueryPlan / QueryPlan.EMPTY, so the immutability contract is actually exercised. I confirmed the updated tests fail with UnsupportedOperationException against the old addAll code and pass after the fix.

2. Fallback suppressed when the live-node plan is empty. Fixed. The gate is now !topologyMonitor.reresolvesNodeAddresses() || regularQueryPlan.isEmpty(), so when the regular plan is empty the contact-point fallback is kept even for re-resolving monitors — otherwise there's nothing to try and reconnect can't recover. Added a test for reresolvesNodeAddresses()==true + empty regular plan.

3. InetAddress.getAllByName() bypassing custom Netty resolvers / interim path. Resolved by combining into #890: the interim MetadataManager.getResolvedContactPoints() that resolved on the admin event loop via getAllByName() is gone; resolution now happens at connection time through EndPoint.resolveAll(Executor), driven off the admin loop by ChannelFactory's resolver pool, with each candidate IP tried in turn — so the interim path never lands on its own.

On the Netty-resolver point specifically: the driver doesn't install a Netty AddressResolverGroup anywhere (nothing calls Bootstrap.resolver(...) in NettyOptions/ChannelFactory), and hostname resolution has always gone through InetAddress.getAllByName() — SniEndPoint does exactly that today, and DefaultEndPoint.resolveAll() documents it. Our MockResolverIT plugs a JVM-level InetAddress name-service interceptor (Burningwave), which getAllByName() honors, so those tests still pass. A pluggable Netty resolver would be a separate enhancement.