vert-x3 / vertx-redis-client

Redis client for Vert.x
http://vertx.io
Apache License 2.0
128 stars 116 forks source link

Fix reading CLUSTER SLOTS when host is null or empty #437

Closed Ladicek closed 5 months ago

Ladicek commented 5 months ago

Since Redis 7.0, the CLUSTER SLOTS command is allowed to return null or "" (empty string) as the hostname of a node that serves a given shard. This return value means that the "current" hostname (that was used to connect to the Redis node in order to execute CLUSTER SLOTS) should be used (but the port number must still be obtained from the result of CLUSTER SLOTS).

See https://redis.io/commands/cluster-slots/ for more info.

The main motivation here is a Quarkus user who reported the issue in Zulip: https://quarkusio.zulipchat.com/#narrow/stream/187030-users/topic/Redis.20client.20in.20cluster.20mode.20cannot.20parse.20.28just.29.20port.20number

Ladicek commented 5 months ago

I was able to configure a Redis cluster locally to return null as the host (cluster-preferred-endpoint-type unknown-endpoint), but that unfortunately isn't enough to write a test. This setting only makes sense when there's a load balancer, proxy or something similar, and I don't know what I could use for that. That's why this PR doesn't include a test. If anyone has any ideas, I'm all ears :-)

vietj commented 5 months ago

@Ladicek how about writing a TCP proxy with Vert.x NetClient/NetServer and use https://netty.io/4.1/api/io/netty/handler/codec/redis/RedisDecoder.html with the client/server to rewrite the protocol ?

vietj commented 5 months ago

https://github.com/eclipse-vertx/vert.x/blob/3be497f28d6b86201d8f6f8b1df28ddbc45052c2/src/test/java/io/vertx/core/net/NetTest.java#L3584

Ladicek commented 5 months ago

That sounds interesting! I'll explore around that, thanks!