Skip to content

Commit

Permalink
feat(Pool): autovacuum fix + refactor (eclipse-edc#3479)
Browse files Browse the repository at this point in the history 
* feat(Pool): disable test connection on return, adds rollback if it's not autocommit, config refactor

* docs: adds new pool configuration values in README.md

* feat: removes types with var
  • Loading branch information
@wolf4ood
wolf4ood authored Sep 28, 2023
1 parent bbfcbab commit 101203a
Show file tree
Hide file tree
Showing 10 changed files with 494 additions and 422 deletions.
41 changes: 28 additions & 13 deletions extensions/common/sql/sql-pool/sql-pool-apache-commons/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,32 @@ the `org.eclipse.edc.transaction.datasource.spi.DataSourceRegistry`
capable of pooling `java.sql.Connection`s. The pooling mechanism is backed by
the [Apache Commons Pool library](https://commons.apache.org/proper/commons-pool/).

## Configuration
## Old Configuration (Deprecated since 0.3.1)

| Key | Description | Mandatory |
|:---|:---|---|
| edc.datasource.<datasource_name>.url | JDBC driver url | X |
| edc.datasource.<datasource_name>.pool.maxIdleConnections | The maximum amount of idling connections maintained by the pool | |
| edc.datasource.<datasource_name>.pool.maxTotalConnections | The maximum amount of total connections maintained by the pool | |
| edc.datasource.<datasource_name>.pool.minIdleConnections | The minimum amount of idling connections maintained by the pool | |
| edc.datasource.<datasource_name>.pool.testConnectionOnBorrow | Flag to define whether connections will be validated when a connection has been obtained from the pool | |
| edc.datasource.<datasource_name>.pool.testConnectionOnCreate | Flag to define whether connections will be validated when a connection has been established | |
| edc.datasource.<datasource_name>.pool.testConnectionOnReturn | Flag to define whether connections will be validated when a connection has been returned to the pool | |
| edc.datasource.<datasource_name>.pool.testConnectionWhileIdle | Flag to define whether idling connections will be validated | |
| edc.datasource.<datasource_name>.pool.testQuery | Test query to validate a connection maintained by the pool | |
| edc.datasource.<datasource_name>.<jdbc_properties> | JDBC driver specific configuration properties | |
| Key | Description | Mandatory |
|:--------------------------------------------------------------|:-------------------------------------------------------------------------------------------------------|-----------|
| edc.datasource.<datasource_name>.url | JDBC driver url | X |
| edc.datasource.<datasource_name>.pool.maxIdleConnections | The maximum amount of idling connections maintained by the pool | |
| edc.datasource.<datasource_name>.pool.maxTotalConnections | The maximum amount of total connections maintained by the pool | |
| edc.datasource.<datasource_name>.pool.minIdleConnections | The minimum amount of idling connections maintained by the pool | |
| edc.datasource.<datasource_name>.pool.testConnectionOnBorrow | Flag to define whether connections will be validated when a connection has been obtained from the pool | |
| edc.datasource.<datasource_name>.pool.testConnectionOnCreate | Flag to define whether connections will be validated when a connection has been established | |
| edc.datasource.<datasource_name>.pool.testConnectionOnReturn | Flag to define whether connections will be validated when a connection has been returned to the pool | |
| edc.datasource.<datasource_name>.pool.testConnectionWhileIdle | Flag to define whether idling connections will be validated | |
| edc.datasource.<datasource_name>.pool.testQuery | Test query to validate a connection maintained by the pool | |
| edc.datasource.<datasource_name>.<jdbc_properties> | JDBC driver specific configuration properties | |

## New Configuration (since 0.3.1)

| Key | Description | Mandatory |
|:-----------------------------------------------------------------|:-------------------------------------------------------------------------------------------------------|-----------|
| edc.datasource.<datasource_name>.url | JDBC driver url | X |
| edc.datasource.<datasource_name>.pool.connections.max-idle | The maximum amount of idling connections maintained by the pool | |
| edc.datasource.<datasource_name>.pool.connections.max-total | The maximum amount of total connections maintained by the pool | |
| edc.datasource.<datasource_name>.pool.connections.min-idle | The minimum amount of idling connections maintained by the pool | |
| edc.datasource.<datasource_name>.pool.connection.test.on-borrow | Flag to define whether connections will be validated when a connection has been obtained from the pool | |
| edc.datasource.<datasource_name>.pool.connection.test.on-create | Flag to define whether connections will be validated when a connection has been established | |
| edc.datasource.<datasource_name>.pool.connection.test.on-return | Flag to define whether connections will be validated when a connection has been returned to the pool | |
| edc.datasource.<datasource_name>.pool.connection.test.while-idle | Flag to define whether idling connections will be validated | |
| edc.datasource.<datasource_name>.pool.connection.test.query | Test query to validate a connection maintained by the pool | |
| edc.datasource.<datasource_name>.<jdbc_properties> | JDBC driver specific configuration properties | |
60 changes: 42 additions & 18 deletions ...-apache-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPool.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.apache.commons.pool2.impl.DefaultPooledObject;
import org.apache.commons.pool2.impl.GenericObjectPool;
import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
import org.eclipse.edc.spi.monitor.Monitor;
import org.eclipse.edc.spi.persistence.EdcPersistenceException;
import org.eclipse.edc.sql.pool.ConnectionPool;
import org.jetbrains.annotations.NotNull;
Expand All @@ -32,13 +33,15 @@

public final class CommonsConnectionPool implements ConnectionPool, AutoCloseable {
private final GenericObjectPool<Connection> connectionObjectPool;
private final CommonsConnectionPoolConfig poolConfig;

public CommonsConnectionPool(DataSource dataSource, CommonsConnectionPoolConfig commonsConnectionPoolConfig) {
public CommonsConnectionPool(DataSource dataSource, CommonsConnectionPoolConfig commonsConnectionPoolConfig, Monitor monitor) {
this.poolConfig = commonsConnectionPoolConfig;
Objects.requireNonNull(dataSource, "connectionFactory");
Objects.requireNonNull(commonsConnectionPoolConfig, "commonsConnectionPoolConfig");

this.connectionObjectPool = new GenericObjectPool<>(
new PooledConnectionObjectFactory(dataSource, commonsConnectionPoolConfig.getTestQuery()),
new PooledConnectionObjectFactory(dataSource, commonsConnectionPoolConfig.getTestQuery(), monitor),
getGenericObjectPoolConfig(commonsConnectionPoolConfig));
}

Expand Down Expand Up @@ -81,13 +84,20 @@ public void close() {
connectionObjectPool.close();
}

public CommonsConnectionPoolConfig getPoolConfig() {
return poolConfig;
}

private static class PooledConnectionObjectFactory extends BasePooledObjectFactory<Connection> {
private final String testQuery;
private final DataSource dataSource;

PooledConnectionObjectFactory(@NotNull DataSource dataSource, @NotNull String testQuery) {
private final Monitor monitor;

PooledConnectionObjectFactory(@NotNull DataSource dataSource, @NotNull String testQuery, Monitor monitor) {
this.dataSource = Objects.requireNonNull(dataSource);
this.testQuery = Objects.requireNonNull(testQuery);
this.monitor = monitor;
}

@Override
Expand All @@ -96,32 +106,37 @@ public Connection create() throws SQLException {
}

@Override
public void destroyObject(PooledObject<Connection> pooledObject, DestroyMode destroyMode) throws Exception {
public boolean validateObject(PooledObject<Connection> pooledObject) {
if (pooledObject == null) {
return;
return false;
}

Connection connection = pooledObject.getObject();

if (connection != null && !connection.isClosed()) {
connection.close();
if (connection == null) {
return false;
}

pooledObject.invalidate();
return isConnectionValid(connection);
}

@Override
public boolean validateObject(PooledObject<Connection> pooledObject) {
public PooledObject<Connection> wrap(Connection connection) {
return new DefaultPooledObject<>(connection);
}

@Override
public void destroyObject(PooledObject<Connection> pooledObject, DestroyMode destroyMode) throws Exception {
if (pooledObject == null) {
return false;
return;
}

Connection connection = pooledObject.getObject();
if (connection == null) {
return false;

if (connection != null && !connection.isClosed()) {
connection.close();
}

return isConnectionValid(connection);
pooledObject.invalidate();
}

private boolean isConnectionValid(Connection connection) {
Expand All @@ -131,16 +146,25 @@ private boolean isConnectionValid(Connection connection) {
}

try (PreparedStatement preparedStatement = connection.prepareStatement(testQuery)) {
return preparedStatement.execute();
preparedStatement.execute();
return rollbackIfNeeded(connection);
}
} catch (Exception e) { // any exception thrown indicates invalidity of the connection
return false;
}
}

@Override
public PooledObject<Connection> wrap(Connection connection) {
return new DefaultPooledObject<>(connection);
private boolean rollbackIfNeeded(Connection connection) {
try {
if (!connection.getAutoCommit()) {
connection.rollback();
}
return true;
} catch (SQLException e) {
monitor.debug("Failed to rollback transaction", e);
}
return false;
}

}
}
2 changes: 1 addition & 1 deletion ...e-commons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public static final class Builder {
private int minIdleConnections = 1;
private boolean testConnectionOnBorrow = true;
private boolean testConnectionOnCreate = true;
private boolean testConnectionOnReturn = true;
private boolean testConnectionOnReturn = false;
private boolean testConnectionWhileIdle = false;
private String testQuery = "SELECT 1;";

Expand Down
49 changes: 41 additions & 8 deletions ...mmons/src/main/java/org/eclipse/edc/sql/pool/commons/CommonsConnectionPoolConfigKeys.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,32 +16,65 @@

import org.eclipse.edc.runtime.metamodel.annotation.Setting;

import java.util.Map;

interface CommonsConnectionPoolConfigKeys {

@Deprecated(since = "0.3.1")
@Setting(required = false)
String POOL_MAX_IDLE_CONNECTIONS = "pool.maxIdleConnections";
String DEPRACATED_POOL_MAX_IDLE_CONNECTIONS = "pool.maxIdleConnections";
String POOL_CONNECTIONS_MAX_IDLE = "pool.connections.max-idle";

@Deprecated(since = "0.3.1")
@Setting(required = false)
String POOL_MAX_TOTAL_CONNECTIONS = "pool.maxTotalConnections";
String DEPRACATED_POOL_MAX_TOTAL_CONNECTIONS = "pool.maxTotalConnections";

String POOL_CONNECTIONS_MAX_TOTAL = "pool.connections.max-total";

@Deprecated(since = "0.3.1")
@Setting(required = false)
String POOL_MIN_IDLE_CONNECTIONS = "pool.minIdleConnections";
String DEPRACATED_POOL_MIN_IDLE_CONNECTIONS = "pool.minIdleConnections";
String POOL_CONNECTIONS_MIN_IDLE = "pool.connections.min-idle";

@Deprecated(since = "0.3.1")
@Setting(required = false)
String POOL_TEST_CONNECTION_ON_BORROW = "pool.testConnectionOnBorrow";
String DEPRACATED_POOL_TEST_CONNECTION_ON_BORROW = "pool.testConnectionOnBorrow";
String POOL_CONNECTION_TEST_ON_BORROW = "pool.connection.test.on-borrow";

@Deprecated(since = "0.3.1")
@Setting(required = false)
String POOL_TEST_CONNECTION_ON_CREATE = "pool.testConnectionOnCreate";
String DEPRACATED_POOL_TEST_CONNECTION_ON_CREATE = "pool.testConnectionOnCreate";

String POOL_CONNECTION_TEST_ON_CREATE = "pool.connection.test.on-create";

@Deprecated(since = "0.3.1")
@Setting(required = false)
String POOL_TEST_CONNECTION_ON_RETURN = "pool.testConnectionOnReturn";
String DEPRACATED_POOL_TEST_CONNECTION_ON_RETURN = "pool.testConnectionOnReturn";

String POOL_CONNECTION_TEST_ON_RETURN = "pool.connection.test.on-return";

@Deprecated(since = "0.3.1")
@Setting(required = false)
String POOL_TEST_CONNECTION_WHILE_IDLE = "pool.testConnectionWhileIdle";
String DEPRACATED_POOL_TEST_CONNECTION_WHILE_IDLE = "pool.testConnectionWhileIdle";
String POOL_CONNECTION_TEST_WHILE_IDLE = "pool.connection.test.while-idle";

@Deprecated(since = "0.3.1")
@Setting(required = false)
String POOL_TEST_QUERY = "pool.testQuery";
String DEPRACATED_POOL_TEST_QUERY = "pool.testQuery";

String POOL_CONNECTION_TEST_QUERY = "pool.connection.test.query";

@Setting(required = true)
String URL = "url";

Map<String, String> CONFIGURATION_MAPPING = Map.of(
POOL_CONNECTIONS_MAX_IDLE, DEPRACATED_POOL_MAX_IDLE_CONNECTIONS,
POOL_CONNECTIONS_MIN_IDLE, DEPRACATED_POOL_MIN_IDLE_CONNECTIONS,
POOL_CONNECTIONS_MAX_TOTAL, DEPRACATED_POOL_MAX_TOTAL_CONNECTIONS,
POOL_CONNECTION_TEST_ON_BORROW, DEPRACATED_POOL_TEST_CONNECTION_ON_BORROW,
POOL_CONNECTION_TEST_ON_CREATE, DEPRACATED_POOL_TEST_CONNECTION_ON_CREATE,
POOL_CONNECTION_TEST_ON_RETURN, DEPRACATED_POOL_TEST_CONNECTION_ON_RETURN,
POOL_CONNECTION_TEST_WHILE_IDLE, DEPRACATED_POOL_TEST_CONNECTION_WHILE_IDLE,
POOL_CONNECTION_TEST_QUERY, DEPRACATED_POOL_TEST_QUERY);

}
Loading

0 comments on commit 101203a

Please sign in to comment.