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.

Sign up for GitHub

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

Resolve hostnames using netty's non-blocking DNS resolver #1517

Closed
wants to merge 3 commits into from
Closed

Resolve hostnames using netty's non-blocking DNS resolver #1517

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bb645d2
Resolve hostnames using netty's non-blocking DNS resolver #1498
yueki1993 Nov 6, 2020
ec85577
make netty-resolver-dns optional
yueki1993 Nov 21, 2020
13cd028
fix build break
yueki1993 Nov 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@
<artifactId>netty-handler</artifactId>
</dependency>

<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-resolver-dns</artifactId>
</dependency>

<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-transport</artifactId>
Expand Down
15 changes: 15 additions & 0 deletions src/main/java/io/lettuce/core/AbstractRedisClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@
import io.netty.channel.group.ChannelGroup;
import io.netty.channel.group.DefaultChannelGroup;
import io.netty.channel.nio.NioEventLoopGroup;
import io.netty.channel.socket.SocketChannel;
import io.netty.resolver.dns.DnsAddressResolverGroup;
import io.netty.resolver.dns.DnsNameResolverBuilder;
import io.netty.util.concurrent.EventExecutorGroup;
import io.netty.util.concurrent.Future;
import io.netty.util.internal.logging.InternalLogger;
Expand All @@ -73,6 +76,7 @@
* @author Mark Paluch
* @author Jongyeol Choi
* @author Poorva Gokhale
* @author Yohei Ueki
* @since 3.0
* @see ClientResources
*/
Expand Down Expand Up @@ -297,6 +301,17 @@ protected void channelType(ConnectionBuilder connectionBuilder, ConnectionPoint
}
}

protected void resolver(ConnectionBuilder connectionBuilder, ConnectionPoint connectionPoint) {

LettuceAssert.notNull(connectionPoint, "ConnectionPoint must not be null");

if (connectionPoint.getSocket() == null) {
connectionBuilder.bootstrap().resolver(
new DnsAddressResolverGroup(new DnsNameResolverBuilder().channelType(Transports.datagramChannelClass())
.socketChannelType(Transports.socketChannelClass().asSubclass(SocketChannel.class))));
Copy link
Contributor Author

@yueki1993 yueki1993 Nov 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

[Supplemental Comments]
The key point of this PR. cf: https://netty.io/news/2016/05/26/4-1-0-Final.html
If we want to use non-blocking dns resolver, we need to pass DnsAddressResolverGroup to Bootstrap.

Also note that netty encourages users to support TCP fallback in Netty 4.1.37 release note:
https://netty.io/news/2019/06/28/4-1-37-Final.html

I followed Netty 4.1.37 release note, i.e., use DnsNameResolverBuilder with DatagramChannel (UDP) and SocketChannel (TCP).

Since Transports.socketChannelClass() returns Class<? extends Channel> and DnsNameResolverBuilder#socketChannelType wants Class<? extends SocketChannel> channelType,
we need to cast the return value using asSubclass (or change the return type of Transports#socketChannelClass).
I didn't attach a test code about this but for just in case I checked we can cast known socket channels safely like:

    @Test
    void socketChannelClassCastTest() {
        // if failed ClassCastException is thrown.
        NioSocketChannel.class.asSubclass(SocketChannel.class);
        KQueueSocketChannel.class.asSubclass(SocketChannel.class);
        EpollSocketChannel.class.asSubclass(SocketChannel.class);
    }

}
}

private EventLoopGroup getEventLoopGroup(ConnectionPoint connectionPoint) {

for (;;) {
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/io/lettuce/core/RedisClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
*
* @author Will Glozer
* @author Mark Paluch
* @author Yohei Ueki
* @see RedisURI
* @see StatefulRedisConnection
* @see RedisFuture
Expand Down Expand Up @@ -322,6 +323,7 @@ private <K, V, S> ConnectionFuture<S> connectStatefulAsync(StatefulRedisConnecti
connectionBuilder(getSocketAddressSupplier(redisURI), connectionBuilder, redisURI);
connectionBuilder.connectionInitializer(createHandshake(state));
channelType(connectionBuilder, redisURI);
resolver(connectionBuilder, redisURI);
Comment on lines 325 to +326
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[Supplemental Comments]
I added resolver(...) all places where channelType(...) is used.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it makes sense to refactor the connect code a bit to pull things more together. I'll take care of this during the merge.


ConnectionFuture<RedisChannelHandler<K, V>> future = initializeChannelAsync(connectionBuilder);

Expand Down Expand Up @@ -599,6 +601,7 @@ private <K, V> ConnectionFuture<StatefulRedisSentinelConnection<K, V>> doConnect
connectionBuilder(getSocketAddressSupplier(redisURI), connectionBuilder, redisURI);

channelType(connectionBuilder, redisURI);
resolver(connectionBuilder, redisURI);
ConnectionFuture<?> sync = initializeChannelAsync(connectionBuilder);

return sync.thenApply(ignore -> (StatefulRedisSentinelConnection<K, V>) connection).whenComplete((ignore, e) -> {
Expand Down
22 changes: 22 additions & 0 deletions src/main/java/io/lettuce/core/Transports.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@
import io.netty.channel.Channel;
import io.netty.channel.EventLoopGroup;
import io.netty.channel.nio.NioEventLoopGroup;
import io.netty.channel.socket.DatagramChannel;
import io.netty.channel.socket.nio.NioDatagramChannel;
import io.netty.channel.socket.nio.NioSocketChannel;

/**
* Transport infrastructure utility class. This class provides {@link EventLoopGroup} and {@link Channel} classes for socket and
* native socket transports.
*
* @author Mark Paluch
* @author Yohei Ueki
* @since 4.4
*/
class Transports {
Expand Down Expand Up @@ -57,6 +60,18 @@ static Class<? extends Channel> socketChannelClass() {
return NioSocketChannel.class;
}

/**
* @return the default {@link DatagramChannel} for socket (network/UDP) transport.
*/
static Class<? extends DatagramChannel> datagramChannelClass() {

if (NativeTransports.isSocketSupported()) {
return NativeTransports.datagramChannelClass();
}

return NioDatagramChannel.class;
}

/**
* Native transport support.
*/
Expand All @@ -79,6 +94,13 @@ static Class<? extends Channel> socketChannelClass() {
return RESOURCES.socketChannelClass();
}

/**
* @return the native transport socket {@link DatagramChannel} class.
*/
static Class<? extends DatagramChannel> datagramChannelClass() {
return RESOURCES.datagramChannelClass();
}

/**
* @return the native transport domain socket {@link Channel} class.
*/
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/io/lettuce/core/cluster/RedisClusterClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@
* possible.
*
* @author Mark Paluch
* @author Yohei Ueki
* @since 3.0
* @see RedisURI
* @see StatefulRedisClusterConnection
Expand Down Expand Up @@ -815,6 +816,7 @@ private <K, V> ConnectionBuilder createConnectionBuilder(RedisChannelHandler<K,
connectionBuilder.commandHandler(commandHandlerSupplier);
connectionBuilder(socketAddressSupplier, connectionBuilder, connectionSettings);
channelType(connectionBuilder, connectionSettings);
resolver(connectionBuilder, connectionSettings);

return connectionBuilder;
}
Expand Down
18 changes: 18 additions & 0 deletions src/main/java/io/lettuce/core/resource/EpollProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@
import io.netty.channel.Channel;
import io.netty.channel.EventLoopGroup;
import io.netty.channel.epoll.Epoll;
import io.netty.channel.epoll.EpollDatagramChannel;
import io.netty.channel.epoll.EpollDomainSocketChannel;
import io.netty.channel.epoll.EpollEventLoopGroup;
import io.netty.channel.epoll.EpollSocketChannel;
import io.netty.channel.socket.DatagramChannel;
import io.netty.channel.unix.DomainSocketAddress;
import io.netty.util.concurrent.EventExecutorGroup;
import io.netty.util.internal.SystemPropertyUtil;
Expand All @@ -36,6 +38,7 @@
* the {@literal netty-transport-native-epoll} library during runtime. Internal API.
*
* @author Mark Paluch
* @author Yohei Ueki
* @since 4.4
*/
public class EpollProvider {
Expand Down Expand Up @@ -153,6 +156,13 @@ public Class<? extends Channel> socketChannelClass() {
return null;
}

@Override
public Class<? extends DatagramChannel> datagramChannelClass() {

checkForEpollLibrary();
return null;
}

}

/**
Expand Down Expand Up @@ -194,6 +204,14 @@ public Class<? extends Channel> socketChannelClass() {
return EpollSocketChannel.class;
}

@Override
public Class<? extends DatagramChannel> datagramChannelClass() {

checkForEpollLibrary();

return EpollDatagramChannel.class;
}

@Override
public Class<? extends EventLoopGroup> eventLoopGroupClass() {

Expand Down
7 changes: 7 additions & 0 deletions src/main/java/io/lettuce/core/resource/EventLoopResources.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@
import io.netty.channel.Channel;
import io.netty.channel.EventLoopGroup;
import io.netty.channel.epoll.EpollEventLoopGroup;
import io.netty.channel.socket.DatagramChannel;
import io.netty.util.concurrent.EventExecutorGroup;

/**
* Interface to encapsulate EventLoopGroup resources.
*
* @author Mark Paluch
* @author Yohei Ueki
* @since 6.0
*/
public interface EventLoopResources {
Expand Down Expand Up @@ -58,6 +60,11 @@ public interface EventLoopResources {
*/
Class<? extends Channel> socketChannelClass();

/**
* @return the {@link DatagramChannel} class.
*/
Class<? extends DatagramChannel> datagramChannelClass();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[Question; nits]
For consistency with socketChannelClass(), is it better to declare here as Class<? extends Channel> datagramChannelClass()?
It seems a bit wired because a method for TCP returns Channel while a method for UDP returns DatagramChannel.
Note that if we return Class<? extends Channel> here, we need cast the return value at
https://github.com/lettuce-io/lettuce-core/pull/1517/files#diff-954f0da1e02d2af777f9afc758b8b20f3f2df40a221f1a0e3569ef05402ccd97R310


/**
* @return the {@link EventLoopGroup} class.
*/
Expand Down
21 changes: 21 additions & 0 deletions src/main/java/io/lettuce/core/resource/KqueueProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@
import io.netty.channel.Channel;
import io.netty.channel.EventLoopGroup;
import io.netty.channel.kqueue.KQueue;
import io.netty.channel.kqueue.KQueueDatagramChannel;
import io.netty.channel.kqueue.KQueueDomainSocketChannel;
import io.netty.channel.kqueue.KQueueEventLoopGroup;
import io.netty.channel.kqueue.KQueueSocketChannel;
import io.netty.channel.socket.DatagramChannel;
import io.netty.channel.unix.DomainSocketAddress;
import io.netty.util.concurrent.EventExecutorGroup;
import io.netty.util.internal.SystemPropertyUtil;
Expand All @@ -36,6 +38,7 @@
* the {@literal netty-transport-native-kqueue} library during runtime. Internal API.
*
* @author Mark Paluch
* @author Yohei Ueki
* @since 4.4
*/
public class KqueueProvider {
Expand Down Expand Up @@ -153,6 +156,15 @@ public Class<? extends Channel> socketChannelClass() {
return null;
}

@Override
public Class<? extends DatagramChannel> datagramChannelClass() {


checkForKqueueLibrary();
return null;
}


}

/**
Expand Down Expand Up @@ -194,6 +206,15 @@ public Class<? extends Channel> socketChannelClass() {
return KQueueSocketChannel.class;
}

@Override
public Class<? extends DatagramChannel> datagramChannelClass() {

checkForKqueueLibrary();

return KQueueDatagramChannel.class;
}


@Override
public Class<? extends EventLoopGroup> eventLoopGroupClass() {

Expand Down