Skip to content

Conversation

@Talon876
Copy link
Contributor

This change allows connecting to sentinel via TLS. Additionally, you can specify if you want to connect to the redis master with TLS or not.

When connecting to sentinel with TLS, it will still broadcast the non-TLS ports for redis. For this scenario to work, toHostAndPort was modified to be protected so consumers can sub-class JedisSentinelPool and implement your own port mapping behavior.

@gkorland gkorland requested a review from sazzad16 March 22, 2020 12:38
protected String sentinelPassword;
protected String sentinelClientName;

protected boolean isRedisSslEnabled;
Copy link
Contributor

Choose a reason for hiding this comment

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

Master would be better that Redis. E.g. isMasterSslEnabled. Also, apply this change entire PR.

Comment on lines 105 to +111
public JedisSentinelPool(String masterName, Set<String> sentinels,
final GenericObjectPoolConfig poolConfig, final int connectionTimeout, final int soTimeout,
final String password, final int database, final String clientName,
final int sentinelConnectionTimeout, final int sentinelSoTimeout, final String sentinelPassword,
final String sentinelClientName) {
final String sentinelClientName, final boolean isRedisSslEnabled, final boolean isSentinelSslEnabled,
final SSLSocketFactory sslSocketFactory, final SSLParameters sslParameters,
final HostnameVerifier hostnameVerifier) {
Copy link
Contributor

Choose a reason for hiding this comment

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

DON'T change an existing constructor or public method. Create a new one.

public MasterListener(String masterName, String host, int port,
long subscribeRetryWaitTimeMillis) {
this(masterName, host, port);
long subscribeRetryWaitTimeMillis) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
long subscribeRetryWaitTimeMillis) {
long subscribeRetryWaitTimeMillis) {

@sazzad16 sazzad16 added this to the 3.6.0 milestone Mar 18, 2021
sazzad16 added a commit that referenced this pull request Mar 18, 2021
Thanks @Talon876 for the test instances in the Makefile (#2139)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants