Skip to content

Deprecate DnsResolver in favor of AddressResolverGroup(#1572) #3291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 29, 2025

Conversation

young0264
Copy link
Contributor

@young0264 young0264 commented May 12, 2025

Checklist

  • You have read the contribution guidelines.
  • You have created a feature request first to discuss your contribution intent. Please reference the feature request ticket number in the pull request.
  • You applied code formatting rules using the mvn formatter:format target. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.

Summary

This PR deprecates Lettuce's internal DNS resolver and removes the conditional logic that checks for the presence of Netty's DNS resolver on the classpath.

Closes #1527

Background

Until Lettuce 6.1, 'netty-resolvers-dns' was treated as an optional dependency and used conditionally if present on the classpath. However [issue #1527] and previous discussions(e.g., #1517) suggest that 'netty-resolver-dns' should become a required dependency starting from version 6.2.

When reviewing the latest codebase (currently 6.6), I found that:

  • The code still uses 'LettuceClassUtils.isPresent(...)' to detect and optionally apply Netty DNS support
  • Lettuce's custom 'DnsResolver' (based on blocking 'InetAddress') is still present
    Based on this history, I've made the following changes:

Changes

  • Removed 'true' from 'netty-resolver-dns' in 'pom.xml'.
  • Removed conditional logic in 'AddressResolverGroupProvider'
  • Marked 'DnsResolver' (based on 'InetAddress') as '@deprecated'
  • Documented the rationale and changes in an internal Google Doc

Let me know if additional coverage or documentation is needed.

Thanks!!

* Mark DnsResolver and related API as @deprecated

* DefaultClientResources now unconditionally uses DnsAddressResolverGroup

* Update test coverage to reflect new default behavior
@young0264 young0264 marked this pull request as draft May 14, 2025 00:23
@young0264 young0264 marked this pull request as ready for review May 14, 2025 00:24
@tishun tishun added this to the 6.7.0.RELEASE milestone May 28, 2025
@tishun tishun added the type: task A general task label May 28, 2025
Copy link
Collaborator

@tishun tishun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

@tishun tishun merged commit 0d48e88 into redis:main May 29, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate lettuce DNS support classes
2 participants