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

Deprecate JedisPoolAbstract #2361

Merged
merged 7 commits into from
Feb 7, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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
5 changes: 3 additions & 2 deletions src/main/java/redis/clients/jedis/Jedis.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,14 @@
import redis.clients.jedis.params.ZAddParams;
import redis.clients.jedis.params.ZIncrByParams;
import redis.clients.jedis.params.LPosParams;
import redis.clients.jedis.util.Pool;
import redis.clients.jedis.util.SafeEncoder;
import redis.clients.jedis.util.Slowlog;

public class Jedis extends BinaryJedis implements JedisCommands, MultiKeyCommands,
AdvancedJedisCommands, ScriptingCommands, BasicCommands, ClusterCommands, SentinelCommands, ModuleCommands {

protected JedisPoolAbstract dataSource = null;
private Pool<Jedis> dataSource = null;

public Jedis() {
super();
Expand Down Expand Up @@ -3603,7 +3604,7 @@ public Map<String, String> pubsubNumSub(String... channels) {
@Override
public void close() {
if (dataSource != null) {
JedisPoolAbstract pool = this.dataSource;
Pool<Jedis> pool = this.dataSource;
this.dataSource = null;
if (client.isBroken()) {
pool.returnBrokenResource(this);
Expand Down
26 changes: 11 additions & 15 deletions src/main/java/redis/clients/jedis/JedisPool.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import javax.net.ssl.SSLParameters;
import javax.net.ssl.SSLSocketFactory;

import org.apache.commons.pool2.impl.GenericObjectPool;
import org.apache.commons.pool2.impl.GenericObjectPoolConfig;

import redis.clients.jedis.exceptions.JedisException;
Expand All @@ -29,26 +28,24 @@ public JedisPool(String host, int port) {
public JedisPool(final String host) {
URI uri = URI.create(host);
if (JedisURIHelper.isValid(uri)) {
this.internalPool = new GenericObjectPool<>(new JedisFactory(uri,
Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null), new GenericObjectPoolConfig());
initPool(new GenericObjectPoolConfig(), new JedisFactory(uri, Protocol.DEFAULT_TIMEOUT,
Protocol.DEFAULT_TIMEOUT, null));
} else {
this.internalPool = new GenericObjectPool<>(new JedisFactory(host,
Protocol.DEFAULT_PORT, Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null,
Protocol.DEFAULT_DATABASE, null), new GenericObjectPoolConfig());
initPool(new GenericObjectPoolConfig(), new JedisFactory(host, Protocol.DEFAULT_PORT,
Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null, Protocol.DEFAULT_DATABASE, null));
}
}

public JedisPool(final String host, final SSLSocketFactory sslSocketFactory,
final SSLParameters sslParameters, final HostnameVerifier hostnameVerifier) {
URI uri = URI.create(host);
if (JedisURIHelper.isValid(uri)) {
this.internalPool = new GenericObjectPool<>(new JedisFactory(uri,
Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null, sslSocketFactory, sslParameters,
hostnameVerifier), new GenericObjectPoolConfig());
initPool(new GenericObjectPoolConfig(), new JedisFactory(uri, Protocol.DEFAULT_TIMEOUT,
Protocol.DEFAULT_TIMEOUT, null, sslSocketFactory, sslParameters, hostnameVerifier));
} else {
this.internalPool = new GenericObjectPool<>(new JedisFactory(host,
Protocol.DEFAULT_PORT, Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null,
Protocol.DEFAULT_DATABASE, null, false, null, null, null), new GenericObjectPoolConfig());
initPool(new GenericObjectPoolConfig(), new JedisFactory(host, Protocol.DEFAULT_PORT,
Protocol.DEFAULT_TIMEOUT, Protocol.DEFAULT_TIMEOUT, null, Protocol.DEFAULT_DATABASE, null,
false, null, null, null));
}
}

Expand Down Expand Up @@ -333,21 +330,20 @@ public Jedis getResource() {
}

@Override
protected void returnBrokenResource(final Jedis resource) {
public void returnBrokenResource(final Jedis resource) {
if (resource != null) {
returnBrokenResourceObject(resource);
}
}

@Override
protected void returnResource(final Jedis resource) {
public void returnResource(final Jedis resource) {
if (resource != null) {
try {
resource.resetState();
returnResourceObject(resource);
} catch (Exception e) {
returnBrokenResource(resource);
throw new JedisException("Resource is returned to the pool as broken", e);
sazzad16 marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down
15 changes: 5 additions & 10 deletions src/main/java/redis/clients/jedis/JedisPoolAbstract.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@

import redis.clients.jedis.util.Pool;

/**
* @deprecated This class will be removed in future. If you are directly manipulating this class,
* you are suggested to change your code to use {@link Pool Pool&lt;Jedis&gt;} instead.
*/
@Deprecated
public class JedisPoolAbstract extends Pool<Jedis> {

public JedisPoolAbstract() {
Expand All @@ -14,14 +19,4 @@ public JedisPoolAbstract() {
public JedisPoolAbstract(GenericObjectPoolConfig poolConfig, PooledObjectFactory<Jedis> factory) {
super(poolConfig, factory);
}

@Override
protected void returnBrokenResource(Jedis resource) {
super.returnBrokenResource(resource);
}

@Override
protected void returnResource(Jedis resource) {
super.returnResource(resource);
}
}
12 changes: 5 additions & 7 deletions src/main/java/redis/clients/jedis/JedisSentinelPool.java
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,9 @@ private void initPool(HostAndPort master) {
initPool(poolConfig, factory);
} else {
factory.setHostAndPort(currentHostMaster);
// although we clear the pool, we still have to check the
// returned object
// in getResource, this call only clears idle instances, not
// borrowed instances
internalPool.clear();
// although we clear the pool, we still have to check the returned object in getResource,
// this call only clears idle instances, not borrowed instances
clearInternalPool();
}

log.info("Created JedisPool to master at {}", master);
Expand Down Expand Up @@ -312,14 +310,14 @@ public Jedis getResource() {
}

@Override
protected void returnBrokenResource(final Jedis resource) {
public void returnBrokenResource(final Jedis resource) {
if (resource != null) {
returnBrokenResourceObject(resource);
}
}

@Override
protected void returnResource(final Jedis resource) {
public void returnResource(final Jedis resource) {
if (resource != null) {
try {
resource.resetState();
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/redis/clients/jedis/ShardedJedisPool.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@ public ShardedJedis getResource() {
}

@Override
protected void returnBrokenResource(final ShardedJedis resource) {
public void returnBrokenResource(final ShardedJedis resource) {
if (resource != null) {
returnBrokenResourceObject(resource);
}
}

@Override
protected void returnResource(final ShardedJedis resource) {
public void returnResource(final ShardedJedis resource) {
if (resource != null) {
resource.resetState();
returnResourceObject(resource);
Expand Down
18 changes: 13 additions & 5 deletions src/main/java/redis/clients/jedis/util/Pool.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
import redis.clients.jedis.exceptions.JedisExhaustedPoolException;

public abstract class Pool<T> implements Closeable {

/**
* @deprecated This will be private in future;
*/
@Deprecated
protected GenericObjectPool<T> internalPool;

/**
Expand Down Expand Up @@ -45,6 +50,12 @@ public void initPool(final GenericObjectPoolConfig poolConfig, PooledObjectFacto
this.internalPool = new GenericObjectPool<>(factory, poolConfig);
}

protected void clearInternalPool() {
if (internalPool != null) {
internalPool.clear();
}
}

public T getResource() {
try {
return internalPool.borrowObject();
Expand All @@ -61,23 +72,20 @@ public T getResource() {
}

protected void returnResourceObject(final T resource) {
if (resource == null) {
return;
}
try {
internalPool.returnObject(resource);
} catch (Exception e) {
throw new JedisException("Could not return the resource to the pool", e);
}
}

protected void returnBrokenResource(final T resource) {
public void returnBrokenResource(final T resource) {
if (resource != null) {
returnBrokenResourceObject(resource);
}
}

protected void returnResource(final T resource) {
public void returnResource(final T resource) {
if (resource != null) {
returnResourceObject(resource);
}
Expand Down