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

amqp_client resolves DNS twice #2748

Closed
haljin opened this issue Jan 26, 2021 · 2 comments · Fixed by #2763
Closed

amqp_client resolves DNS twice #2748

haljin opened this issue Jan 26, 2021 · 2 comments · Fixed by #2763

Comments

@haljin
Copy link
Contributor

haljin commented Jan 26, 2021

Hi,

I have noticed that in amqp_network_connection.erl there are the following functions:

inet_address_preference() ->
    case application:get_env(amqp_client, prefer_ipv6) of
        {ok, true}  -> [inet6, inet];
        {ok, false} -> [inet, inet6]
    end.

gethostaddr(Host) ->
    Lookups = [{Family, inet:getaddr(Host, Family)}
               || Family <- inet_address_preference()],
    [{IP, Family} || {Family, {ok, IP}} <- Lookups].

What is the purpose of always doing the DNS resolution on both IPv4 and IPv6 regardless of preference? I'd think that if you prefer IPv4 and that DNS resolves to an IPv4 IP, there isn't really a point of resolving IPv6?

The reason I am bringing this up is because DNS resolution can sometimes (due to various problems) take a very long time. E.g. on our corporate network resolving localhost in IPv6 sometimes takes upwards of 5 seconds, causing intermittent test failures on development machines as it happens to be a gen_server:call/2 timeout. Similarly in Kubernetes these DNS resolution can cause issues.

If you agree that there is no need to do this resolution, I'd be more than happy to make a PR.

@haljin haljin changed the title amqp_client resolved DNS twice amqp_client resolves DNS twice Jan 26, 2021
@michaelklishin
Copy link
Member

inet_address_preference/0 was likely introduced after the client already resolved thing in a specific way. You are welcome to make gethostaddr/1 respect the preference setting and submit a PR.

@lukebakken
Copy link
Collaborator

Resolved by #2763

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants