Skip to content
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

Use Default DNS Resolver when Creating a New HttpClient #22661

Merged

Conversation

alzimmermsft
Copy link
Member

Fixes #22618

Updates NettyAsyncHttpClientBuilder to use the default DNS resolver when an HttpClient instance is being created without an initial underlying Netty HTTP client. This resolves an issue where the first request made in an application has a much higher completion time (>10s) compared to subsequent requests (generally in ms).

@alzimmermsft alzimmermsft added Client This issue points to a problem in the data-plane of the library. Azure.Core azure-core HttpClient labels Jun 30, 2021
@alzimmermsft alzimmermsft self-assigned this Jun 30, 2021
@@ -92,9 +93,9 @@ public NettyAsyncHttpClientBuilder(HttpClient nettyHttpClient) {
if (this.baseHttpClient != null) {
nettyHttpClient = baseHttpClient;
} else if (this.connectionProvider != null) {
nettyHttpClient = HttpClient.create(this.connectionProvider);
nettyHttpClient = HttpClient.create(this.connectionProvider).resolver(DefaultAddressResolverGroup.INSTANCE);
Copy link
Member

Choose a reason for hiding this comment

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

Is this not supported in OkHttp?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is possible to configure the DNS resolver in OkHttp as well but it doesn't run into the same issue being seen with Reactor Netty.

Copy link
Member

Choose a reason for hiding this comment

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

Since this is applicable to all HTTP clients, is this something we should put on HttpClientOptions so that the user doesn't have to go build their own HTTP client when they have to configure this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Issue is that the types being used aren't in the base Java library so there will need to be a wrapper interface

@alzimmermsft alzimmermsft merged commit 5c6a5ff into Azure:main Jul 19, 2021
@alzimmermsft alzimmermsft deleted the AzCore_UseDefaultAddressResolver branch July 19, 2021 23:50
@jbkervyn
Copy link

@alzimmermsft Hello Alan, this is causing my application to be unable to connect to the azure keyvault (who's behind a corporate proxy). Before your fix, the http client resolver was null, and later in the same class initialized with an NoopAddressResolver. Because of this fix, this is not happening anymore.

In other words, this code that was executed before the fix is not anymore :)

if (resolver == null) {
                    nettyHttpClient.resolver(NoopAddressResolverGroup.INSTANCE);
                }

Is this a regression, or is there something else I should be doing? Thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core azure-core Client This issue points to a problem in the data-plane of the library. HttpClient
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Azure Core slow response with newer versions.
4 participants