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.

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

JedisSentinelPool contains a buggy constructor #2572

Closed
fancige opened this issue Jun 30, 2021 · 5 comments
Closed

JedisSentinelPool contains a buggy constructor #2572

fancige opened this issue Jun 30, 2021 · 5 comments
Labels
Milestone

Comments

@fancige
Copy link

fancige commented Jun 30, 2021

Expected behavior

Can connect to sentinel redis with password

Actual behavior

redis.clients.jedis.exceptions.JedisDataException: NOAUTH Authentication required.
at redis.clients.jedis.Protocol.processError(Protocol.java:135)
at redis.clients.jedis.Protocol.process(Protocol.java:169)
at redis.clients.jedis.Protocol.read(Protocol.java:223)
at redis.clients.jedis.Connection.readProtocolWithCheckingBroken(Connection.java:352)
at redis.clients.jedis.Connection.getUnflushedObjectMultiBulkReply(Connection.java:314)
at redis.clients.jedis.Connection.getObjectMultiBulkReply(Connection.java:319)
at redis.clients.jedis.Jedis.sentinelGetMasterAddrByName(Jedis.java:3451)
at redis.clients.jedis.JedisSentinelPool.initSentinels(JedisSentinelPool.java:253)
at redis.clients.jedis.JedisSentinelPool.(JedisSentinelPool.java:205)
at redis.clients.jedis.JedisSentinelPool.(JedisSentinelPool.java:186)
at redis.clients.jedis.JedisSentinelPool.(JedisSentinelPool.java:169)
at redis.clients.jedis.JedisSentinelPool.(JedisSentinelPool.java:159)

Steps to reproduce:

Please create a reproducible case of your problem. Make sure
that case repeats consistently and it's not random
1.use this constructor below, source code line 154, to create a JedisSentinelPool

JedisSentinelPool(String masterName, Set sentinels,
final GenericObjectPoolConfig poolConfig, final int connectionTimeout, final int soTimeout,
final String user, final String password, final int database, final String clientName,
final int sentinelConnectionTimeout, final int sentinelSoTimeout, final String sentinelUser,
final String sentinelPassword, final String sentinelClientName)

2.then the constructor below, source code line 186, invoke DefaultJedisClientConfig.builder().build() to create an empty JedisClientConfig which would cause the all informations passed by the first step lose

public JedisSentinelPool(String masterName, Set sentinels,
final GenericObjectPoolConfig poolConfig, final JedisFactory factory) {
this(masterName, parseHostAndPorts(sentinels), poolConfig, factory,
DefaultJedisClientConfig.builder().build());
}

Redis / Jedis Configuration

Jedis version:

3.6.1

Redis version:

5.0.4

Java version:

1.8

@sazzad16
Copy link
Collaborator

@fancige

  public JedisSentinelPool(String masterName, Set<String> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig,
      final int connectionTimeout, final int soTimeout, final int infiniteSoTimeout,
      final String user, final String password, final int database, final String clientName,
      final int sentinelConnectionTimeout, final int sentinelSoTimeout, final String sentinelUser,
      final String sentinelPassword, final String sentinelClientName)

which calls

  public JedisSentinelPool(String masterName, Set<HostAndPort> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig, final JedisClientConfig masteClientConfig,
      final JedisClientConfig sentinelClientConfig)

which then finally calls

  public JedisSentinelPool(String masterName, Set<HostAndPort> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig, final JedisFactory factory,
      final JedisClientConfig sentinelClientConfig)

Nothing is lost in any of those stages.

@sazzad16
Copy link
Collaborator

PS: DefaultJedisClientConfig.builder().build() is used only in

  public JedisSentinelPool(String masterName, Set<String> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig, final JedisFactory factory)

where no sentinel socket configs are provided.

@fancige
Copy link
Author

fancige commented Jun 30, 2021

PS: DefaultJedisClientConfig.builder().build() is used only in

  public JedisSentinelPool(String masterName, Set<String> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig, final JedisFactory factory)

where no sentinel socket configs are provided.

but the constructor is called by annother constructor, check the code at line 169,

this(masterName, sentinels, poolConfig, new JedisFactory(connectionTimeout, soTimeout, infiniteSoTimeout, user, password, database, clientName));

jedis version is 3.6.1

full code (line 154-188) look like this

public JedisSentinelPool(String masterName, Set<String> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig, final int connectionTimeout, final int soTimeout,
      final String user, final String password, final int database, final String clientName,
      final int sentinelConnectionTimeout, final int sentinelSoTimeout, final String sentinelUser,
      final String sentinelPassword, final String sentinelClientName) {
    this(masterName, sentinels, poolConfig, connectionTimeout, soTimeout, 0, user, password, database, clientName,
        sentinelConnectionTimeout, sentinelSoTimeout, sentinelUser, sentinelPassword, sentinelClientName);
  }

  public JedisSentinelPool(String masterName, Set<String> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig,
      final int connectionTimeout, final int soTimeout, final int infiniteSoTimeout,
      final String user, final String password, final int database, final String clientName,
      final int sentinelConnectionTimeout, final int sentinelSoTimeout, final String sentinelUser,
      final String sentinelPassword, final String sentinelClientName) {
    this(masterName, sentinels, poolConfig, new JedisFactory(connectionTimeout, soTimeout, infiniteSoTimeout, user, password, database, clientName));
    this.connectionTimeout = connectionTimeout;
    this.soTimeout = soTimeout;
    this.infiniteSoTimeout = infiniteSoTimeout;
    this.user = user;
    this.password = password;
    this.database = database;
    this.clientName = clientName;
    this.sentinelConnectionTimeout = sentinelConnectionTimeout;
    this.sentinelSoTimeout = sentinelSoTimeout;
    this.sentinelUser = sentinelUser;
    this.sentinelPassword = sentinelPassword;
    this.sentinelClientName = sentinelClientName;
  }

  public JedisSentinelPool(String masterName, Set<String> sentinels,
      final GenericObjectPoolConfig<Jedis> poolConfig, final JedisFactory factory) {
    this(masterName, parseHostAndPorts(sentinels), poolConfig, factory,
        DefaultJedisClientConfig.builder().build());
  }

@sazzad16
Copy link
Collaborator

@fancige Ahh, got it. Thanks!

@sazzad16 sazzad16 reopened this Jun 30, 2021
@sazzad16 sazzad16 added the bug label Jun 30, 2021
@sazzad16 sazzad16 added this to the 3.6.2 milestone Jul 8, 2021
@sazzad16
Copy link
Collaborator

Fixed by #2574 and #2582

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants