Add support for ReadFrom.subnet and ReadFrom.regex

[yueki1993]

Lettuce now supports ReadFrom.subnet which is an option of reading from any node in the given subnets.

Closes: #1569

Make sure that:

• 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 use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
• You submit test cases (unit or integration tests) that back your changes.

[codecov]

Codecov Report

Merging #1570 (c298d90) into main (9e30d62) will increase coverage by 0.23%.
The diff coverage is 82.19%.

@@             Coverage Diff              @@
##               main    #1570      +/-   ##
============================================
+ Coverage     78.64%   78.88%   +0.23%     
- Complexity     6341     6407      +66     
============================================
  Files           477      479       +2     
  Lines         21335    21539     +204     
  Branches       2334     2360      +26     
============================================
+ Hits          16779    16990     +211     
+ Misses         3479     3474       -5     
+ Partials       1077     1075       -2     

Impacted Files	Coverage Δ	Complexity Δ
src/main/java/io/lettuce/core/ReadFromImpl.java	84.46% <80.59%> (-7.21%)	2.00 <0.00> (ø)
src/main/java/io/lettuce/core/ReadFrom.java	80.00% <100.00%> (+3.52%)	15.00 <2.00> (+4.00)
...o/lettuce/core/event/command/CommandBaseEvent.java	53.84% <0.00%> (-46.16%)	3.00% <0.00%> (-1.00%)
...n/java/io/lettuce/core/cluster/ClusterCommand.java	70.73% <0.00%> (-17.08%)	18.00% <0.00%> (-1.00%)
...ain/java/io/lettuce/core/SslConnectionBuilder.java	88.88% <0.00%> (-1.59%)	6.00% <0.00%> (ø%)
.../io/lettuce/core/dynamic/ReactiveTypeAdapters.java	86.52% <0.00%> (-0.87%)	1.00% <0.00%> (ø%)
src/main/java/io/lettuce/core/RedisClient.java	87.34% <0.00%> (-0.85%)	91.00% <0.00%> (ø%)
...re/cluster/StatefulRedisClusterConnectionImpl.java	75.72% <0.00%> (-0.69%)	35.00% <0.00%> (-2.00%)
...pi/coroutines/RedisStringCoroutinesCommandsImpl.kt	8.57% <0.00%> (-0.52%)	3.00% <0.00%> (ø%)
...java/io/lettuce/core/protocol/DefaultEndpoint.java	69.60% <0.00%> (-0.47%)	99.00% <0.00%> (ø%)
... and 25 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e30d62...c298d90. Read the comment docs.

[yueki1993]

[Question] As far as I tried with debugger, node.getUri() always returns IP address rather than hostname like not redis://foobar.com but redis://192.168.0.1.
Is there any case of returning hostname here?
My concern is, if there is any case that node.getUri() returns initial endpoint, this code may not work as we intend because the initial endpoint may be associated with multiple IP addresses.

For AWS specific, they issue configuration endpoint (e.g., foobar.cache.amazonaws.com) and we use it for initial endpoint. The endpoint is associated with (almost all) cluster nodes' ip address.

$ dig foobar.cache.amazonaws.com +short
192.168.0.1
192.168.0.2
192.168.0.3

So there is any case that node.getUri() returns initial endpoint, its IP address is any one of cluster nodes', thus this class does not work as we intend.

[mp911de]

The host part of RedisURI depends on from where the URI was supplied. In a Redis Cluster or Redis Sentinel arrangement, the topology source is always Redis itself which reports IP addresses only.

When using Master/Replica with provided URL's, then you get hold of the hostname.

ReadFrom gets always called with the target node. Does that help?

[yueki1993]

Thank you for the information and review. I will investigate the behaviour in detail!

[mp911de]

This line can lead to a DNS lookup (blocking call). Would createUnresolved work as well?

[yueki1993]

IpSubnetFilterRule uses InetSocketAddress#getAddress, so createUnresolved would not work because getAddress returns always null for unresolved socket addresses.
InetAddress#getByName does not make DNS resolution if IP address is given (see javadoc), so I use it and I add hostname validation to avoid unintended DNS lookup.

[yueki1993]

I feel DNS resolving in ReadFrom.subnet is bad idea because:

1. Associated IPaddress with a hostname can be changed and a hostname can hold multiple IPs.
   There can be inconsistency with remote IP of actual TCP connection and result of dns looking up in ReadFrom.
2. We need to introduce reactive method Mono<List<RedisNodeDescription>> select(Nodes nodes) to avoid blocking operation. I did not estimate how much workload is required but it should be tough.

At first I came up with holding remote IP address when connection established in RedisURI or RedisNodeDescription.
However we need to modify the core part of lettuce to achieve it, and it is very difficult to get real ipaddress of a redis node when considering port forwarding, NAT, middlewares etc, so I could not come up with perfect solution which can meet every requirements.

So simply I want to propose as follows:

• ReadFrom.subnet is available only if ip-address style RedisURI is provided, i.e., Redis Cluster and Redis Sentinel with dynamic topology discovery.
  • Note that if Add hostname support in Sentinel. redis#8282 is merged, this feature should be affected for redis sentinel with dynamic topology discovery.
• Domain-name style RedisURI is not supported and if given we will throw RuntimeException.

[mp911de]

Looking at the rule implementations in netty, I wonder whether we could basically implement IPv4 and IPv6 rules on our own by parsing the IP address into long respective BigInteger (for IPv6).

We could approach the case where hostnames are used by a ReadFrom that accepts a Set<String>. Basically, that would allow filtering as well but based on hostnames.

[yueki1993]

You mean, we maintain a subnet as Set<Integer> (or Set<BigInteger>) , e.g., for a subnet 192.168.0.0/24 we use a set that holds integer notation of 192.168.0.0, 192.168.0.1, ..., 192.168.0.255? If so standard Set implementations (HashSet, TreeSet) theirself do not work because up to 2^64 elements must be maintained.
We can achieve it by providing RangeSet implementation, but before implementing it I want to confirm your intent, e.g., you want to avoid too much dependency to netty, worry about performance, or want to make the interface more generic etc.

[mp911de]

Sorry, I think my comment was a bit confusing. I actually made two proposals:

1. Implement IP to long/BigInteger parsing our selves and apply subnet masking for IPv4 and IPv6 addresses. That way we can entirely avoid DNS lookups by not using InetSocketAddress
2. Introduce another ReadFrom that accepts a Set<String> hostnames for the case where someone uses hostnames (e.g. the future Sentinel enhancement).

[yueki1993]

Got it, thanks. Those proposals sound reasonable to me.
Will tackle at the weekend.

[yueki1993]

For hostname-based matching, I feel regex-based is more in demand than set-based because it is more simple and extensible, but what do you think? If not I will replace to set-based as originally proposed.

[mp911de]

I think that's fine, too.

[yueki1993]

To avoid license issue (though netty is Apache license), IPv4/IPv6 rules are re-implemented by myself, so some detailed implementations are different from Netty's one.

[mp911de]

Thank you for your contribution. That's merged and polished now.