Skip to content

Commit d9c5e0a

Browse files
mina-ashamMina Ashamsazzad16
authored
Add support to the use of JedisSocketFactory using a pool (#2293)
* Add support to the use of JedisSocketFactory using a pool - Support for JedisSocketFactory has already been added to the lowest level Jedis to support adding any custom socket factory (e.g. UDS), this propagates the support in the JedisPool too - Also fix Jedis/BinaryJedis constructors that broke after the introduction of config due to missing client initialization * Rework host and port updates through the socket factory * Add a new constructor to DefaultJedisSocketFactory that accepts a JedisClientConfig and cleanup JedisFactory * Only set host and port if we pass a non-null value * Update Deprecation JavaDoc * Use a volatile instead of atomic reference since we are not doing any CAS operations Co-authored-by: Mina Asham <minaasha@amazon.com> Co-authored-by: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com>
1 parent bda6f99 commit d9c5e0a

File tree

7 files changed

+63
-13
lines changed

7 files changed

+63
-13
lines changed

src/main/java/redis/clients/jedis/BinaryJedis.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,10 +278,21 @@ private void initializeFromURI(URI uri) {
278278
}
279279
}
280280

281+
/**
282+
* @deprecated This constructor will be removed in future major release.
283+
*
284+
* Use {@link BinaryJedis#BinaryJedis(redis.clients.jedis.JedisSocketFactory, redis.clients.jedis.JedisClientConfig)}.
285+
*/
286+
@Deprecated
281287
public BinaryJedis(final JedisSocketFactory jedisSocketFactory) {
282288
client = new Client(jedisSocketFactory);
283289
}
284290

291+
public BinaryJedis(final JedisSocketFactory jedisSocketFactory, final JedisClientConfig clientConfig) {
292+
client = new Client(jedisSocketFactory);
293+
initializeFromClientConfig(clientConfig);
294+
}
295+
285296
public boolean isConnected() {
286297
return client.isConnected();
287298
}

