feat: multi-address DNS resolution for contact points and connections (DRIVER-201)

[nikagra]

Problem

DRIVER-201: when a contact point or a cluster node is given as a hostname that maps to multiple IPs (e.g. a DNS round-robin / dynamic-DNS entry), the driver only ever tried the first address — at initial contact, at connection time, and on control-connection reconnect. If that first IP was unreachable the driver raised AllNodesFailedException even though the hostname also resolved to healthy IPs.

This PR fixes DRIVER-201 end-to-end: every such hostname is now resolved to all its addresses and each is tried in turn, at both the contact-point and the general connection layer.

Note: This is a single consolidated PR for DRIVER-201. The work was originally split into #889 (Part 1 — expanding contact-point hostnames in the load-balancing query plan, an interim approach that resolved DNS on the admin event loop under a timeout) and #890 (Part 2 — the EndPoint API + ChannelFactory fix). They are now combined here and #889 is closed. This PR lands the EndPoint.resolveAll() / ChannelFactory design so that any connection attempt — pool connections, reconnections, cloud SNI, and the initial control connection — gets multi-address fallback, and removes the interim query-plan resolution now that it is redundant (the admin event loop no longer blocks on DNS). Because everything lands together, the interim JVM-DNS path never ships on its own.

Changes

EndPoint interface

• resolve() is now @Deprecated.
• New resolveAll() default method returns SocketAddress[]. The default implementation wraps resolve() in a single-element array, so existing third-party implementations keep working (no new abstract method → source/binary compatible).

DefaultEndPoint

• Overrides resolveAll(): for unresolved addresses calls InetAddress.getAllByName() and returns one InetSocketAddress per IP (built from the resolved InetAddress so the original hostname is retained for TLS peer host / SNI / hostname verification). Falls back to a single-element array (the unresolved address) if DNS fails, so the connect attempt surfaces a descriptive error rather than an empty array.

SniEndPoint

