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

Add support to the use of JedisSocketFactory using a pool #2293

Merged
merged 7 commits into from
Mar 28, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/main/java/redis/clients/jedis/BinaryJedis.java
Original file line number Diff line number Diff line change
Expand Up @@ -278,10 +278,21 @@ private void initializeFromURI(URI uri) {
}
}

/**
* @deprecated This will constructor will be removed in future releases
sazzad16 marked this conversation as resolved.
Show resolved Hide resolved
*
* Use {@link BinaryJedis#BinaryJedis(redis.clients.jedis.JedisSocketFactory, redis.clients.jedis.JedisClientConfig)}
sazzad16 marked this conversation as resolved.
Show resolved Hide resolved
*/
@Deprecated
public BinaryJedis(final JedisSocketFactory jedisSocketFactory) {
client = new Client(jedisSocketFactory);
}

public BinaryJedis(final JedisSocketFactory jedisSocketFactory, final JedisClientConfig clientConfig) {
client = new Client(jedisSocketFactory);
initializeFromClientConfig(clientConfig);
}

public boolean isConnected() {
return client.isConnected();
}
Expand Down
24 changes: 15 additions & 9 deletions src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.Socket;
import java.util.concurrent.atomic.AtomicReference;
import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLParameters;
import javax.net.ssl.SSLSocket;
Expand All @@ -16,7 +17,7 @@ public class DefaultJedisSocketFactory implements JedisSocketFactory {
protected static final HostAndPort DEFAULT_HOST_AND_PORT = new HostAndPort(Protocol.DEFAULT_HOST,
Protocol.DEFAULT_PORT);

private HostAndPort hostAndPort = DEFAULT_HOST_AND_PORT;
private final AtomicReference<HostAndPort> hostAndPort = new AtomicReference<>(DEFAULT_HOST_AND_PORT);
mina-asham marked this conversation as resolved.
Show resolved Hide resolved
private int connectionTimeout = Protocol.DEFAULT_TIMEOUT;
private int socketTimeout = Protocol.DEFAULT_TIMEOUT;
private boolean ssl = false;
Expand All @@ -36,7 +37,7 @@ public DefaultJedisSocketFactory(HostAndPort hostAndPort) {
public DefaultJedisSocketFactory(String host, int port, int connectionTimeout, int socketTimeout,
boolean ssl, SSLSocketFactory sslSocketFactory, SSLParameters sslParameters,
HostnameVerifier hostnameVerifier) {
this.hostAndPort = new HostAndPort(host, port);
this.hostAndPort.set(new HostAndPort(host, port));
this.connectionTimeout = connectionTimeout;
this.socketTimeout = socketTimeout;
this.ssl = ssl;
Expand All @@ -46,7 +47,7 @@ public DefaultJedisSocketFactory(String host, int port, int connectionTimeout, i
}

public DefaultJedisSocketFactory(HostAndPort hostAndPort, JedisClientConfig config) {
this.hostAndPort = hostAndPort;
this.hostAndPort.set(hostAndPort);
mina-asham marked this conversation as resolved.
Show resolved Hide resolved
if (config != null) {
this.connectionTimeout = config.getConnectionTimeoutMillis();
this.socketTimeout = config.getSocketTimeoutMillis();
Expand Down Expand Up @@ -105,6 +106,11 @@ public Socket createSocket() throws JedisConnectionException {
}
}

@Override
public void updateHostAndPort(HostAndPort hostAndPort) {
sazzad16 marked this conversation as resolved.
Show resolved Hide resolved
this.hostAndPort.set(hostAndPort);
}

public HostAndPort getSocketHostAndPort() {
HostAndPortMapper mapper = getHostAndPortMapper();
HostAndPort hostAndPort = getHostAndPort();
Expand All @@ -118,11 +124,11 @@ public HostAndPort getSocketHostAndPort() {
}

public HostAndPort getHostAndPort() {
return this.hostAndPort;
return this.hostAndPort.get();
}

public void setHostAndPort(HostAndPort hostAndPort) {
this.hostAndPort = hostAndPort;
this.hostAndPort.set(hostAndPort);
}

@Override
Expand All @@ -132,22 +138,22 @@ public String getDescription() {

@Override
public String getHost() {
return this.hostAndPort.getHost();
return this.hostAndPort.get().getHost();
}

@Override
public void setHost(String host) {
this.hostAndPort = new HostAndPort(host, this.hostAndPort.getPort());
this.hostAndPort.set(new HostAndPort(host, this.hostAndPort.get().getPort()));
}

@Override
public int getPort() {
return this.hostAndPort.getPort();
return this.hostAndPort.get().getPort();
}

@Override
public void setPort(int port) {
this.hostAndPort = new HostAndPort(this.hostAndPort.getHost(), port);
this.hostAndPort.set(new HostAndPort(this.hostAndPort.get().getHost(), port));
}

@Override
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/redis/clients/jedis/Jedis.java
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,20 @@ public Jedis(final URI uri, JedisClientConfig config) {
super(uri, config);
}

/**
* @deprecated This will constructor will be removed in future releases
*
* Use {@link Jedis#Jedis(redis.clients.jedis.JedisSocketFactory, redis.clients.jedis.JedisClientConfig)}
sazzad16 marked this conversation as resolved.
Show resolved Hide resolved
*/
@Deprecated
public Jedis(final JedisSocketFactory jedisSocketFactory) {
super(jedisSocketFactory);
}

public Jedis(final JedisSocketFactory jedisSocketFactory, final JedisClientConfig clientConfig) {
super(jedisSocketFactory, clientConfig);
}

/**
* COPY source destination [DB destination-db] [REPLACE]
*
Expand Down
35 changes: 25 additions & 10 deletions src/main/java/redis/clients/jedis/JedisFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public class JedisFactory implements PooledObjectFactory<Jedis> {

private static final Logger logger = LoggerFactory.getLogger(JedisFactory.class);

private final AtomicReference<HostAndPort> hostAndPort = new AtomicReference<>();
private final AtomicReference<JedisSocketFactory> jedisSocketFactory = new AtomicReference<>();
mina-asham marked this conversation as resolved.
Show resolved Hide resolved

private final JedisClientConfig clientConfig;

Expand Down Expand Up @@ -66,20 +66,25 @@ protected JedisFactory(final String host, final int port, final int connectionTi
}

protected JedisFactory(final HostAndPort hostAndPort, final JedisClientConfig clientConfig) {
this.hostAndPort.set(hostAndPort);
this.clientConfig = DefaultJedisClientConfig.copyConfig(clientConfig);
this.jedisSocketFactory.set(new DefaultJedisSocketFactory(hostAndPort, this.clientConfig));
}

protected JedisFactory(final String host, final int port, final int connectionTimeout, final int soTimeout,
final int infiniteSoTimeout, final String user, final String password, final int database,
final String clientName, final boolean ssl, final SSLSocketFactory sslSocketFactory,
final SSLParameters sslParameters, final HostnameVerifier hostnameVerifier) {
this.hostAndPort.set(new HostAndPort(host, port));
this.clientConfig = DefaultJedisClientConfig.builder().connectionTimeoutMillis(connectionTimeout)
.socketTimeoutMillis(soTimeout).blockingSocketTimeoutMillis(infiniteSoTimeout).user(user)
.password(password).database(database).clientName(clientName)
.ssl(ssl).sslSocketFactory(sslSocketFactory)
.sslParameters(sslParameters).hostnameVerifier(hostnameVerifier).build();
this.jedisSocketFactory.set(new DefaultJedisSocketFactory(new HostAndPort(host, port), this.clientConfig));
}

protected JedisFactory(final JedisSocketFactory jedisSocketFactory, final JedisClientConfig clientConfig) {
this.clientConfig = DefaultJedisClientConfig.copyConfig(clientConfig);
this.jedisSocketFactory.set(jedisSocketFactory);
}

/**
Expand Down Expand Up @@ -120,17 +125,26 @@ protected JedisFactory(final URI uri, final int connectionTimeout, final int soT
throw new InvalidURIException(String.format(
"Cannot open Redis connection due invalid URI. %s", uri.toString()));
}
this.hostAndPort.set(new HostAndPort(uri.getHost(), uri.getPort()));
this.clientConfig = DefaultJedisClientConfig.builder().connectionTimeoutMillis(connectionTimeout)
.socketTimeoutMillis(soTimeout).blockingSocketTimeoutMillis(infiniteSoTimeout)
.user(JedisURIHelper.getUser(uri)).password(JedisURIHelper.getPassword(uri))
.database(JedisURIHelper.getDBIndex(uri)).clientName(clientName)
.ssl(JedisURIHelper.isRedisSSLScheme(uri)).sslSocketFactory(sslSocketFactory)
.sslParameters(sslParameters).hostnameVerifier(hostnameVerifier).build();
this.jedisSocketFactory.set(new DefaultJedisSocketFactory(new HostAndPort(uri.getHost(), uri.getPort()), this.clientConfig));
}

public void setHostAndPort(final HostAndPort hostAndPort) {
mina-asham marked this conversation as resolved.
Show resolved Hide resolved
this.hostAndPort.set(hostAndPort);
// This can only happen if we use a constructor that requires setHostAndPort to be called after,
// in that case we need to give it an initial value, so we use the default socket factory
if (jedisSocketFactory.get() == null) {
JedisSocketFactory factory = new DefaultJedisSocketFactory(hostAndPort, this.clientConfig);
// If someone has some logic that can call this code twice in parallel (highly unlikely!)
// this should protect against setting it with the default twice
if (!this.jedisSocketFactory.compareAndSet(null, factory)) {
this.jedisSocketFactory.get().updateHostAndPort(hostAndPort);
}
}
}

public void setPassword(final String password) {
Expand Down Expand Up @@ -167,10 +181,9 @@ public void destroyObject(PooledObject<Jedis> pooledJedis) throws Exception {

@Override
public PooledObject<Jedis> makeObject() throws Exception {
final HostAndPort hostPort = this.hostAndPort.get();
Jedis jedis = null;
try {
jedis = new Jedis(hostPort, clientConfig);
jedis = new Jedis(jedisSocketFactory.get(), clientConfig);
jedis.connect();
return new DefaultPooledObject<>(jedis);
} catch (JedisException je) {
Expand Down Expand Up @@ -199,13 +212,15 @@ public void passivateObject(PooledObject<Jedis> pooledJedis) throws Exception {
public boolean validateObject(PooledObject<Jedis> pooledJedis) {
final BinaryJedis jedis = pooledJedis.getObject();
try {
HostAndPort hostAndPort = this.hostAndPort.get();
JedisSocketFactory factory = this.jedisSocketFactory.get();
String host = factory.getHost();
int port = factory.getPort();

String connectionHost = jedis.getClient().getHost();
int connectionPort = jedis.getClient().getPort();

return hostAndPort.getHost().equals(connectionHost)
&& hostAndPort.getPort() == connectionPort && jedis.isConnected()
return host.equals(connectionHost)
&& port == connectionPort && jedis.isConnected()
&& jedis.ping().equals("PONG");
} catch (final Exception e) {
return false;
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/redis/clients/jedis/JedisPool.java
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,11 @@ public JedisPool(final GenericObjectPoolConfig<Jedis> poolConfig, final HostAndP
super(poolConfig, new JedisFactory(hostAndPort, clientConfig));
}

public JedisPool(final GenericObjectPoolConfig<Jedis> poolConfig, final JedisSocketFactory jedisSocketFactory,
final JedisClientConfig clientConfig) {
super(poolConfig, new JedisFactory(jedisSocketFactory, clientConfig));
}

public JedisPool(final GenericObjectPoolConfig<Jedis> poolConfig) {
this(poolConfig, Protocol.DEFAULT_HOST, Protocol.DEFAULT_PORT);
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/redis/clients/jedis/JedisSocketFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ public interface JedisSocketFactory {
*/
Socket createSocket() throws IOException, JedisConnectionException;

void updateHostAndPort(HostAndPort hostAndPort);

@Deprecated
String getDescription();

Expand Down
6 changes: 6 additions & 0 deletions src/test/java/redis/clients/jedis/tests/UdsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.newsclub.net.unix.AFUNIXSocket;
import org.newsclub.net.unix.AFUNIXSocketAddress;

import redis.clients.jedis.HostAndPort;
import redis.clients.jedis.Jedis;
import redis.clients.jedis.JedisSocketFactory;
import redis.clients.jedis.Protocol;
Expand Down Expand Up @@ -38,6 +39,11 @@ public Socket createSocket() throws JedisConnectionException {
}
}

@Override
public void updateHostAndPort(HostAndPort hostAndPort) {
throw new UnsupportedOperationException("UDS cannot update host and port");
}

@Override
public String getDescription() {
return UDS_SOCKET.toString();
Expand Down