src/main/java/redis/clients/jedis/DefaultJedisSocketFactory.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public class DefaultJedisSocketFactory implements JedisSocketFactory {
1616
protected static final HostAndPort DEFAULT_HOST_AND_PORT = new HostAndPort(Protocol.DEFAULT_HOST,
1717
Protocol.DEFAULT_PORT);
1818

19-
private HostAndPort hostAndPort = DEFAULT_HOST_AND_PORT;
19+
private volatile HostAndPort hostAndPort = DEFAULT_HOST_AND_PORT;
2020
private int connectionTimeout = Protocol.DEFAULT_TIMEOUT;
2121
private int socketTimeout = Protocol.DEFAULT_TIMEOUT;
2222
private boolean ssl = false;
@@ -32,6 +32,10 @@ public DefaultJedisSocketFactory(HostAndPort hostAndPort) {
3232
this(hostAndPort, null);
3333
}
3434

35+
public DefaultJedisSocketFactory(JedisClientConfig config) {
36+
this(null, config);
37+
}
38+
3539
@Deprecated
3640
public DefaultJedisSocketFactory(String host, int port, int connectionTimeout, int socketTimeout,
3741
boolean ssl, SSLSocketFactory sslSocketFactory, SSLParameters sslParameters,
@@ -46,7 +50,9 @@ public DefaultJedisSocketFactory(String host, int port, int connectionTimeout, i
4650
}
4751

4852
public DefaultJedisSocketFactory(HostAndPort hostAndPort, JedisClientConfig config) {
49-
this.hostAndPort = hostAndPort;
53+
if (hostAndPort != null) {
54+
this.hostAndPort = hostAndPort;
55+
}
5056
if (config != null) {
5157
this.connectionTimeout = config.getConnectionTimeoutMillis();
5258
this.socketTimeout = config.getSocketTimeoutMillis();
@@ -105,6 +111,11 @@ public Socket createSocket() throws JedisConnectionException {
105111
}
106112
}
107113

114+
@Override
115+
public void updateHostAndPort(HostAndPort hostAndPort) {
116+
this.hostAndPort = hostAndPort;
117+
}
118+
108119
public HostAndPort getSocketHostAndPort() {
109120
HostAndPortMapper mapper = getHostAndPortMapper();
110121
HostAndPort hostAndPort = getHostAndPort();

src/main/java/redis/clients/jedis/Jedis.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,20 @@ public Jedis(final URI uri, JedisClientConfig config) {
153153
super(uri, config);
154154
}
155155

156+
/**
157+
* @deprecated This constructor will be removed in future major release.
158+
*
159+
* Use {@link Jedis#Jedis(redis.clients.jedis.JedisSocketFactory, redis.clients.jedis.JedisClientConfig)}.
160+
*/
161+
@Deprecated
156162
public Jedis(final JedisSocketFactory jedisSocketFactory) {
157163
super(jedisSocketFactory);
158164
}
159165

166+
public Jedis(final JedisSocketFactory jedisSocketFactory, final JedisClientConfig clientConfig) {
167+
super(jedisSocketFactory, clientConfig);
168+
}
169+
160170
/**
161171
* COPY source destination [DB destination-db] [REPLACE]
162172
*

src/main/java/redis/clients/jedis/JedisFactory.java

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package redis.clients.jedis;
22

33
import java.net.URI;
4-
import java.util.concurrent.atomic.AtomicReference;
54

65
import javax.net.ssl.HostnameVerifier;
76
import javax.net.ssl.SSLParameters;
@@ -24,7 +23,7 @@ public class JedisFactory implements PooledObjectFactory<Jedis> {
2423

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

27-
private final AtomicReference<HostAndPort> hostAndPort = new AtomicReference<>();
26+
private final JedisSocketFactory jedisSocketFactory;
2827

2928
private final JedisClientConfig clientConfig;
3029

@@ -66,20 +65,25 @@ protected JedisFactory(final String host, final int port, final int connectionTi
6665
}
6766

6867
protected JedisFactory(final HostAndPort hostAndPort, final JedisClientConfig clientConfig) {
69-
this.hostAndPort.set(hostAndPort);
7068
this.clientConfig = DefaultJedisClientConfig.copyConfig(clientConfig);
69+
this.jedisSocketFactory = new DefaultJedisSocketFactory(hostAndPort, this.clientConfig);
7170
}
7271

7372
protected JedisFactory(final String host, final int port, final int connectionTimeout, final int soTimeout,
7473
final int infiniteSoTimeout, final String user, final String password, final int database,
7574
final String clientName, final boolean ssl, final SSLSocketFactory sslSocketFactory,
7675
final SSLParameters sslParameters, final HostnameVerifier hostnameVerifier) {
77-
this.hostAndPort.set(new HostAndPort(host, port));
7876
this.clientConfig = DefaultJedisClientConfig.builder().connectionTimeoutMillis(connectionTimeout)
7977
.socketTimeoutMillis(soTimeout).blockingSocketTimeoutMillis(infiniteSoTimeout).user(user)
8078
.password(password).database(database).clientName(clientName)
8179
.ssl(ssl).sslSocketFactory(sslSocketFactory)
8280
.sslParameters(sslParameters).hostnameVerifier(hostnameVerifier).build();
81+
this.jedisSocketFactory = new DefaultJedisSocketFactory(new HostAndPort(host, port), this.clientConfig);
82+
}
83+
84+
protected JedisFactory(final JedisSocketFactory jedisSocketFactory, final JedisClientConfig clientConfig) {
85+
this.clientConfig = DefaultJedisClientConfig.copyConfig(clientConfig);
86+
this.jedisSocketFactory = jedisSocketFactory;
8387
}
8488

8589
/**
@@ -100,6 +104,7 @@ protected JedisFactory(final int connectionTimeout, final int soTimeout, final i
100104
*/
101105
protected JedisFactory(final JedisClientConfig clientConfig) {
102106
this.clientConfig = clientConfig;
107+
this.jedisSocketFactory = new DefaultJedisSocketFactory(clientConfig);
103108
}
104109

105110
protected JedisFactory(final URI uri, final int connectionTimeout, final int soTimeout,
@@ -120,17 +125,17 @@ protected JedisFactory(final URI uri, final int connectionTimeout, final int soT
120125
throw new InvalidURIException(String.format(
121126
"Cannot open Redis connection due invalid URI. %s", uri.toString()));
122127
}
123-
this.hostAndPort.set(new HostAndPort(uri.getHost(), uri.getPort()));
124128
this.clientConfig = DefaultJedisClientConfig.builder().connectionTimeoutMillis(connectionTimeout)
125129
.socketTimeoutMillis(soTimeout).blockingSocketTimeoutMillis(infiniteSoTimeout)
126130
.user(JedisURIHelper.getUser(uri)).password(JedisURIHelper.getPassword(uri))
127131
.database(JedisURIHelper.getDBIndex(uri)).clientName(clientName)
128132
.ssl(JedisURIHelper.isRedisSSLScheme(uri)).sslSocketFactory(sslSocketFactory)
129133
.sslParameters(sslParameters).hostnameVerifier(hostnameVerifier).build();
134+
this.jedisSocketFactory = new DefaultJedisSocketFactory(new HostAndPort(uri.getHost(), uri.getPort()), this.clientConfig);
130135
}
131136

132137
public void setHostAndPort(final HostAndPort hostAndPort) {
133-
this.hostAndPort.set(hostAndPort);
138+
jedisSocketFactory.updateHostAndPort(hostAndPort);
134139
}
135140

136141
public void setPassword(final String password) {
@@ -167,10 +172,9 @@ public void destroyObject(PooledObject<Jedis> pooledJedis) throws Exception {
167172

168173
@Override
169174
public PooledObject<Jedis> makeObject() throws Exception {
170-
final HostAndPort hostPort = this.hostAndPort.get();
171175
Jedis jedis = null;
172176
try {
173-
jedis = new Jedis(hostPort, clientConfig);
177+
jedis = new Jedis(jedisSocketFactory, clientConfig);
174178
jedis.connect();
175179
return new DefaultPooledObject<>(jedis);
176180
} catch (JedisException je) {
@@ -199,13 +203,14 @@ public void passivateObject(PooledObject<Jedis> pooledJedis) throws Exception {
199203
public boolean validateObject(PooledObject<Jedis> pooledJedis) {
200204
final BinaryJedis jedis = pooledJedis.getObject();
201205
try {
202-
HostAndPort hostAndPort = this.hostAndPort.get();
206+
String host = jedisSocketFactory.getHost();
207+
int port = jedisSocketFactory.getPort();
203208

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

207-
return hostAndPort.getHost().equals(connectionHost)
208-
&& hostAndPort.getPort() == connectionPort && jedis.isConnected()
212+
return host.equals(connectionHost)
213+
&& port == connectionPort && jedis.isConnected()
209214
&& jedis.ping().equals("PONG");
210215
} catch (final Exception e) {
211216
return false;

src/main/java/redis/clients/jedis/JedisPool.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,11 @@ public JedisPool(final GenericObjectPoolConfig<Jedis> poolConfig, final HostAndP
261261
super(poolConfig, new JedisFactory(hostAndPort, clientConfig));
262262
}
263263

264+
public JedisPool(final GenericObjectPoolConfig<Jedis> poolConfig, final JedisSocketFactory jedisSocketFactory,
265+
final JedisClientConfig clientConfig) {
266+
super(poolConfig, new JedisFactory(jedisSocketFactory, clientConfig));
267+
}
268+
264269
public JedisPool(final GenericObjectPoolConfig<Jedis> poolConfig) {
265270
this(poolConfig, Protocol.DEFAULT_HOST, Protocol.DEFAULT_PORT);
266271
}

src/main/java/redis/clients/jedis/JedisSocketFactory.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ public interface JedisSocketFactory {
2424
*/
2525
Socket createSocket() throws IOException, JedisConnectionException;
2626

27+
void updateHostAndPort(HostAndPort hostAndPort);
28+
2729
@Deprecated
2830
String getDescription();
2931

src/test/java/redis/clients/jedis/tests/UdsTest.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import org.newsclub.net.unix.AFUNIXSocket;
88
import org.newsclub.net.unix.AFUNIXSocketAddress;
99

10+
import redis.clients.jedis.HostAndPort;
1011
import redis.clients.jedis.Jedis;
1112
import redis.clients.jedis.JedisSocketFactory;
1213
import redis.clients.jedis.Protocol;
@@ -38,6 +39,11 @@ public Socket createSocket() throws JedisConnectionException {
3839
}
3940
}
4041

42+
@Override
43+
public void updateHostAndPort(HostAndPort hostAndPort) {
44+
throw new UnsupportedOperationException("UDS cannot update host and port");
45+
}
46+
4147
@Override
4248
public String getDescription() {
4349
return UDS_SOCKET.toString();

0 commit comments

Comments
 (0)