• Overrides resolveAll(): re-resolves the proxy hostname on each call, sorts all A-records by IP, and returns all records so a single connection attempt can fall back across every proxy IP. The candidate order is rotated each call using the same round-robin OFFSET counter as resolve(), so healthy connections stay spread across proxy IPs instead of always starting at index 0. (dkropachev's CHANGES_REQUESTED fix.)

ClientRoutesEndPoint

• Overrides resolveAll(): wraps the single topology-monitor-resolved address in a one-element array (single-address by design).

ChannelFactory

• connect() now calls endPoint.resolveAll() instead of endPoint.resolve(), and guards against a null/empty array from a custom EndPoint by failing resultFuture (instead of NPE/AIOOBE).
• New tryNextCandidate() iterates the returned array; on per-address failure it logs and tries the next; only fails the overall resultFuture once all candidates are exhausted.
• New connectToAddress() scopes protocol-version negotiation (downgrade retries) to a single address.
• Trade-off: a single connect() now serially attempts every candidate, so the worst-case time to declare a node unreachable is N × connect-timeout. This is an intentional trade-off (failing on the first unreachable IP would prevent fallback) and is documented on EndPoint.resolveAll().

Remove the interim query-plan resolution

Now that ChannelFactory.connect() → resolveAll() handles multi-address fallback at connection time — for both control-connection init and pool connections — the earlier interim query-plan-time DNS expansion (the contact-point hostname expansion from the initial approach) is redundant and is removed:

• MetadataManager: drops getResolvedContactPoints() and its dedicated resolver executor, 3s timeout, and helpers. That method resolved contact-point hostnames on the admin event loop (offloaded to a bounded executor because InetAddress.getAllByName() blocks and the admin loop must never block). The event loop no longer blocks on DNS at all — the query plan now holds one unresolved node per contact point, and resolveAll() expands each to all its IPs at connect time.
• LoadBalancingPolicyWrapper / InsightsClient: revert to getContactPoints(). The RUNNING-state reconnection fallback and the TopologyMonitor.reresolvesNodeAddresses() gate are preserved.
• Docs (reference.conf, SessionBuilder, DefaultEndPoint) updated to say resolution happens at connection time via EndPoint.resolveAll().

Control-connection reconnection query plan (folded from #889 review)

LoadBalancingPolicyWrapper.newControlReconnectionQueryPlan() now composes the contact-point fallback via CompositeQueryPlan(regularPlan, new SimpleQueryPlan(contactNodes)) instead of mutating the policy's plan. Built-in QueryPlans reject add()/addAll() (poll() is their only mutator), so the previous addAll(...) threw UnsupportedOperationException on every post-init control reconnect once the fallback defaulted on. The fallback is also kept when the live-node plan is empty, even for re-resolving topology monitors, so reconnection can still recover when there is nothing else to try. The wrapper tests now stub the policy plan with a real SimpleQueryPlan / QueryPlan.EMPTY (the earlier mutable LinkedList stub masked the crash), plus a new empty-plan + re-resolving-monitor case. (dkropachev's #889 CHANGES_REQUESTED fix.)

Internal callers

Callers that legitimately need a single canonical address (InsightsClient, DseGssApiAuthProviderBase, DefaultTopologyMonitor, the SNI / Default SSL engine factories) keep calling the deprecated resolve() under a scoped @SuppressWarnings("deprecation").

Tests

• DefaultEndPointTest: already-resolved passthrough, unresolved hostname expansion, unresolvable hostname fallback.
• SniEndPointTest: resolveAll() happy path, unresolvable host exception, resolve() sanity check, and a rotation/completeness case asserting the full candidate set is returned every call and the starting candidate rotates when multiple IPs exist.
• ChannelFactoryResolveAllGuardTest: null array, empty array, and resolveAll() throwing all fail the connect future.
• LoadBalancingPolicyWrapperTest: real QueryPlan stubs; append-ordering, empty-plan, and re-resolving-monitor cases for the control-reconnection plan.
• All 13 existing ChannelFactory tests pass unchanged (LocalEndPoint uses the default single-element resolveAll() via the interface default).
• Removed the seven MetadataManagerTest contact-point resolution/timeout unit tests (that behavior now lives in DefaultEndPointTest.resolveAll and the ChannelFactory tests); adapted the LoadBalancingPolicyWrapper / InsightsClient tests to getContactPoints().

[Copilot]

Pull request overview

Part 2/2 of DRIVER-201: extends the EndPoint API and ChannelFactory so that a hostname mapping to multiple IPs is tried address-by-address at the connection layer, instead of only the first IP. The EndPoint.resolve() method is deprecated in favor of a new resolveAll() default method; DefaultEndPoint, SniEndPoint, and ClientRoutesEndPoint override it; ChannelFactory.connect() now iterates over candidates and only fails when all are exhausted, while keeping protocol-version downgrade scoped to a single address.

Changes:

• Add EndPoint.resolveAll() (default impl delegating to deprecated resolve()); override in DefaultEndPoint, SniEndPoint, ClientRoutesEndPoint.
• Rework ChannelFactory.connect() into tryNextCandidate / connectToAddress so per-address failures fall back to the next IP while protocol-version downgrades stay scoped to one address.
• Add unit tests for DefaultEndPoint.resolveAll() and a new SniEndPointTest.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
core/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.java	Deprecates resolve(); adds default resolveAll() method.
core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.java	Overrides resolveAll() using InetAddress.getAllByName with single-address fallback.
core/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.java	Overrides resolveAll() returning one address per sorted A-record.
core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPoint.java	Overrides resolveAll() to wrap the single topology-monitor address.
core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.java	Adds candidate-iteration and per-address protocol-negotiation methods.
core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.java	New tests for resolveAll() (resolved, unresolved expansion, unresolvable fallback).
core/src/test/java/com/datastax/oss/driver/internal/core/metadata/SniEndPointTest.java	New test class covering SNI resolveAll() happy path, unresolvable host, and resolve() sanity check.

Comments suppressed due to low confidence (1)

core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.java:303

• When connectToAddress fails with UnsupportedProtocolVersionException.forNegotiation (i.e. all protocol downgrades exhausted), tryNextCandidate will treat this like any other per-address failure and try the next IP, even though the protocol-negotiation failure is a server-wide condition that will recur on every other IP of the same node. This also reuses the shared attemptedVersions CopyOnWriteArrayList across candidates, so on each subsequent address the downgrade loop re-attempts the same protocol versions and adds duplicate entries, and the final exception ultimately reported will list each version multiple times. Consider distinguishing non-address-specific failures (UnsupportedProtocolVersionException, authentication errors, etc.) and short-circuiting the candidate loop in those cases.

    perAddressFuture.whenComplete(
        (channel, error) -> {
          if (error == null) {
            resultFuture.complete(channel);
          } else if (index + 1 < candidates.length) {
            LOG.debug(
                "[{}] Failed to connect to {} ({}), trying next address",
                logPrefix,
                candidate,
                error.getMessage());
            tryNextCandidate(
                endPoint,
                shardingInfo,
                shardId,
                options,
                nodeMetricUpdater,
                currentVersion,
                isNegotiating,
                attemptedVersions,
                resultFuture,
                candidates,
                index + 1);
          } else {
            // Note: might be completed already if the failure happened in initializer()
            resultFuture.completeExceptionally(error);
          }
        });

---

💡 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 adds EndPoint.resolveAll() and deprecates single-address resolve(). Default, SNI, and client-route endpoints now provide candidate addresses. ChannelFactory tries resolved candidates sequentially, including protocol downgrade handling. Contact points are expanded through MetadataManager and used in query planning and control-connection reconnection. Reconnection defaults and topology-monitor behavior are updated, while local-datacenter discovery no longer checks contact-point compatibility. Tests cover endpoint resolution, connection guards, metadata expansion, query plans, and integration behavior.

Sequence Diagram(s)

sequenceDiagram
  participant ChannelFactory
  participant EndPoint
  participant tryNextCandidate
  participant connectToAddress
  participant resultFuture

  ChannelFactory->>EndPoint: resolveAll()
  EndPoint-->>ChannelFactory: SocketAddress[] candidates
  ChannelFactory->>tryNextCandidate: attempt candidate at index 0
  tryNextCandidate->>connectToAddress: connect using perAddressFuture
  alt connection succeeds
    connectToAddress-->>tryNextCandidate: DriverChannel
    tryNextCandidate->>resultFuture: complete successfully
  else connection or negotiation fails
    connectToAddress-->>tryNextCandidate: complete perAddressFuture exceptionally
    tryNextCandidate->>tryNextCandidate: attempt next candidate
  end
  tryNextCandidate->>resultFuture: fail after all candidates

Loading

Suggested reviewers: copilot, dkropachev

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 26.47% 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: multi-address DNS resolution for contact points and connections.
Description check	✅ Passed	The description is directly about the same DRIVER-201 DNS-resolution and connection-fallback changes in this PR.

---

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

🤖 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/channel/ChannelFactory.java`:
- Around line 222-242: The code calls endPoint.resolveAll() and passes the
resulting candidates array into tryNextCandidate() which immediately indexes
candidates[0]; guard against null or empty results by validating the output of
endPoint.resolveAll()—if it returns null or candidates.length == 0, complete
resultFuture exceptionally (or create a specific error) and return; otherwise
call tryNextCandidate(...) with the non-empty candidates. Update the block
around resolveAll(), candidates, and the call to tryNextCandidate() to perform
this check and fail fast via resultFuture.completeExceptionally when
appropriate.

🪄 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: 7ad3d5b5-6473-4c88-8777-93861f5de639

📥 Commits

Reviewing files that changed from the base of the PR and between c830c20 and 860a34d.

📒 Files selected for processing (12)

• core/src/main/java/com/datastax/dse/driver/api/core/auth/DseGssApiAuthProviderBase.java
• core/src/main/java/com/datastax/dse/driver/internal/core/insights/InsightsClient.java
• core/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTopologyMonitor.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/SniSslEngineFactory.java
• core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.java
• core/src/test/java/com/datastax/oss/driver/internal/core/metadata/SniEndPointTest.java

[coderabbitai]

🧹 Nitpick comments (1)

core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryResolveAllGuardTest.java (1)

37-37: ⚡ Quick win

Consider adding test coverage for resolveAll() throwing an exception.

The ChannelFactory.connect() implementation includes a catch block for exceptions thrown by resolveAll() (see context snippet 1, line 232). Adding a third test case where the mocked EndPoint.resolveAll() throws an exception (e.g., UnknownHostException) would ensure all three defensive paths are tested:

1. ✓ Returns null (covered)
2. ✓ Returns empty array (covered)
3. ✗ Throws exception (not covered)

📋 Suggested test case

`@Test`
public void should_fail_future_when_resolve_all_throws_exception() {
  // Given
  when(defaultProfile.isDefined(DefaultDriverOption.PROTOCOL_VERSION)).thenReturn(false);
  when(protocolVersionRegistry.highestNonBeta()).thenReturn(DefaultProtocolVersion.V4);
  ChannelFactory factory = newChannelFactory();

  EndPoint badEndPoint = mock(EndPoint.class);
  RuntimeException testException = new RuntimeException("DNS lookup failed");
  when(badEndPoint.resolveAll()).thenThrow(testException);

  // When
  CompletionStage<DriverChannel> channelFuture =
      factory.connect(
          badEndPoint, null, null, DriverChannelOptions.DEFAULT, NoopNodeMetricUpdater.INSTANCE);

  // Then – future must complete exceptionally with the thrown exception
  assertThatStage(channelFuture)
      .isFailed(e -> assertThat(e).isSameAs(testException));
}

🤖 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/channel/ChannelFactoryResolveAllGuardTest.java`
at line 37, Add a third test in ChannelFactoryResolveAllGuardTest that verifies
ChannelFactory.connect() propagates exceptions thrown by EndPoint.resolveAll():
mock an EndPoint (e.g., badEndPoint) to throw a RuntimeException (or
UnknownHostException) from resolveAll(), create the factory via
newChannelFactory(), call factory.connect(badEndPoint, ...) with
DriverChannelOptions.DEFAULT and NoopNodeMetricUpdater.INSTANCE, and assert the
returned CompletionStage<DriverChannel> completes exceptionally with the same
exception; this mirrors the existing tests for null/empty resolveAll() and
targets the catch path in ChannelFactory.connect().

🤖 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.

Nitpick comments:
In
`@core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryResolveAllGuardTest.java`:
- Line 37: Add a third test in ChannelFactoryResolveAllGuardTest that verifies
ChannelFactory.connect() propagates exceptions thrown by EndPoint.resolveAll():
mock an EndPoint (e.g., badEndPoint) to throw a RuntimeException (or
UnknownHostException) from resolveAll(), create the factory via
newChannelFactory(), call factory.connect(badEndPoint, ...) with
DriverChannelOptions.DEFAULT and NoopNodeMetricUpdater.INSTANCE, and assert the
returned CompletionStage<DriverChannel> completes exceptionally with the same
exception; this mirrors the existing tests for null/empty resolveAll() and
targets the catch path in ChannelFactory.connect().

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b702fd48-9ba7-4994-8bb9-351438fb02a8

📥 Commits

Reviewing files that changed from the base of the PR and between 860a34d and a6d0e48.

📒 Files selected for processing (13)

• core/src/main/java/com/datastax/dse/driver/api/core/auth/DseGssApiAuthProviderBase.java
• core/src/main/java/com/datastax/dse/driver/internal/core/insights/InsightsClient.java
• core/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTopologyMonitor.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/SniSslEngineFactory.java
• core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryResolveAllGuardTest.java
• core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.java
• core/src/test/java/com/datastax/oss/driver/internal/core/metadata/SniEndPointTest.java

✅ Files skipped from review due to trivial changes (5)

• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/SniSslEngineFactory.java
• core/src/main/java/com/datastax/dse/driver/api/core/auth/DseGssApiAuthProviderBase.java
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTopologyMonitor.java
• core/src/main/java/com/datastax/dse/driver/internal/core/insights/InsightsClient.java

🚧 Files skipped from review as they are similar to previous changes (7)

• core/src/test/java/com/datastax/oss/driver/internal/core/metadata/SniEndPointTest.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.java
• core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.java
• core/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.java

[nikagra]

🤖: Valid nitpick. Added a third test should_fail_future_when_resolve_all_throws_exception() to ChannelFactoryResolveAllGuardTest that mocks resolveAll() to throw a RuntimeException and asserts the future completes exceptionally with the same exception instance, covering the catch block in ChannelFactory.connect(). All three defensive paths are now tested: null return, empty array, and thrown exception.

[dkropachev]

When the SNI proxy hostname has multiple A records, ChannelFactory now calls resolveAll() for every connection instead of resolve(). This always returns the sorted records starting at index 0, so all healthy connections go to the first proxy IP, and if that IP is down every connection pays the timeout before trying the next one. Please rotate the returned candidate order with the existing OFFSET logic so resolveAll() preserves the previous round-robin behavior.

[nikagra]

🤖: Valid, fixed. resolveAll() now rotates the returned candidate order using the same OFFSET round-robin counter as resolve(): after sorting the A-records by IP, it picks a rotating start index (OFFSET.getAndUpdate(...) % length) and fills the result array as addr[(start + i) % length]. So healthy connections are still spread across proxy IPs (previous round-robin behavior preserved), while the full set of records is always returned so a single connection attempt can still fall back across every IP if one is down. Single-record hostnames keep start index 0. Added resolve_all_returns_complete_and_rotated_candidate_order() to SniEndPointTest asserting both invariants (complete set every call + starting candidate rotates when multiple IPs exist).

[nikagra]

resolveAll() returns the complete A-record set on every call and rotates the starting candidate. The rotation added in the previous push was undermined by the shared OFFSET counter (see the Copilot thread on the same file): SSL engine setup calls resolve() once per TLS connection, advancing the same counter, which pinned the start index to 0 whenever the proxy resolved to an even number of A-records. Fixed in 46d1ae6 by giving resolveAll() a dedicated RESOLVE_ALL_OFFSET, with an interleaved-resolve() unit test to guard against regressions.

[nikagra]

Rebased this PR (Part 2/2) on top of #889 (30a585f) so it now stacks cleanly on Part 1 and refreshes onto current scylla-4.x. Base stays scylla-4.x; the incremental diff will be clean once #889 merges. New head: 1c8dfa2.

Also addressed the outstanding review feedback:

• SNI round-robin (@dkropachev): SniEndPoint.resolveAll() now rotates the returned candidate order using the same OFFSET counter as resolve() — healthy connections are spread across proxy IPs while the full record set is still returned for in-connection fallback. Added a rotation/completeness test.

Previously-addressed items (Copilot / CodeRabbit) remain in place after the rebase: N×timeout Javadoc on resolveAll(), calling-thread DNS note on DefaultEndPoint, @SuppressWarnings("deprecation") on the 5 internal single-address callers, and the ChannelFactory null/empty-array guard with ChannelFactoryResolveAllGuardTest.

Verified locally on JDK 11: SniEndPointTest, DefaultEndPointTest, ChannelFactoryResolveAllGuardTest, and the full ChannelFactory*Test suite all pass.

[Copilot]

Pull request overview

Copilot reviewed 29 out of 29 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/internal/core/metadata/DefaultEndPoint.java:62

• NETTY_ADMIN_SIZE only configures the number of admin event-loop threads (DefaultDriverOption.java:807-811); it does not configure an AddressResolverGroup. This link gives users an incorrect way to identify or change the resolver. Refer to a custom NettyOptions bootstrap hook instead, or omit the configuration link.

   * <p><b>Note on resolver:</b> DNS lookup is performed via {@link
   * InetAddress#getAllByName(String)} on the calling thread, bypassing any custom Netty {@code
   * AddressResolverGroup} configured via {@link
   * com.datastax.oss.driver.api.core.config.DefaultDriverOption#NETTY_ADMIN_SIZE}. This is

[coderabbitai]

Actionable comments posted: 4

🤖 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/config/TypedDriverOption.java`:
- Line 603: Update the public Javadoc for the reconnection-plan option in
TypedDriverOption to state that it appends DNS-expanded candidates returned by
getResolvedContactPoints(), rather than raw original contact points, and that
monitors which re-resolve addresses skip this behavior; retain the documented
default of true.

In
`@core/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.java`:
- Around line 147-153: Prevent blocking DNS resolution from query-plan creation
by moving MetadataManager.getResolvedContactPoints() off the caller thread or
introducing bounded caching before using its results. Apply the fix to the
BEFORE_INIT/DURING_INIT path in
core/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.java:147-153
and the control-reconnect path in
core/src/main/java/com/datastax/oss/driver/internal/core/metadata/LoadBalancingPolicyWrapper.java:164-184;
update core/src/main/resources/reference.conf:2321-2334 if needed so
fallback-to-original-contact-points is not enabled without bounded, non-blocking
resolution.

In `@core/src/main/resources/reference.conf`:
- Around line 2321-2334: The default for fallback-to-original-contact-points
must not enable the blocking DNS fallback path; change this configuration
default back to false while preserving the existing setting name and
documentation.

In
`@core/src/test/java/com/datastax/oss/driver/internal/core/metadata/MetadataManagerTest.java`:
- Around line 512-529: The test should enforce expansion to the complete DNS
result set, not merely verify that one resolved node exists. Update
should_expand_unresolved_hostname_to_all_ips to obtain
InetAddress.getAllByName("localhost"), compare the returned node count and
endpoint addresses against all expected addresses on port 9042, and retain the
resolved-address assertions.

🪄 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: 648940a1-36ee-47f0-8f02-aff008723307

📥 Commits

Reviewing files that changed from the base of the PR and between 4448119 and 1c8dfa2.

📒 Files selected for processing (29)

• core/src/main/java/com/datastax/dse/driver/api/core/auth/DseGssApiAuthProviderBase.java
• core/src/main/java/com/datastax/dse/driver/internal/core/insights/InsightsClient.java
• core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java
• core/src/main/java/com/datastax/oss/driver/api/core/config/OptionsMap.java
• core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java
• core/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.java
• core/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.java
• core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.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/ClientRoutesEndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesTopologyMonitor.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/CloudTopologyMonitor.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTopologyMonitor.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/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/TopologyMonitor.java
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/DefaultSslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/SniSslEngineFactory.java
• core/src/main/resources/reference.conf
• core/src/test/java/com/datastax/dse/driver/internal/core/insights/InsightsClientTest.java
• core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryResolveAllGuardTest.java
• core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.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
• core/src/test/java/com/datastax/oss/driver/internal/core/metadata/SniEndPointTest.java
• integration-tests/src/test/java/com/datastax/oss/driver/core/heartbeat/HeartbeatIT.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 (11)

• core/src/test/java/com/datastax/oss/driver/internal/core/channel/ChannelFactoryResolveAllGuardTest.java
• core/src/main/java/com/datastax/dse/driver/api/core/auth/DseGssApiAuthProviderBase.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultTopologyMonitor.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/ClientRoutesEndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/ssl/SniSslEngineFactory.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/SniEndPoint.java
• core/src/main/java/com/datastax/oss/driver/api/core/metadata/EndPoint.java
• core/src/main/java/com/datastax/oss/driver/internal/core/channel/ChannelFactory.java
• core/src/main/java/com/datastax/dse/driver/internal/core/insights/InsightsClient.java
• core/src/main/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPoint.java
• core/src/test/java/com/datastax/oss/driver/internal/core/metadata/DefaultEndPointTest.java

[nikagra]

Pushed 5e312d74: EndPoint.resolveAll() is now asynchronous, addressing @dkropachev's point that contact-point DNS expansion still blocked the control-connection admin event loop (the three threads on #889).

• EndPoint.resolveAll() now returns CompletionStage<SocketAddress[]> and takes an Executor.
• DefaultEndPoint / SniEndPoint offload the blocking getAllByName() to that executor; ClientRoutesEndPoint (in-memory per-host-id lookup) and already-resolved addresses return a completed stage with no thread hop.
• ChannelFactory owns a dedicated cached daemon resolver pool (<session>-connection-resolver-N), passes it into resolveAll(), and composes the connection attempt off the returned stage (unwrapping CompletionException so the original cause surfaces). DefaultSession.onChildrenClosed() shuts the pool down.
• Since ChannelFactory.connect() is invoked from the admin event loop (control-connection reconnect), the admin thread now submits resolution and resumes via callback instead of blocking on DNS.

90 core unit tests pass, including new coverage that resolution runs on the dedicated resolver thread and that connect() returns without waiting on DNS.

Note: this does not change the SNI resolveAll() rotation semantics from your earlier thread — the blocking lookup is simply moved onto the resolver executor.