Enable DNS Resolving via TCP in case of Timeout

[cgm-aw]

Hi everyone,

I noticed that Vertx enables Netty DNS Resolving via TCP (see https://github.com/eclipse-vertx/vert.x/blob/4.5.17/src/main/java/io/vertx/core/impl/resolver/DnsResolverProvider.java#L108):
builder.socketChannelFactory(() -> (SocketChannel) vertx.transport().channelFactory(false).newChannel());

However, this sets "retryOnTimeout" to "false". It would be nice if Vertx would enable TCP resolving on timeouts - maybe just with some kind of environment variable for toggling, like this:

builder.socketChannelFactory(() -> (SocketChannel) vertx.transport().channelFactory(false).newChannel(), Boolean.getBoolean("vertx.dns.retryOnTimeout"));

I'm happy to provide a PR if you approve of this.

Best regards

Contribution

Me

[cgm-aw]

By the way, I think #5114 requests the same thing.

[vietj]

@cgm-aw this should be configured using vertx options or address resolver options, contribution is welcome

[cgm-aw]

Done: #5692
I just noticed this is flagged for 5.x, I based the fix on 4.x. Not sure what your policy on backports is - should I base the fix on 5.x?

[vietj]

we can backport very small features

[vietj]

@cgm-aw can you port this contribution to master ?

[cgm-aw]

Sure, here is the PR for master:
#6116

[codingkiddo]

Hi @cgm-aw / maintainers, I’d like to work on this if it’s still available.

I checked current master and the equivalent call now appears in:

vertx-core/src/main/java/io/vertx/core/dns/impl/DnsAddressResolverProvider.java

My proposed approach:

• add a Vert.x system property, for example vertx.dns.retryOnTimeout
• wire it into Netty’s DnsNameResolverBuilder#socketChannelFactory(ChannelFactory, boolean)
• keep the default behavior unchanged by using false
• add focused test coverage where feasible, making sure the system property is restored after the test

Does this API shape sound acceptable, or would you prefer this as an AddressResolverOptions property instead of a system property?

[vietj]

system properties must be avoided

[cgm-aw]

Hi @cgm-aw / maintainers, I’d like to work on this if it’s still available.

I checked current master and the equivalent call now appears in:

vertx-core/src/main/java/io/vertx/core/dns/impl/DnsAddressResolverProvider.java

My proposed approach:

• add a Vert.x system property, for example vertx.dns.retryOnTimeout
• wire it into Netty’s DnsNameResolverBuilder#socketChannelFactory(ChannelFactory, boolean)
• keep the default behavior unchanged by using false
• add focused test coverage where feasible, making sure the system property is restored after the test

Does this API shape sound acceptable, or would you prefer this as an AddressResolverOptions property instead of a system property?

My PR 6116 does exactly that already using the existing Vertx configuration possibilities, @vietj please check if the tests are sufficient for you.

[vietj]

@cgm-aw it is not clear how your contribution is tested, normally the vertx mock dns resolver should implement TCP to implement the test