redis / jedis

Redis Java client
MIT License
11.79k stars 3.86k forks source link

Cannot connect to Redis using Sentinelhost:port and a specified database number #2567

Closed steam0 closed 3 years ago

steam0 commented 3 years ago

Expected behavior

Connecting to Redis using Jedis with sentinels and datbase number should connect to the "cluster" and put keys in the specified database number

Actual behavior

While trying to connect to the cluster using a sentinels-host:port and a database number greater than 0 the funciton call select(dbIndex) returns an error ERR unknown command 'SELECT', with args beginning with '1',".

In the previous version of Jedis that I used (3.3.0) this configuration works fine and I can connect.

Steps to reproduce:

Create a new Spring Boot project, configure database number 1 and connect to Redis using a sentinels host:port.

Redis / Jedis Configuration

Other:

Spring Boot: 2.5.1 Spring Data Redis: 2.5.1 (and 2.5.2 if it matters)

Jedis version:

3.6.0 (and 3.6.1)

Redis version:

redis_version:5.0.3

Java version:

Java 11

Comments

I am by no means an expert in Redis or Redis Sentinels. For what I can understand it seems like Redis Sentinels have no command "SELECT " and that is why this fails.

However I can see that in our different qa and production environments where we have used Jedis 3.3.0 (and older) we see that our Redis installation have keys spread out over the different databases (but only the once we have explicitly specified)

In the example below we see that we have 1 key in db:12 which is the one I tried to test with.

# Keyspace
db1:keys=326,expires=326,avg_ttl=28478186582
db4:keys=120,expires=120,avg_ttl=7022392
db6:keys=27,expires=27,avg_ttl=11324440008
db12:keys=1,expires=1,avg_ttl=93543547

When I look at the previous version of my code (Spring Boot 2.4.5, Spring Data Redis 2.4.8, Jedis 3.3.0) I see that the constructor used is the following piece of code.

JedisSentinelPool.java

207  jedis = new Jedis(hap.getHost(), hap.getPort(), sentinelConnectionTimeout, sentinelSoTimeout);

This code creates a new Jedis instance using host and port (to a sentinel instance)

In the newest version of my code (Spring Boot 2.5.1, Spring Data Redis 2.5.1, Jedis 3.6.0) I see that the constructor used is using a sentinelClientconfig

JedisSentinelPool.java

251 try (Jedis jedis = new Jedis(sentinel, sentinelClientConfig)) {

which then ends up in

BinaryJedis.java

85 private void initializeFromClientConfig(JedisClientConfig config) {
...     (...)
97     int dbIndex = config.getDatabase();
98     if (dbIndex > 0) {
99       select(dbIndex);
100    }

And it is this piece of code that throws an exception. I am not sure how it is meant to work, but it seems like this initializeFromClientConfig-function is accepting any JedisClientConfig even if it comes through the JedisSentinelPool.java-class.

Screenshot 2021-06-23 at 17 07 39
sazzad16 commented 3 years ago

@steam0 Is your issue relates to https://github.com/redis/jedis/issues/2523, https://github.com/spring-projects/spring-data-redis/issues/2053 and https://github.com/spring-projects/spring-data-redis/issues/2057 ? If so, I'd suggest you to follow the guideline of spring-data-redis.

steam0 commented 3 years ago

@sazzad16 I'm not sure if I agree. The issue with spring-data-redis#2053 on spring-data-redis is named Database index not applied to Jedis client and this is excatly the oppsite of what I am seeing?

In my issue, the connection fails inside the Jedis client because I configure a database number. The function call select(dbIndex) is triggered which uses a SELECT-command and that makes no sense since the Sentinel doesnt have this command available.

Or am I missing something here?

sazzad16 commented 3 years ago

... because I configure a database number. ... which uses a SELECT-command and that makes no sense since the Sentinel doesnt have this command available

@steam0 If you know Sentinel doesn't have SELECT command available then why would you configure a database number?

steam0 commented 3 years ago

@sazzad16 Because you can still use database numbers with sentinels? Or at least it did work before. I tried to write a short piece about it in my original post.

I am by no means an expert in Redis or Redis Sentinels. For what I can understand it seems like Redis Sentinels have no command "SELECT " and that is why this fails.

However I can see that in our different qa and production environments where we have used Jedis 3.3.0 (and older) we see that our Redis installation have keys spread out over the different databases (but only the once we have explicitly specified)

In the example below we see that we have 1 key in db:12 which is the one I tried to test with.

Keyspace

db1:keys=326,expires=326,avg_ttl=28478186582 db4:keys=120,expires=120,avg_ttl=7022392 db6:keys=27,expires=27,avg_ttl=11324440008 db12:keys=1,expires=1,avg_ttl=93543547

EDIT I am sorry, but Redis is definitely not my strong suit. I think that you can use sentinels and database numbers, because I see that using older versions of Jedis client actually puts the keys in the database that I configure

sazzad16 commented 3 years ago

@steam0

There are two types of nodes involved in Sentinel; the sentinel nodes and the master node(s). Sentinel nodes do not process data and so they do not support commands related to data processing (like SELECT). Master node(s) process data and support those commands. Please put the config parameters according to your needs. If Jedis does not work properly with right inputs, it will definitely be considered a bug and we can try to fix it or at least address it.

Previous successful run with wrong inputs does not invalidate the justification of failure to run (with wrong inputs). It is not always feasible to handle all possible wrong inputs.

Thank you for your understanding.

steam0 commented 3 years ago

@sazzad16

There are two types of nodes involved in Sentinel; the sentinel nodes and the master node(s). Sentinel nodes do not process data and so they do not support commands related to data processing (like SELECT).

Sure, I get that. But how are you supposed to configure which database on the master node that you are supposed to use? Are you telling me that while using Redis Sentinels we cannot tell Jedis which database we wish to use? Is that not a feature available while using Redis Sentinels all together (even outside the context of Jedis)?

As stated in the original post, we are inside the Jedis Library in JedisSentinelPool.java in the initSentinels(Set<HostAndPort> sentinels, final String masterName) function on line 240. On line 251 we execute new Jedis(sentinel, sentinelClientConfig) and the latter argument contains data as seen in the image below. (Note the database number)

Screenshot 2021-06-24 at 19 20 59

Inside this "new Jedis()"-function, the function initializeFromClientConfig(config) is executed and this function performs the select(dbIndex) call which crashes the application.

If it is not possible to use Sentinels and database number > 0, why does the initSentinels-function even allow it? Shouldn't there be separate "init" functions for sentinel configs and standalone configs?

Thank you so much for your insight so far

sazzad16 commented 3 years ago

@steam0 While implementing this, we had few, mainly two, options.

  1. Have several configs like:

    • ConnectionConfig
    • JedisConfig
    • JedisPoolConfig
    • JedisFactoryConfig
    • JedisClusterConfig
    • JedisClusterConnectionHandlerConfig
    • JedisClusterSeedConfig
    • JedisClusterMasterConfig
    • JedisCluster(Replica/Slave)Config
    • ShardedJedisConfig
    • ShardedJedisPoolConfig
    • JedisSentinelMasterConfig
    • JedisSentinelMasterPoolConfig
    • JedisSentinelSentinelConfig
    • etc. where there would be a lot of conversion from one config to another, meaning greater chance of unintended bugs.
  2. Have very few generic configs which ultimately became just one

    • JedisClientConfig and expect that the users will set the params accordingly.

We chose the second option and it seems that decision is not 100% suitable for your requirement. I sincerely apologize for that.

By the way, this is an open source project and you are very much welcome to craft a PR as per your requirement.

Thank you.

steam0 commented 3 years ago

Ok, so if I understand you correctly then Jedis does no longer support using Sentinels and Database numbers > 0. Even if the previous support was by accident.

I’m unfortunately not skilled enough with Redis to contribute to this project.

Thank you for your time

sazzad16 commented 3 years ago

@steam0 You're welcome.