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

Pool extends GenericObjectPool #2521

Merged
merged 5 commits into from
Jul 2, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion src/main/java/redis/clients/jedis/JedisPool.java
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ public void returnResource(final Jedis resource) {
if (resource != null) {
try {
resource.resetState();
returnResourceObject(resource);
super.returnResource(resource);
} catch (Exception e) {
returnBrokenResource(resource);
log.warn("Resource is returned to the pool as broken", e);
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/redis/clients/jedis/JedisSentinelPool.java
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ private void initMaster(HostAndPort master) {
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
clearInternalPool();
super.clear();

LOG.info("Created JedisSentinelPool to master at {}", master);
}
Expand Down Expand Up @@ -301,7 +301,7 @@ public void returnResource(final Jedis resource) {
if (resource != null) {
try {
resource.resetState();
returnResourceObject(resource);
super.returnResource(resource);
} catch (Exception e) {
returnBrokenResource(resource);
LOG.debug("Resource is returned to the pool as broken", e);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/redis/clients/jedis/ShardedJedisPool.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public ShardedJedis getResource() {
public void returnResource(final ShardedJedis resource) {
if (resource != null) {
resource.resetState();
returnResourceObject(resource);
super.returnResource(resource);
}
}

Expand Down
132 changes: 15 additions & 117 deletions src/main/java/redis/clients/jedis/util/Pool.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,37 +12,20 @@
import redis.clients.jedis.exceptions.JedisException;
import redis.clients.jedis.exceptions.JedisExhaustedPoolException;

public abstract class Pool<T> implements Closeable {

private final GenericObjectPool<T> internalPool;
public class Pool<T> extends GenericObjectPool<T> implements Closeable {

public Pool(final GenericObjectPoolConfig<T> poolConfig, PooledObjectFactory<T> factory) {
this.internalPool = new GenericObjectPool<>(factory, poolConfig);
super(factory, poolConfig);
}

@Override
public void close() {
destroy();
}

public boolean isClosed() {
return this.internalPool.isClosed();
}

/**
* This call only clears idle instances, not borrowed instances.
*/
protected void clearInternalPool() {
try {
this.internalPool.clear();
} catch (Exception e) {
throw new JedisException("Could not clear the pool", e);
}
}

public T getResource() {
try {
return internalPool.borrowObject();
return super.borrowObject();
} catch (JedisDataException jde) {
throw jde;
} catch (NoSuchElementException nse) {
Expand All @@ -57,126 +40,41 @@ public T getResource() {
}
}

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

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

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

public void destroy() {
closeInternalPool();
}

protected void returnBrokenResourceObject(final T resource) {
try {
internalPool.invalidateObject(resource);
super.invalidateObject(resource);
} catch (Exception e) {
throw new JedisException("Could not return the broken resource to the pool", e);
}
}

protected void closeInternalPool() {
public void destroy() {
try {
internalPool.close();
super.close();
} catch (Exception e) {
throw new JedisException("Could not destroy the pool", e);
}
}

/**
* Returns the number of instances currently borrowed from this pool.
*
* @return The number of instances currently borrowed from this pool, -1 if
* the pool is inactive.
*/
public int getNumActive() {
if (poolInactive()) {
return -1;
}

return this.internalPool.getNumActive();
}

/**
* Returns the number of instances currently idle in this pool.
*
* @return The number of instances currently idle in this pool, -1 if the
* pool is inactive.
*/
public int getNumIdle() {
if (poolInactive()) {
return -1;
}

return this.internalPool.getNumIdle();
}

/**
* Returns an estimate of the number of threads currently blocked waiting for
* a resource from this pool.
*
* @return The number of threads waiting, -1 if the pool is inactive.
*/
public int getNumWaiters() {
if (poolInactive()) {
return -1;
}

return this.internalPool.getNumWaiters();
}

/**
* Returns the mean waiting time spent by threads to obtain a resource from
* this pool.
*
* @return The mean waiting time, in milliseconds, -1 if the pool is
* inactive.
*/
public long getMeanBorrowWaitTimeMillis() {
if (poolInactive()) {
return -1;
}

return this.internalPool.getMeanBorrowWaitTimeMillis();
}

/**
* Returns the maximum waiting time spent by threads to obtain a resource
* from this pool.
*
* @return The maximum waiting time, in milliseconds, -1 if the pool is
* inactive.
*/
public long getMaxBorrowWaitTimeMillis() {
if (poolInactive()) {
return -1;
}

return this.internalPool.getMaxBorrowWaitTimeMillis();
}

private boolean poolInactive() {
return this.internalPool == null || this.internalPool.isClosed();
}

@Override
public void addObjects(int count) {
try {
for (int i = 0; i < count; i++) {
this.internalPool.addObject();
addObject();
}
} catch (Exception e) {
throw new JedisException("Error trying to add idle objects", e);
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/redis/clients/jedis/tests/JedisPoolTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ public void returnResourceShouldResetState() {
}

@Test
public void getNumActiveIsNegativeWhenPoolIsClosed() {
public void getNumActiveWhenPoolIsClosed() {
JedisPool pool = new JedisPool(new JedisPoolConfig(), hnp.getHost(), hnp.getPort(), 2000,
"foobared", 0, "my_shiny_client_name");

Expand All @@ -301,7 +301,7 @@ public void getNumActiveIsNegativeWhenPoolIsClosed() {
}

pool.close();
assertTrue(pool.getNumActive() < 0);
assertEquals(0, pool.getNumActive());
}

@Test
Expand Down