This is an automated email from the ASF dual-hosted git repository.
wlo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/gobblin.git
The following commit(s) were added to refs/heads/master by this push:
new 72bf8d5d1 [GOBBLIN-1753] Migrate DB connection pool from
o.a.commons.dbcp/dbcp2 to HikariCP (#3613)
72bf8d5d1 is described below
commit 72bf8d5d10ad77d3c54cc88202e05890231f9da4
Author: Kip Kohn <[email protected]>
AuthorDate: Tue Dec 6 09:14:34 2022 -0800
[GOBBLIN-1753] Migrate DB connection pool from o.a.commons.dbcp/dbcp2 to
HikariCP (#3613)
* [GOBBLIN-1753] Migrate DB connection pool from o.a.commons.dbcp/dbcp2 to
HikariCP
* minor change to use `Duration`
---
.../gobblin/configuration/ConfigurationKeys.java | 4 ---
gobblin-core/build.gradle | 2 +-
.../CleanableMysqlDatasetStoreDatasetTest.java | 16 ++++-----
.../retention/sql/SqlBasedRetentionPoc.java | 15 ++++----
gobblin-metastore/build.gradle | 2 +-
.../metastore/JobHistoryDataSourceProvider.java | 22 ++++++++----
.../metastore/MysqlDagStateStoreFactory.java | 6 ++--
.../gobblin/metastore/MysqlDataSourceFactory.java | 19 +++++-----
.../gobblin/metastore/MysqlDataSourceKey.java | 4 +--
.../metastore/MysqlJobStatusStateStoreFactory.java | 6 ++--
.../apache/gobblin/metastore/MysqlStateStore.java | 40 ++++++++++++----------
.../gobblin/metastore/MysqlStateStoreFactory.java | 6 ++--
.../metastore/MysqlDataSourceFactoryTest.java | 20 +++++------
.../AvroToJdbcEntryConverterInitializer.java | 2 +-
.../apache/gobblin/publisher/JdbcPublisher.java | 2 +-
.../apache/gobblin/source/jdbc/JdbcProvider.java | 22 +++++++-----
.../java/org/apache/gobblin/writer/JdbcWriter.java | 2 +-
.../writer/initializer/JdbcWriterInitializer.java | 2 +-
.../runtime/MysqlDatasetStateStoreFactory.java | 6 ++--
.../runtime/spec_store/MysqlBaseSpecStore.java | 12 ++++---
.../runtime/MysqlDatasetStateStoreTest.java | 16 ++++-----
.../gobblin/runtime/cli/JobStateStoreCliTest.java | 18 +++++-----
gobblin-service/build.gradle | 2 +-
.../modules/db/ServiceDatabaseProviderImpl.java | 30 ++++++++--------
.../orchestration/MysqlUserQuotaManager.java | 24 ++++++++-----
.../orchestration/MysqlDagStateStoreTest.java | 16 ++++-----
gobblin-utility/build.gradle | 2 +-
.../gobblin/util/jdbc/DataSourceBuilder.java | 12 -------
.../gobblin/util/jdbc/DataSourceProvider.java | 39 +++++++++++----------
gradle/scripts/dependencyDefinitions.gradle | 3 +-
30 files changed, 195 insertions(+), 177 deletions(-)
diff --git
a/gobblin-api/src/main/java/org/apache/gobblin/configuration/ConfigurationKeys.java
b/gobblin-api/src/main/java/org/apache/gobblin/configuration/ConfigurationKeys.java
index cb5f664e8..4344f3575 100644
---
a/gobblin-api/src/main/java/org/apache/gobblin/configuration/ConfigurationKeys.java
+++
b/gobblin-api/src/main/java/org/apache/gobblin/configuration/ConfigurationKeys.java
@@ -75,10 +75,6 @@ public class ConfigurationKeys {
// DB state store configuration
public static final String STATE_STORE_DB_JDBC_DRIVER_KEY =
"state.store.db.jdbc.driver";
public static final String DEFAULT_STATE_STORE_DB_JDBC_DRIVER =
"com.mysql.jdbc.Driver";
- // min idle time for eviction
- public static final String STATE_STORE_DB_CONN_MIN_EVICTABLE_IDLE_TIME_KEY =
- "state.store.db.conn.min.evictable.idle.time";
- public static final long DEFAULT_STATE_STORE_DB_CONN_MIN_EVICTABLE_IDLE_TIME
= 5 * 60 * 1000;
public static final String STATE_STORE_DB_URL_KEY = "state.store.db.url";
public static final String STATE_STORE_DB_USER_KEY = "state.store.db.user";
public static final String STATE_STORE_DB_PASSWORD_KEY =
"state.store.db.password";
diff --git a/gobblin-core/build.gradle b/gobblin-core/build.gradle
index 9ca24678d..7daa1a9b3 100644
--- a/gobblin-core/build.gradle
+++ b/gobblin-core/build.gradle
@@ -32,7 +32,6 @@ dependencies {
compile externalDependency.avroMapredH2
compile externalDependency.commonsCodec
- compile externalDependency.commonsDbcp
compile externalDependency.commonsMath
compile externalDependency.commonsHttpClient
compile externalDependency.avro
@@ -46,6 +45,7 @@ dependencies {
compile externalDependency.jsch
compile externalDependency.commonsLang3
compile externalDependency.commonsIo
+ compile externalDependency.hikariCP
compile externalDependency.hiveExec
compile externalDependency.hiveSerDe
compile externalDependency.httpclient
diff --git
a/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/retention/CleanableMysqlDatasetStoreDatasetTest.java
b/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/retention/CleanableMysqlDatasetStoreDatasetTest.java
index fad2df795..c0d0719cb 100644
---
a/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/retention/CleanableMysqlDatasetStoreDatasetTest.java
+++
b/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/retention/CleanableMysqlDatasetStoreDatasetTest.java
@@ -20,7 +20,6 @@ package org.apache.gobblin.data.management.retention;
import java.io.IOException;
import java.util.List;
-import org.apache.commons.dbcp.BasicDataSource;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.testng.Assert;
@@ -29,6 +28,7 @@ import org.testng.annotations.Test;
import com.typesafe.config.Config;
import com.typesafe.config.ConfigValueFactory;
+import com.zaxxer.hikari.HikariDataSource;
import org.apache.gobblin.config.ConfigBuilder;
import org.apache.gobblin.configuration.ConfigurationKeys;
@@ -78,15 +78,15 @@ public class CleanableMysqlDatasetStoreDatasetTest {
this.testMetastoreDatabase = TestMetastoreDatabaseFactory.get();
String jdbcUrl = this.testMetastoreDatabase.getJdbcUrl();
ConfigBuilder configBuilder = ConfigBuilder.create();
- BasicDataSource mySqlDs = new BasicDataSource();
+ HikariDataSource dataSource = new HikariDataSource();
-
mySqlDs.setDriverClassName(ConfigurationKeys.DEFAULT_STATE_STORE_DB_JDBC_DRIVER);
- mySqlDs.setDefaultAutoCommit(false);
- mySqlDs.setUrl(jdbcUrl);
- mySqlDs.setUsername(TEST_USER);
- mySqlDs.setPassword(TEST_PASSWORD);
+
dataSource.setDriverClassName(ConfigurationKeys.DEFAULT_STATE_STORE_DB_JDBC_DRIVER);
+ dataSource.setAutoCommit(false);
+ dataSource.setJdbcUrl(jdbcUrl);
+ dataSource.setUsername(TEST_USER);
+ dataSource.setPassword(TEST_PASSWORD);
- this.dbJobStateStore = new MysqlStateStore<>(mySqlDs, TEST_STATE_STORE,
false, JobState.class);
+ this.dbJobStateStore = new MysqlStateStore<>(dataSource, TEST_STATE_STORE,
false, JobState.class);
configBuilder.addPrimitive("selection.timeBased.lookbackTime", "10m");
configBuilder.addPrimitive(ConfigurationKeys.STATE_STORE_TYPE_KEY,
"mysql");
diff --git
a/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/retention/sql/SqlBasedRetentionPoc.java
b/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/retention/sql/SqlBasedRetentionPoc.java
index ab62d5bcd..e7899f800 100644
---
a/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/retention/sql/SqlBasedRetentionPoc.java
+++
b/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/retention/sql/SqlBasedRetentionPoc.java
@@ -22,7 +22,8 @@ import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Timestamp;
-import org.apache.commons.dbcp.BasicDataSource;
+import com.zaxxer.hikari.HikariDataSource;
+
import org.apache.commons.lang.StringUtils;
import org.apache.hadoop.fs.Path;
import org.joda.time.DateTime;
@@ -42,7 +43,7 @@ public class SqlBasedRetentionPoc {
private static final int TWO_YEARS_IN_DAYS = 365 * 2;
private static final String DAILY_PARTITION_PATTERN = "yyyy/MM/dd";
- private BasicDataSource basicDataSource;
+ private HikariDataSource dataSource;
private Connection connection;
/**
@@ -55,11 +56,11 @@ public class SqlBasedRetentionPoc {
*/
@BeforeClass
public void setup() throws SQLException {
- basicDataSource = new BasicDataSource();
- basicDataSource.setDriverClassName("org.apache.derby.jdbc.EmbeddedDriver");
- basicDataSource.setUrl("jdbc:derby:memory:derbypoc;create=true");
+ dataSource = new HikariDataSource();
+ dataSource.setDriverClassName("org.apache.derby.jdbc.EmbeddedDriver");
+ dataSource.setJdbcUrl("jdbc:derby:memory:derbypoc;create=true");
- Connection connection = basicDataSource.getConnection();
+ Connection connection = dataSource.getConnection();
connection.setAutoCommit(false);
this.connection = connection;
@@ -72,7 +73,7 @@ public class SqlBasedRetentionPoc {
@AfterClass
public void cleanUp() throws Exception {
- basicDataSource.close();
+ dataSource.close();
}
/**
diff --git a/gobblin-metastore/build.gradle b/gobblin-metastore/build.gradle
index 2a0e85e26..7a0d83b2e 100644
--- a/gobblin-metastore/build.gradle
+++ b/gobblin-metastore/build.gradle
@@ -25,12 +25,12 @@ dependencies {
compile externalDependency.guava
compile externalDependency.slf4j
compile externalDependency.pegasus.data
- compile externalDependency.commonsDbcp
compile externalDependency.commonsLang
compile externalDependency.commonsLang3
compile externalDependency.guice
compile externalDependency.javaxInject
compile externalDependency.jodaTime
+ compile externalDependency.hikariCP
compile externalDependency.httpclient
compile externalDependency.flyway
compile externalDependency.commonsConfiguration
diff --git
a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/JobHistoryDataSourceProvider.java
b/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/JobHistoryDataSourceProvider.java
index 73d0b0339..edf35d637 100644
---
a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/JobHistoryDataSourceProvider.java
+++
b/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/JobHistoryDataSourceProvider.java
@@ -39,7 +39,7 @@ public class JobHistoryDataSourceProvider extends
org.apache.gobblin.util.jdbc.D
@Inject
public JobHistoryDataSourceProvider(@Named("dataSourceProperties")
Properties properties) {
-
this.basicDataSource.setDriverClassName(properties.getProperty(ConfigurationKeys.JOB_HISTORY_STORE_JDBC_DRIVER_KEY,
+
this.dataSource.setDriverClassName(properties.getProperty(ConfigurationKeys.JOB_HISTORY_STORE_JDBC_DRIVER_KEY,
ConfigurationKeys.DEFAULT_JOB_HISTORY_STORE_JDBC_DRIVER));
// Set validation query to verify connection
@@ -47,16 +47,24 @@ public class JobHistoryDataSourceProvider extends
org.apache.gobblin.util.jdbc.D
// MySQL server can timeout a connection so need to validate connections
before use
final String validationQuery =
MysqlDataSourceUtils.QUERY_CONNECTION_IS_VALID_AND_NOT_READONLY;
LOG.info("setting `DataSource` validation query: '" + validationQuery +
"'");
- this.basicDataSource.setValidationQuery(validationQuery);
- this.basicDataSource.setTestOnBorrow(true);
-
this.basicDataSource.setTimeBetweenEvictionRunsMillis(Duration.ofSeconds(60).toMillis());
+ // TODO: revisit following verification of successful connection pool
migration:
+ // If your driver supports JDBC4 we strongly recommend not setting
this property. This is for "legacy" drivers
+ // that do not support the JDBC4 Connection.isValid() API; see:
+ //
https://github.com/brettwooldridge/HikariCP#gear-configuration-knobs-baby
+ this.dataSource.setConnectionTestQuery(validationQuery);
+ this.dataSource.setIdleTimeout(Duration.ofSeconds(60).toMillis());
}
-
this.basicDataSource.setUrl(properties.getProperty(ConfigurationKeys.JOB_HISTORY_STORE_URL_KEY));
+
this.dataSource.setJdbcUrl(properties.getProperty(ConfigurationKeys.JOB_HISTORY_STORE_URL_KEY));
+ // TODO: revisit following verification of successful connection pool
migration:
+ // whereas `o.a.commons.dbcp.BasicDataSource` defaults min idle conns to
0, hikari defaults to 10.
+ // perhaps non-zero would have desirable runtime perf, but anything >0
currently fails unit tests (even 1!);
+ // (so experimenting with a higher number would first require adjusting
tests)
+ this.dataSource.setMinimumIdle(0);
if (properties.containsKey(ConfigurationKeys.JOB_HISTORY_STORE_USER_KEY)
&&
properties.containsKey(ConfigurationKeys.JOB_HISTORY_STORE_PASSWORD_KEY)) {
-
this.basicDataSource.setUsername(properties.getProperty(ConfigurationKeys.JOB_HISTORY_STORE_USER_KEY));
- this.basicDataSource.setPassword(PasswordManager.getInstance(properties)
+
this.dataSource.setUsername(properties.getProperty(ConfigurationKeys.JOB_HISTORY_STORE_USER_KEY));
+ this.dataSource.setPassword(PasswordManager.getInstance(properties)
.readPassword(properties.getProperty(ConfigurationKeys.JOB_HISTORY_STORE_PASSWORD_KEY)));
}
}
diff --git
a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlDagStateStoreFactory.java
b/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlDagStateStoreFactory.java
index 5327c4b11..9282d51e1 100644
---
a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlDagStateStoreFactory.java
+++
b/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlDagStateStoreFactory.java
@@ -17,7 +17,7 @@
package org.apache.gobblin.metastore;
-import org.apache.commons.dbcp.BasicDataSource;
+import javax.sql.DataSource;
import com.typesafe.config.Config;
@@ -37,10 +37,10 @@ public class MysqlDagStateStoreFactory extends
MysqlStateStoreFactory {
ConfigurationKeys.DEFAULT_STATE_STORE_COMPRESSED_VALUES);
try {
- BasicDataSource basicDataSource = MysqlDataSourceFactory.get(config,
+ DataSource dataSource = MysqlDataSourceFactory.get(config,
SharedResourcesBrokerFactory.getImplicitBroker());
- return new MysqlDagStore<>(basicDataSource, stateStoreTableName,
compressedValues, stateClass);
+ return new MysqlDagStore<>(dataSource, stateStoreTableName,
compressedValues, stateClass);
} catch (Exception e) {
throw new RuntimeException("Failed to create MysqlDagStore with
factory", e);
}
diff --git
a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlDataSourceFactory.java
b/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlDataSourceFactory.java
index 45bd045ef..c00e011df 100644
---
a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlDataSourceFactory.java
+++
b/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlDataSourceFactory.java
@@ -19,7 +19,7 @@ package org.apache.gobblin.metastore;
import java.io.IOException;
-import org.apache.commons.dbcp.BasicDataSource;
+import javax.sql.DataSource;
import com.typesafe.config.Config;
@@ -35,24 +35,25 @@ import
org.apache.gobblin.broker.iface.SharedResourcesBroker;
import lombok.extern.slf4j.Slf4j;
/**
- * A {@link SharedResourceFactory} for creating {@link BasicDataSource}s.
+ * A {@link SharedResourceFactory} for creating {@link DataSource}s.
*
- * The factory creates a {@link BasicDataSource} with the config.
+ * The factory creates a {@link DataSource} with the config.
*/
@Slf4j
public class MysqlDataSourceFactory<S extends ScopeType<S>>
- implements SharedResourceFactory<BasicDataSource, MysqlDataSourceKey, S> {
+ implements SharedResourceFactory<DataSource, MysqlDataSourceKey, S> {
+ // WARNING: now a misnomer, but retained for legacy compatibility, despite
move from `o.a.commons.dbcp.BasicDataSource` to `HikariCP`
public static final String FACTORY_NAME = "basicDataSource";
/**
- * Get a {@link BasicDataSource} based on the config
+ * Get a {@link DataSource} based on the config
* @param config configuration
* @param broker broker
- * @return a {@link BasicDataSource}
+ * @return a {@link DataSource}
* @throws IOException
*/
- public static <S extends ScopeType<S>> BasicDataSource get(Config config,
+ public static <S extends ScopeType<S>> DataSource get(Config config,
SharedResourcesBroker<S> broker) throws IOException {
try {
return broker.getSharedResource(new MysqlDataSourceFactory<S>(),
@@ -68,12 +69,12 @@ public class MysqlDataSourceFactory<S extends ScopeType<S>>
}
@Override
- public SharedResourceFactoryResponse<BasicDataSource>
createResource(SharedResourcesBroker<S> broker,
+ public SharedResourceFactoryResponse<DataSource>
createResource(SharedResourcesBroker<S> broker,
ScopedConfigView<S, MysqlDataSourceKey> config) throws
NotConfiguredException {
MysqlDataSourceKey key = config.getKey();
Config configuration = key.getConfig();
- BasicDataSource dataSource = MysqlStateStore.newDataSource(configuration);
+ DataSource dataSource = MysqlStateStore.newDataSource(configuration);
return new ResourceInstance<>(dataSource);
}
diff --git
a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlDataSourceKey.java
b/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlDataSourceKey.java
index bccab28f5..5e8b20927 100644
---
a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlDataSourceKey.java
+++
b/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlDataSourceKey.java
@@ -17,7 +17,7 @@
package org.apache.gobblin.metastore;
-import org.apache.commons.dbcp.BasicDataSource;
+import javax.sql.DataSource;
import com.typesafe.config.Config;
@@ -26,7 +26,7 @@ import org.apache.gobblin.broker.iface.SharedResourceKey;
import lombok.Getter;
/**
- * {@link SharedResourceKey} for requesting {@link BasicDataSource}s from a
+ * {@link SharedResourceKey} for requesting {@link DataSource}s from a
* {@link org.apache.gobblin.broker.iface.SharedResourceFactory}
*/
@Getter
diff --git
a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlJobStatusStateStoreFactory.java
b/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlJobStatusStateStoreFactory.java
index 5a2c68473..34d34d478 100644
---
a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlJobStatusStateStoreFactory.java
+++
b/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlJobStatusStateStoreFactory.java
@@ -17,7 +17,7 @@
package org.apache.gobblin.metastore;
-import org.apache.commons.dbcp.BasicDataSource;
+import javax.sql.DataSource;
import com.typesafe.config.Config;
@@ -36,10 +36,10 @@ public class MysqlJobStatusStateStoreFactory extends
MysqlStateStoreFactory impl
ConfigurationKeys.DEFAULT_STATE_STORE_COMPRESSED_VALUES);
try {
- BasicDataSource basicDataSource = MysqlDataSourceFactory.get(config,
+ DataSource dataSource = MysqlDataSourceFactory.get(config,
SharedResourcesBrokerFactory.getImplicitBroker());
- return new MysqlJobStatusStateStore<>(basicDataSource,
stateStoreTableName, compressedValues, stateClass);
+ return new MysqlJobStatusStateStore<>(dataSource, stateStoreTableName,
compressedValues, stateClass);
} catch (Exception e) {
throw new RuntimeException("Failed to create MysqlStateStore with
factory", e);
}
diff --git
a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlStateStore.java
b/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlStateStore.java
index f75cddc5a..3bbf8747b 100644
---
a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlStateStore.java
+++
b/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlStateStore.java
@@ -38,11 +38,11 @@ import java.util.Collections;
import java.util.List;
import java.util.zip.GZIPInputStream;
import java.util.zip.GZIPOutputStream;
+import javax.sql.DataSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import org.apache.commons.dbcp.BasicDataSource;
import org.apache.hadoop.io.Text;
import com.google.common.annotations.VisibleForTesting;
@@ -50,8 +50,7 @@ import com.google.common.base.Predicate;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import com.typesafe.config.Config;
-
-import javax.sql.DataSource;
+import com.zaxxer.hikari.HikariDataSource;
import org.apache.gobblin.configuration.ConfigurationKeys;
import org.apache.gobblin.configuration.State;
@@ -194,33 +193,38 @@ public class MysqlStateStore<T extends State> implements
StateStore<T> {
}
/**
- * creates a new {@link BasicDataSource}
+ * creates a new {@link DataSource}
* @param config the properties used for datasource instantiation
* @return
*/
- public static BasicDataSource newDataSource(Config config) {
- BasicDataSource basicDataSource = new BasicDataSource();
+ public static DataSource newDataSource(Config config) {
+ HikariDataSource dataSource = new HikariDataSource();
PasswordManager passwordManager =
PasswordManager.getInstance(ConfigUtils.configToProperties(config));
- basicDataSource.setDriverClassName(ConfigUtils.getString(config,
ConfigurationKeys.STATE_STORE_DB_JDBC_DRIVER_KEY,
+ dataSource.setDriverClassName(ConfigUtils.getString(config,
ConfigurationKeys.STATE_STORE_DB_JDBC_DRIVER_KEY,
ConfigurationKeys.DEFAULT_STATE_STORE_DB_JDBC_DRIVER));
// MySQL server can timeout a connection so need to validate connections
before use
final String validationQuery =
MysqlDataSourceUtils.QUERY_CONNECTION_IS_VALID_AND_NOT_READONLY;
LOG.info("setting `DataSource` validation query: '" + validationQuery +
"'");
- basicDataSource.setValidationQuery(validationQuery);
- basicDataSource.setTestOnBorrow(true);
- basicDataSource.setDefaultAutoCommit(false);
-
basicDataSource.setTimeBetweenEvictionRunsMillis(Duration.ofSeconds(60).toMillis());
-
basicDataSource.setUrl(config.getString(ConfigurationKeys.STATE_STORE_DB_URL_KEY));
- basicDataSource.setUsername(passwordManager.readPassword(
+ // TODO: revisit following verification of successful connection pool
migration:
+ // If your driver supports JDBC4 we strongly recommend not setting this
property. This is for "legacy" drivers
+ // that do not support the JDBC4 Connection.isValid() API; see:
+ //
https://github.com/brettwooldridge/HikariCP#gear-configuration-knobs-baby
+ dataSource.setConnectionTestQuery(validationQuery);
+ dataSource.setAutoCommit(false);
+ dataSource.setIdleTimeout(Duration.ofSeconds(60).toMillis());
+
dataSource.setJdbcUrl(config.getString(ConfigurationKeys.STATE_STORE_DB_URL_KEY));
+ // TODO: revisit following verification of successful connection pool
migration:
+ // whereas `o.a.commons.dbcp.BasicDataSource` defaults min idle conns to
0, hikari defaults to 10.
+ // perhaps non-zero would have desirable runtime perf, but anything >0
currently fails unit tests (even 1!);
+ // (so experimenting with a higher number would first require adjusting
tests)
+ dataSource.setMinimumIdle(0);
+ dataSource.setUsername(passwordManager.readPassword(
config.getString(ConfigurationKeys.STATE_STORE_DB_USER_KEY)));
- basicDataSource.setPassword(passwordManager.readPassword(
+ dataSource.setPassword(passwordManager.readPassword(
config.getString(ConfigurationKeys.STATE_STORE_DB_PASSWORD_KEY)));
- basicDataSource.setMinEvictableIdleTimeMillis(
- ConfigUtils.getLong(config,
ConfigurationKeys.STATE_STORE_DB_CONN_MIN_EVICTABLE_IDLE_TIME_KEY,
-
ConfigurationKeys.DEFAULT_STATE_STORE_DB_CONN_MIN_EVICTABLE_IDLE_TIME));
- return basicDataSource;
+ return dataSource;
}
/**
diff --git
a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlStateStoreFactory.java
b/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlStateStoreFactory.java
index fc9e362d7..eccb0cd59 100644
---
a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlStateStoreFactory.java
+++
b/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/MysqlStateStoreFactory.java
@@ -16,7 +16,7 @@
*/
package org.apache.gobblin.metastore;
-import org.apache.commons.dbcp.BasicDataSource;
+import javax.sql.DataSource;
import com.typesafe.config.Config;
@@ -36,10 +36,10 @@ public class MysqlStateStoreFactory implements
StateStore.Factory {
ConfigurationKeys.DEFAULT_STATE_STORE_COMPRESSED_VALUES);
try {
- BasicDataSource basicDataSource = MysqlDataSourceFactory.get(config,
+ DataSource dataSource = MysqlDataSourceFactory.get(config,
SharedResourcesBrokerFactory.getImplicitBroker());
- return new MysqlStateStore<>(basicDataSource, stateStoreTableName,
compressedValues, stateClass);
+ return new MysqlStateStore<>(dataSource, stateStoreTableName,
compressedValues, stateClass);
} catch (Exception e) {
throw new RuntimeException("Failed to create MysqlStateStore with
factory", e);
}
diff --git
a/gobblin-metastore/src/test/java/org/apache/gobblin/metastore/MysqlDataSourceFactoryTest.java
b/gobblin-metastore/src/test/java/org/apache/gobblin/metastore/MysqlDataSourceFactoryTest.java
index a11a4aff4..40e0aaa31 100644
---
a/gobblin-metastore/src/test/java/org/apache/gobblin/metastore/MysqlDataSourceFactoryTest.java
+++
b/gobblin-metastore/src/test/java/org/apache/gobblin/metastore/MysqlDataSourceFactoryTest.java
@@ -17,8 +17,8 @@
package org.apache.gobblin.metastore;
import java.io.IOException;
+import javax.sql.DataSource;
-import org.apache.commons.dbcp.BasicDataSource;
import org.testng.Assert;
import org.testng.annotations.Test;
@@ -42,13 +42,13 @@ public class MysqlDataSourceFactoryTest {
ConfigurationKeys.STATE_STORE_DB_USER_KEY, "user",
ConfigurationKeys.STATE_STORE_DB_PASSWORD_KEY, "dummypwd"));
- BasicDataSource basicDataSource1 = MysqlDataSourceFactory.get(config,
+ DataSource dataSource1 = MysqlDataSourceFactory.get(config,
SharedResourcesBrokerFactory.getImplicitBroker());
- BasicDataSource basicDataSource2 = MysqlDataSourceFactory.get(config,
+ DataSource dataSource2 = MysqlDataSourceFactory.get(config,
SharedResourcesBrokerFactory.getImplicitBroker());
- Assert.assertEquals(basicDataSource1, basicDataSource2);
+ Assert.assertEquals(dataSource1, dataSource2);
}
@Test
@@ -62,13 +62,13 @@ public class MysqlDataSourceFactoryTest {
ConfigurationKeys.STATE_STORE_DB_USER_KEY, "user",
ConfigurationKeys.STATE_STORE_DB_PASSWORD_KEY, "dummypwd"));
- BasicDataSource basicDataSource1 = MysqlDataSourceFactory.get(config1,
+ DataSource dataSource1 = MysqlDataSourceFactory.get(config1,
SharedResourcesBrokerFactory.getImplicitBroker());
- BasicDataSource basicDataSource2 = MysqlDataSourceFactory.get(config2,
+ DataSource dataSource2 = MysqlDataSourceFactory.get(config2,
SharedResourcesBrokerFactory.getImplicitBroker());
- Assert.assertNotEquals(basicDataSource1, basicDataSource2);
+ Assert.assertNotEquals(dataSource1, dataSource2);
}
@Test
@@ -82,12 +82,12 @@ public class MysqlDataSourceFactoryTest {
ConfigurationKeys.STATE_STORE_DB_USER_KEY, "user2",
ConfigurationKeys.STATE_STORE_DB_PASSWORD_KEY, "dummypwd"));
- BasicDataSource basicDataSource1 = MysqlDataSourceFactory.get(config1,
+ DataSource dataSource1 = MysqlDataSourceFactory.get(config1,
SharedResourcesBrokerFactory.getImplicitBroker());
- BasicDataSource basicDataSource2 = MysqlDataSourceFactory.get(config2,
+ DataSource dataSource2 = MysqlDataSourceFactory.get(config2,
SharedResourcesBrokerFactory.getImplicitBroker());
- Assert.assertNotEquals(basicDataSource1, basicDataSource2);
+ Assert.assertNotEquals(dataSource1, dataSource2);
}
}
diff --git
a/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/converter/initializer/AvroToJdbcEntryConverterInitializer.java
b/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/converter/initializer/AvroToJdbcEntryConverterInitializer.java
index d3ca9f9c7..1802da98f 100644
---
a/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/converter/initializer/AvroToJdbcEntryConverterInitializer.java
+++
b/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/converter/initializer/AvroToJdbcEntryConverterInitializer.java
@@ -108,7 +108,7 @@ public class AvroToJdbcEntryConverterInitializer implements
ConverterInitializer
.userName(this.state.getProp(JdbcPublisher.JDBC_PUBLISHER_USERNAME))
.passWord(this.state.getProp(JdbcPublisher.JDBC_PUBLISHER_PASSWORD))
.cryptoKeyLocation(this.state.getProp(JdbcPublisher.JDBC_PUBLISHER_ENCRYPTION_KEY_LOC)).maxActiveConnections(1)
- .maxIdleConnections(1).state(this.state).build();
+ .state(this.state).build();
return dataSource.getConnection();
}
diff --git
a/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/publisher/JdbcPublisher.java
b/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/publisher/JdbcPublisher.java
index 4561bb651..cf4909bcf 100644
---
a/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/publisher/JdbcPublisher.java
+++
b/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/publisher/JdbcPublisher.java
@@ -106,7 +106,7 @@ public class JdbcPublisher extends DataPublisher {
.driver(this.state.getProp(JDBC_PUBLISHER_DRIVER)).userName(this.state.getProp(JDBC_PUBLISHER_USERNAME))
.passWord(this.state.getProp(JDBC_PUBLISHER_PASSWORD))
.cryptoKeyLocation(this.state.getProp(JDBC_PUBLISHER_ENCRYPTION_KEY_LOC)).maxActiveConnections(1)
- .maxIdleConnections(1).state(this.state).build();
+ .state(this.state).build();
try {
return dataSource.getConnection();
} catch (SQLException e) {
diff --git
a/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/source/jdbc/JdbcProvider.java
b/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/source/jdbc/JdbcProvider.java
index 6775f224e..db6494088 100644
---
a/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/source/jdbc/JdbcProvider.java
+++
b/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/source/jdbc/JdbcProvider.java
@@ -17,11 +17,11 @@
package org.apache.gobblin.source.jdbc;
+import java.io.IOException;
+
import org.apache.gobblin.tunnel.Tunnel;
-import org.apache.commons.dbcp.BasicDataSource;
-import java.io.IOException;
-import java.sql.SQLException;
+import com.zaxxer.hikari.HikariDataSource;
/**
@@ -29,7 +29,7 @@ import java.sql.SQLException;
*
* @author nveeramr
*/
-public class JdbcProvider extends BasicDataSource {
+public class JdbcProvider extends HikariDataSource {
private Tunnel tunnel;
// If extract type is not provided then consider it as a default type
@@ -76,14 +76,18 @@ public class JdbcProvider extends BasicDataSource {
this.setDriverClassName(driver);
this.setUsername(user);
this.setPassword(password);
- this.setUrl(connectionUrl);
- this.setInitialSize(0);
- this.setMaxIdle(numconn);
- this.setMaxWait(timeout);
+ this.setJdbcUrl(connectionUrl);
+ // TODO: revisit following verification of successful connection pool
migration:
+ // whereas `o.a.commons.dbcp.BasicDataSource` defaults min idle conns to
0, hikari defaults to 10.
+ // perhaps non-zero would have desirable runtime perf, but anything >0
currently fails unit tests (even 1!);
+ // (so experimenting with a higher number would first require adjusting
tests)
+ this.setMinimumIdle(0);
+ this.setMaximumPoolSize(numconn);
+ this.setConnectionTimeout(timeout);
}
@Override
- public synchronized void close() throws SQLException {
+ public synchronized void close() {
super.close();
if (this.tunnel != null) {
this.tunnel.close();
diff --git
a/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/writer/JdbcWriter.java
b/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/writer/JdbcWriter.java
index 7404f30db..c30a453e9 100644
---
a/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/writer/JdbcWriter.java
+++
b/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/writer/JdbcWriter.java
@@ -116,7 +116,7 @@ public class JdbcWriter implements
DataWriter<JdbcEntryData> {
.userName(this.state.getProp(JdbcPublisher.JDBC_PUBLISHER_USERNAME))
.passWord(this.state.getProp(JdbcPublisher.JDBC_PUBLISHER_PASSWORD))
.cryptoKeyLocation(this.state.getProp(JdbcPublisher.JDBC_PUBLISHER_ENCRYPTION_KEY_LOC)).maxActiveConnections(1)
- .maxIdleConnections(1).state(this.state).build();
+ .state(this.state).build();
return dataSource.getConnection();
}
diff --git
a/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/writer/initializer/JdbcWriterInitializer.java
b/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/writer/initializer/JdbcWriterInitializer.java
index bdaf1e1cc..8226de769 100644
---
a/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/writer/initializer/JdbcWriterInitializer.java
+++
b/gobblin-modules/gobblin-sql/src/main/java/org/apache/gobblin/writer/initializer/JdbcWriterInitializer.java
@@ -132,7 +132,7 @@ public class JdbcWriterInitializer implements
WriterInitializer {
.userName(this.state.getProp(JdbcPublisher.JDBC_PUBLISHER_USERNAME))
.passWord(this.state.getProp(JdbcPublisher.JDBC_PUBLISHER_PASSWORD))
.cryptoKeyLocation(this.state.getProp(JdbcPublisher.JDBC_PUBLISHER_ENCRYPTION_KEY_LOC)).maxActiveConnections(1)
- .maxIdleConnections(1).state(this.state).build();
+ .state(this.state).build();
return dataSource.getConnection();
}
diff --git
a/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/MysqlDatasetStateStoreFactory.java
b/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/MysqlDatasetStateStoreFactory.java
index 8d5b29185..ce621bbbb 100644
---
a/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/MysqlDatasetStateStoreFactory.java
+++
b/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/MysqlDatasetStateStoreFactory.java
@@ -16,7 +16,7 @@
*/
package org.apache.gobblin.runtime;
-import org.apache.commons.dbcp.BasicDataSource;
+import javax.sql.DataSource;
import com.typesafe.config.Config;
@@ -38,10 +38,10 @@ public class MysqlDatasetStateStoreFactory implements
DatasetStateStore.Factory
ConfigurationKeys.DEFAULT_STATE_STORE_COMPRESSED_VALUES;
try {
- BasicDataSource basicDataSource = MysqlDataSourceFactory.get(config,
+ DataSource dataSource = MysqlDataSourceFactory.get(config,
SharedResourcesBrokerFactory.getImplicitBroker());
- return new MysqlDatasetStateStore(basicDataSource, stateStoreTableName,
compressedValues);
+ return new MysqlDatasetStateStore(dataSource, stateStoreTableName,
compressedValues);
} catch (Exception e) {
throw new RuntimeException("Failed to create MysqlDatasetStateStore with
factory", e);
}
diff --git
a/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_store/MysqlBaseSpecStore.java
b/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_store/MysqlBaseSpecStore.java
index 3e3a78201..b12360466 100644
---
a/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_store/MysqlBaseSpecStore.java
+++
b/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_store/MysqlBaseSpecStore.java
@@ -28,13 +28,13 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
-
-import org.apache.commons.dbcp.BasicDataSource;
+import javax.sql.DataSource;
import com.google.common.base.Optional;
import com.google.common.collect.Lists;
import com.google.common.io.ByteStreams;
import com.typesafe.config.Config;
+import com.zaxxer.hikari.HikariDataSource;
import lombok.extern.slf4j.Slf4j;
@@ -132,7 +132,7 @@ public class MysqlBaseSpecStore extends
InstrumentedSpecStore {
}
- protected final BasicDataSource dataSource;
+ protected final DataSource dataSource;
protected final String tableName;
private final URI specStoreURI;
protected final SpecSerDe specSerDe;
@@ -331,7 +331,11 @@ public class MysqlBaseSpecStore extends
InstrumentedSpecStore {
}
return result;
} catch (SQLException e) {
- log.warn("Received SQL exception that can result from invalid
connection. Checking if validation query is set {} Exception is {}",
((BasicDataSource) this.dataSource).getValidationQuery(), e);
+ // TODO: revisit use of connection test query following verification of
successful connection pool migration:
+ // If your driver supports JDBC4 we strongly recommend not setting
this property. This is for "legacy" drivers
+ // that do not support the JDBC4 Connection.isValid() API; see:
+ //
https://github.com/brettwooldridge/HikariCP#gear-configuration-knobs-baby
+ log.warn("Received SQL exception that can result from invalid
connection. Checking if validation query is set {} Exception is {}",
((HikariDataSource) this.dataSource).getConnectionTestQuery(), e);
throw new IOException(e);
}
}
diff --git
a/gobblin-runtime/src/test/java/org/apache/gobblin/runtime/MysqlDatasetStateStoreTest.java
b/gobblin-runtime/src/test/java/org/apache/gobblin/runtime/MysqlDatasetStateStoreTest.java
index 956a87576..14c5a9166 100644
---
a/gobblin-runtime/src/test/java/org/apache/gobblin/runtime/MysqlDatasetStateStoreTest.java
+++
b/gobblin-runtime/src/test/java/org/apache/gobblin/runtime/MysqlDatasetStateStoreTest.java
@@ -22,13 +22,13 @@ import java.util.Collections;
import java.util.List;
import java.util.Map;
-import org.apache.commons.dbcp.BasicDataSource;
import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
import com.google.common.base.Predicates;
+import com.zaxxer.hikari.HikariDataSource;
import org.apache.gobblin.config.ConfigBuilder;
import org.apache.gobblin.configuration.ConfigurationKeys;
@@ -70,15 +70,15 @@ public class MysqlDatasetStateStoreTest {
testMetastoreDatabase = TestMetastoreDatabaseFactory.get();
String jdbcUrl = testMetastoreDatabase.getJdbcUrl();
ConfigBuilder configBuilder = ConfigBuilder.create();
- BasicDataSource mySqlDs = new BasicDataSource();
+ HikariDataSource dataSource = new HikariDataSource();
-
mySqlDs.setDriverClassName(ConfigurationKeys.DEFAULT_STATE_STORE_DB_JDBC_DRIVER);
- mySqlDs.setDefaultAutoCommit(false);
- mySqlDs.setUrl(jdbcUrl);
- mySqlDs.setUsername(TEST_USER);
- mySqlDs.setPassword(TEST_PASSWORD);
+
dataSource.setDriverClassName(ConfigurationKeys.DEFAULT_STATE_STORE_DB_JDBC_DRIVER);
+ dataSource.setAutoCommit(false);
+ dataSource.setJdbcUrl(jdbcUrl);
+ dataSource.setUsername(TEST_USER);
+ dataSource.setPassword(TEST_PASSWORD);
- dbJobStateStore = new MysqlStateStore<>(mySqlDs, TEST_STATE_STORE, false,
JobState.class);
+ dbJobStateStore = new MysqlStateStore<>(dataSource, TEST_STATE_STORE,
false, JobState.class);
configBuilder.addPrimitive(ConfigurationKeys.STATE_STORE_DB_URL_KEY,
jdbcUrl);
configBuilder.addPrimitive(ConfigurationKeys.STATE_STORE_DB_USER_KEY,
TEST_USER);
diff --git
a/gobblin-runtime/src/test/java/org/apache/gobblin/runtime/cli/JobStateStoreCliTest.java
b/gobblin-runtime/src/test/java/org/apache/gobblin/runtime/cli/JobStateStoreCliTest.java
index 075d056e5..6b5213284 100644
---
a/gobblin-runtime/src/test/java/org/apache/gobblin/runtime/cli/JobStateStoreCliTest.java
+++
b/gobblin-runtime/src/test/java/org/apache/gobblin/runtime/cli/JobStateStoreCliTest.java
@@ -20,7 +20,9 @@ package org.apache.gobblin.runtime.cli;
import java.io.File;
import java.io.FileOutputStream;
import java.io.OutputStream;
-import org.apache.commons.dbcp.BasicDataSource;
+
+import com.zaxxer.hikari.HikariDataSource;
+
import org.apache.gobblin.config.ConfigBuilder;
import org.apache.gobblin.configuration.ConfigurationKeys;
import org.apache.gobblin.configuration.WorkUnitState;
@@ -64,15 +66,15 @@ public class JobStateStoreCliTest {
testMetastoreDatabase = TestMetastoreDatabaseFactory.get();
String jdbcUrl = testMetastoreDatabase.getJdbcUrl();
ConfigBuilder configBuilder = ConfigBuilder.create();
- BasicDataSource mySqlDs = new BasicDataSource();
+ HikariDataSource dataSource = new HikariDataSource();
-
mySqlDs.setDriverClassName(ConfigurationKeys.DEFAULT_STATE_STORE_DB_JDBC_DRIVER);
- mySqlDs.setDefaultAutoCommit(false);
- mySqlDs.setUrl(jdbcUrl);
- mySqlDs.setUsername(TEST_USER);
- mySqlDs.setPassword(TEST_PASSWORD);
+
dataSource.setDriverClassName(ConfigurationKeys.DEFAULT_STATE_STORE_DB_JDBC_DRIVER);
+ dataSource.setAutoCommit(false);
+ dataSource.setJdbcUrl(jdbcUrl);
+ dataSource.setUsername(TEST_USER);
+ dataSource.setPassword(TEST_PASSWORD);
- dbJobStateStore = new MysqlStateStore<>(mySqlDs, TEST_STATE_STORE, false,
JobState.class);
+ dbJobStateStore = new MysqlStateStore<>(dataSource, TEST_STATE_STORE,
false, JobState.class);
configBuilder.addPrimitive(ConfigurationKeys.STATE_STORE_DB_URL_KEY,
jdbcUrl);
configBuilder.addPrimitive(ConfigurationKeys.STATE_STORE_DB_USER_KEY,
TEST_USER);
diff --git a/gobblin-service/build.gradle b/gobblin-service/build.gradle
index 6fb986e93..af029ad5c 100644
--- a/gobblin-service/build.gradle
+++ b/gobblin-service/build.gradle
@@ -43,7 +43,6 @@ dependencies {
compile externalDependency.curatorFramework
compile externalDependency.curatorClient
compile externalDependency.curatorRecipes
- compile externalDependency.commonsDbcp2
compile externalDependency.findBugsAnnotations
compile externalDependency.flyway
compile externalDependency.gson
@@ -56,6 +55,7 @@ dependencies {
compile (externalDependency.helix) {
exclude group: 'io.dropwizard.metrics', module: 'metrics-core'
}
+ compile externalDependency.hikariCP
compile externalDependency.hiveCommon
compile externalDependency.httpclient
compile externalDependency.httpcore
diff --git
a/gobblin-service/src/main/java/org/apache/gobblin/service/modules/db/ServiceDatabaseProviderImpl.java
b/gobblin-service/src/main/java/org/apache/gobblin/service/modules/db/ServiceDatabaseProviderImpl.java
index b9c2d32e4..7d2d719da 100644
---
a/gobblin-service/src/main/java/org/apache/gobblin/service/modules/db/ServiceDatabaseProviderImpl.java
+++
b/gobblin-service/src/main/java/org/apache/gobblin/service/modules/db/ServiceDatabaseProviderImpl.java
@@ -23,9 +23,8 @@ import java.util.Objects;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import org.apache.commons.dbcp2.BasicDataSource;
-
import com.typesafe.config.Config;
+import com.zaxxer.hikari.HikariDataSource;
import javax.inject.Inject;
import javax.sql.DataSource;
@@ -44,7 +43,7 @@ public class ServiceDatabaseProviderImpl implements
ServiceDatabaseProvider {
private static final Logger LOG =
LoggerFactory.getLogger(ServiceDatabaseProviderImpl.class);
private final Configuration configuration;
- private BasicDataSource dataSource;
+ private HikariDataSource dataSource;
@Inject
public ServiceDatabaseProviderImpl(Configuration configuration) {
@@ -61,25 +60,28 @@ public class ServiceDatabaseProviderImpl implements
ServiceDatabaseProvider {
return;
}
- dataSource = new BasicDataSource();
+ dataSource = new HikariDataSource();
- dataSource.setUrl(configuration.getUrl());
+ dataSource.setJdbcUrl(configuration.getUrl());
dataSource.setUsername(configuration.getUserName());
dataSource.setPassword(configuration.getPassword());
// MySQL server can timeout a connection so need to validate connections
before use
final String validationQuery =
MysqlDataSourceUtils.QUERY_CONNECTION_IS_VALID_AND_NOT_READONLY;
LOG.info("setting `DataSource` validation query: '" + validationQuery +
"'");
- dataSource.setValidationQuery(validationQuery);
- dataSource.setValidationQueryTimeout(5);
-
- // To improve performance, we only check connections on creation, and set
a maximum connection lifetime
+ // TODO: revisit following verification of successful connection pool
migration:
+ // If your driver supports JDBC4 we strongly recommend not setting this
property. This is for "legacy" drivers
+ // that do not support the JDBC4 Connection.isValid() API; see:
+ //
https://github.com/brettwooldridge/HikariCP#gear-configuration-knobs-baby
+ dataSource.setConnectionTestQuery(validationQuery);
+ dataSource.setValidationTimeout(Duration.ofSeconds(5).toMillis());
+
+ // To improve performance, we set a maximum connection lifetime
// If database goes to read-only mode, then connection would not work
correctly for up to configured lifetime
- dataSource.setTestOnCreate(true);
-
dataSource.setMaxConnLifetimeMillis(configuration.getMaxConnectionLifetime().toMillis());
-
dataSource.setTimeBetweenEvictionRunsMillis(Duration.ofSeconds(10).toMillis());
- dataSource.setMinIdle(2);
- dataSource.setMaxTotal(configuration.getMaxConnections());
+
dataSource.setMaxLifetime(configuration.getMaxConnectionLifetime().toMillis());
+ dataSource.setIdleTimeout(Duration.ofSeconds(10).toMillis());
+ dataSource.setMinimumIdle(2);
+ dataSource.setMaximumPoolSize(configuration.getMaxConnections());
}
@Builder
diff --git
a/gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/MysqlUserQuotaManager.java
b/gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/MysqlUserQuotaManager.java
index 41a9a03a8..8f5c57e49 100644
---
a/gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/MysqlUserQuotaManager.java
+++
b/gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/MysqlUserQuotaManager.java
@@ -27,11 +27,11 @@ import java.sql.SQLException;
import java.util.Collection;
import java.util.List;
-import org.apache.commons.dbcp.BasicDataSource;
import com.google.common.annotations.VisibleForTesting;
import com.google.inject.Inject;
import com.typesafe.config.Config;
+import com.zaxxer.hikari.HikariDataSource;
import javax.inject.Singleton;
import javax.sql.DataSource;
@@ -265,22 +265,22 @@ public class MysqlUserQuotaManager extends
AbstractUserQuotaManager {
String quotaStoreTableName = ConfigUtils.getString(config,
ServiceConfigKeys.QUOTA_STORE_DB_TABLE_KEY,
ServiceConfigKeys.DEFAULT_QUOTA_STORE_DB_TABLE);
- BasicDataSource basicDataSource = MysqlStateStore.newDataSource(config);
+ DataSource dataSource = MysqlStateStore.newDataSource(config);
- return new MysqlQuotaStore(basicDataSource, quotaStoreTableName);
+ return new MysqlQuotaStore(dataSource, quotaStoreTableName);
}
protected RunningDagIdsStore createRunningDagStore(Config config) throws
IOException {
String quotaStoreTableName = ConfigUtils.getString(config,
ServiceConfigKeys.RUNNING_DAG_IDS_DB_TABLE_KEY,
ServiceConfigKeys.DEFAULT_RUNNING_DAG_IDS_DB_TABLE);
- BasicDataSource basicDataSource = MysqlStateStore.newDataSource(config);
+ DataSource dataSource = MysqlStateStore.newDataSource(config);
- return new RunningDagIdsStore(basicDataSource, quotaStoreTableName);
+ return new RunningDagIdsStore(dataSource, quotaStoreTableName);
}
static class MysqlQuotaStore {
- protected final BasicDataSource dataSource;
+ protected final DataSource dataSource;
final String tableName;
private final String GET_USER_COUNT;
private final String GET_REQUESTER_COUNT;
@@ -293,7 +293,8 @@ public class MysqlUserQuotaManager extends
AbstractUserQuotaManager {
private final String DECREASE_FLOWGROUP_COUNT_SQL;
private final String DELETE_USER_SQL;
- public MysqlQuotaStore(BasicDataSource dataSource, String tableName)
+
@edu.umd.cs.findbugs.annotations.SuppressFBWarnings("SQL_PREPARED_STATEMENT_GENERATED_FROM_NONCONSTANT_STRING")
+ public MysqlQuotaStore(DataSource dataSource, String tableName)
throws IOException {
this.dataSource = dataSource;
this.tableName = tableName;
@@ -319,8 +320,12 @@ public class MysqlUserQuotaManager extends
AbstractUserQuotaManager {
try (Connection connection = dataSource.getConnection();
PreparedStatement createStatement =
connection.prepareStatement(createQuotaTable)) {
createStatement.executeUpdate();
} catch (SQLException e) {
+ // TODO: revisit use of connection test query following verification
of successful connection pool migration:
+ // If your driver supports JDBC4 we strongly recommend not setting
this property. This is for "legacy" drivers
+ // that do not support the JDBC4 Connection.isValid() API; see:
+ //
https://github.com/brettwooldridge/HikariCP#gear-configuration-knobs-baby
log.warn("Failure in creating table {}. Validation query is set to {}
Exception is {}",
- tableName, this.dataSource.getValidationQuery(), e);
+ tableName, ((HikariDataSource)
this.dataSource).getConnectionTestQuery(), e);
throw new IOException(e);
}
}
@@ -437,7 +442,8 @@ public class MysqlUserQuotaManager extends
AbstractUserQuotaManager {
private final String ADD_DAG_ID;
private final String REMOVE_DAG_ID;
- public RunningDagIdsStore(BasicDataSource dataSource, String tableName)
+
@edu.umd.cs.findbugs.annotations.SuppressFBWarnings("SQL_PREPARED_STATEMENT_GENERATED_FROM_NONCONSTANT_STRING")
+ public RunningDagIdsStore(DataSource dataSource, String tableName)
throws IOException {
this.dataSource = dataSource;
this.tableName = tableName;
diff --git
a/gobblin-service/src/test/java/org/apache/gobblin/service/modules/orchestration/MysqlDagStateStoreTest.java
b/gobblin-service/src/test/java/org/apache/gobblin/service/modules/orchestration/MysqlDagStateStoreTest.java
index dfbbd6a92..97d2ae698 100644
---
a/gobblin-service/src/test/java/org/apache/gobblin/service/modules/orchestration/MysqlDagStateStoreTest.java
+++
b/gobblin-service/src/test/java/org/apache/gobblin/service/modules/orchestration/MysqlDagStateStoreTest.java
@@ -23,12 +23,12 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
-import org.apache.commons.dbcp.BasicDataSource;
import org.testng.Assert;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
import com.typesafe.config.Config;
+import com.zaxxer.hikari.HikariDataSource;
import org.apache.gobblin.config.ConfigBuilder;
import org.apache.gobblin.configuration.ConfigurationKeys;
@@ -166,15 +166,15 @@ public class MysqlDagStateStoreTest {
// Setting up mock DB
ITestMetastoreDatabase testMetastoreDatabase =
TestMetastoreDatabaseFactory.get();
String jdbcUrl = testMetastoreDatabase.getJdbcUrl();
- BasicDataSource mySqlDs = new BasicDataSource();
+ HikariDataSource dataSource = new HikariDataSource();
-
mySqlDs.setDriverClassName(ConfigurationKeys.DEFAULT_STATE_STORE_DB_JDBC_DRIVER);
- mySqlDs.setDefaultAutoCommit(false);
- mySqlDs.setUrl(jdbcUrl);
- mySqlDs.setUsername(TEST_USER);
- mySqlDs.setPassword(TEST_PASSWORD);
+
dataSource.setDriverClassName(ConfigurationKeys.DEFAULT_STATE_STORE_DB_JDBC_DRIVER);
+ dataSource.setAutoCommit(false);
+ dataSource.setJdbcUrl(jdbcUrl);
+ dataSource.setUsername(TEST_USER);
+ dataSource.setPassword(TEST_PASSWORD);
- return new MysqlStateStore<>(mySqlDs, TEST_DAG_STATE_STORE, false,
State.class);
+ return new MysqlStateStore<>(dataSource, TEST_DAG_STATE_STORE, false,
State.class);
} catch (Exception e) {
throw new RuntimeException(e);
}
diff --git a/gobblin-utility/build.gradle b/gobblin-utility/build.gradle
index c80ea8346..7bba0ec2b 100644
--- a/gobblin-utility/build.gradle
+++ b/gobblin-utility/build.gradle
@@ -23,7 +23,6 @@ dependencies {
compile externalDependency.commonsCli
compile externalDependency.commonsConfiguration
compile externalDependency.commonsCompress
- compile externalDependency.commonsDbcp
compile externalDependency.commonsEmail
compile externalDependency.commonsIo
compile externalDependency.commonsLang
@@ -33,6 +32,7 @@ dependencies {
compile externalDependency.avroCompiler
compile externalDependency.avroCompatHelper
compile externalDependency.orcCore
+ compile externalDependency.hikariCP
compile externalDependency.hiveMetastore
compile externalDependency.jodaTime
compile externalDependency.jacksonCore
diff --git
a/gobblin-utility/src/main/java/org/apache/gobblin/util/jdbc/DataSourceBuilder.java
b/gobblin-utility/src/main/java/org/apache/gobblin/util/jdbc/DataSourceBuilder.java
index 83cafffbb..528950fed 100644
---
a/gobblin-utility/src/main/java/org/apache/gobblin/util/jdbc/DataSourceBuilder.java
+++
b/gobblin-utility/src/main/java/org/apache/gobblin/util/jdbc/DataSourceBuilder.java
@@ -41,7 +41,6 @@ public class DataSourceBuilder {
private String driver;
private String userName;
private String passWord;
- private Integer maxIdleConnections;
private Integer maxActiveConnections;
private String cryptoKeyLocation;
private Boolean useStrongEncryption;
@@ -71,11 +70,6 @@ public class DataSourceBuilder {
return this;
}
- public DataSourceBuilder maxIdleConnections(int maxIdleConnections) {
- this.maxIdleConnections = maxIdleConnections;
- return this;
- }
-
public DataSourceBuilder maxActiveConnections(int maxActiveConnections) {
this.maxActiveConnections = maxActiveConnections;
return this;
@@ -110,10 +104,6 @@ public class DataSourceBuilder {
properties.setProperty(ConfigurationKeys.ENCRYPT_KEY_LOC,
this.cryptoKeyLocation);
}
- if (this.maxIdleConnections != null) {
- properties.setProperty(DataSourceProvider.MAX_IDLE_CONNS,
this.maxIdleConnections.toString());
- }
-
if (this.maxActiveConnections != null) {
properties.setProperty(DataSourceProvider.MAX_ACTIVE_CONNS,
this.maxActiveConnections.toString());
}
@@ -133,8 +123,6 @@ public class DataSourceBuilder {
validateNotEmpty(this.url, "url");
validateNotEmpty(this.driver, "driver");
validateNotEmpty(this.passWord, "passWord");
- Preconditions.checkArgument(this.maxIdleConnections == null ||
this.maxIdleConnections > 0,
- "maxIdleConnections should be a positive integer.");
Preconditions.checkArgument(this.maxActiveConnections == null ||
this.maxActiveConnections > 0,
"maxActiveConnections should be a positive integer.");
}
diff --git
a/gobblin-utility/src/main/java/org/apache/gobblin/util/jdbc/DataSourceProvider.java
b/gobblin-utility/src/main/java/org/apache/gobblin/util/jdbc/DataSourceProvider.java
index 86afeea60..dbd4222bc 100644
---
a/gobblin-utility/src/main/java/org/apache/gobblin/util/jdbc/DataSourceProvider.java
+++
b/gobblin-utility/src/main/java/org/apache/gobblin/util/jdbc/DataSourceProvider.java
@@ -26,11 +26,10 @@ import javax.sql.DataSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import org.apache.commons.dbcp.BasicDataSource;
-
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.name.Named;
+import com.zaxxer.hikari.HikariDataSource;
import org.apache.gobblin.password.PasswordManager;
@@ -48,36 +47,40 @@ public class DataSourceProvider implements
Provider<DataSource> {
public static final String USERNAME = GOBBLIN_UTIL_JDBC_PREFIX + "username";
public static final String PASSWORD = GOBBLIN_UTIL_JDBC_PREFIX + "password";
public static final String SKIP_VALIDATION_QUERY = GOBBLIN_UTIL_JDBC_PREFIX
+ "skip.validation.query";
- public static final String MAX_IDLE_CONNS = GOBBLIN_UTIL_JDBC_PREFIX +
"max.idle.connections";
public static final String MAX_ACTIVE_CONNS = GOBBLIN_UTIL_JDBC_PREFIX +
"max.active.connections";
public static final String DEFAULT_CONN_DRIVER = "com.mysql.jdbc.Driver";
- protected final BasicDataSource basicDataSource;
+ protected final HikariDataSource dataSource;
@Inject
public DataSourceProvider(@Named("dataSourceProperties") Properties
properties) {
- this.basicDataSource = new BasicDataSource();
-
this.basicDataSource.setDriverClassName(properties.getProperty(CONN_DRIVER,
DEFAULT_CONN_DRIVER));
+ this.dataSource = new HikariDataSource();
+ this.dataSource.setDriverClassName(properties.getProperty(CONN_DRIVER,
DEFAULT_CONN_DRIVER));
// the validation query should work beyond mysql; still, to bypass for any
reason, heed directive
if (!Boolean.parseBoolean(properties.getProperty(SKIP_VALIDATION_QUERY,
"false"))) {
// MySQL server can timeout a connection so need to validate connections
before use
final String validationQuery =
MysqlDataSourceUtils.QUERY_CONNECTION_IS_VALID_AND_NOT_READONLY;
LOG.info("setting `DataSource` validation query: '" + validationQuery +
"'");
- this.basicDataSource.setValidationQuery(validationQuery);
- this.basicDataSource.setTestOnBorrow(true);
-
this.basicDataSource.setTimeBetweenEvictionRunsMillis(Duration.ofSeconds(60).toMillis());
+ // TODO: revisit following verification of successful connection pool
migration:
+ // If your driver supports JDBC4 we strongly recommend not setting
this property. This is for "legacy" drivers
+ // that do not support the JDBC4 Connection.isValid() API; see:
+ //
https://github.com/brettwooldridge/HikariCP#gear-configuration-knobs-baby
+ this.dataSource.setConnectionTestQuery(validationQuery);
+ this.dataSource.setIdleTimeout(Duration.ofSeconds(60).toMillis());
}
- this.basicDataSource.setUrl(properties.getProperty(CONN_URL));
+ this.dataSource.setJdbcUrl(properties.getProperty(CONN_URL));
+ // TODO: revisit following verification of successful connection pool
migration:
+ // whereas `o.a.commons.dbcp.BasicDataSource` defaults min idle conns to
0, hikari defaults to 10.
+ // perhaps non-zero would have desirable runtime perf, but anything >0
currently fails unit tests (even 1!);
+ // (so experimenting with a higher number would first require adjusting
tests)
+ this.dataSource.setMinimumIdle(0);
if (properties.containsKey(USERNAME) && properties.containsKey(PASSWORD)) {
- this.basicDataSource.setUsername(properties.getProperty(USERNAME));
- this.basicDataSource
+ this.dataSource.setUsername(properties.getProperty(USERNAME));
+ this.dataSource
.setPassword(PasswordManager.getInstance(properties).readPassword(properties.getProperty(PASSWORD)));
}
- if (properties.containsKey(MAX_IDLE_CONNS)) {
-
this.basicDataSource.setMaxIdle(Integer.parseInt(properties.getProperty(MAX_IDLE_CONNS)));
- }
if (properties.containsKey(MAX_ACTIVE_CONNS)) {
-
this.basicDataSource.setMaxActive(Integer.parseInt(properties.getProperty(MAX_ACTIVE_CONNS)));
+
this.dataSource.setMaximumPoolSize(Integer.parseInt(properties.getProperty(MAX_ACTIVE_CONNS)));
}
}
@@ -85,11 +88,11 @@ public class DataSourceProvider implements
Provider<DataSource> {
LOG.warn("Creating {} without setting validation query.\n Stacktrace of
current thread {}",
this.getClass().getSimpleName(),
Arrays.toString(Thread.currentThread().getStackTrace()).replace(", ",
"\n at "));
- this.basicDataSource = new BasicDataSource();
+ this.dataSource = new HikariDataSource();
}
@Override
public DataSource get() {
- return this.basicDataSource;
+ return this.dataSource;
}
}
diff --git a/gradle/scripts/dependencyDefinitions.gradle
b/gradle/scripts/dependencyDefinitions.gradle
index 1ffbae691..b15f76e0c 100644
--- a/gradle/scripts/dependencyDefinitions.gradle
+++ b/gradle/scripts/dependencyDefinitions.gradle
@@ -37,8 +37,6 @@ ext.externalDependency = [
"awsSts": "com.amazonaws:aws-java-sdk-sts:" + awsVersion,
"commonsCli": "commons-cli:commons-cli:1.3.1",
"commonsCodec": "commons-codec:commons-codec:1.10",
- "commonsDbcp": "commons-dbcp:commons-dbcp:1.4",
- "commonsDbcp2": "org.apache.commons:commons-dbcp2:2.9.0",
"commonsEmail": "org.apache.commons:commons-email:1.4",
"commonsLang": "commons-lang:commons-lang:2.6",
"commonsLang3": "org.apache.commons:commons-lang3:3.4",
@@ -70,6 +68,7 @@ ext.externalDependency = [
"hadoopAws": "org.apache.hadoop:hadoop-aws:2.6.0",
"hdrHistogram": "org.hdrhistogram:HdrHistogram:2.1.11",
"helix": "org.apache.helix:helix-core:1.0.2",
+ "hikariCP": "com.zaxxer:HikariCP:3.2.0",
"hiveCommon": "com.linkedin.hive:hive-common:" + hiveVersion,
"hiveService": "com.linkedin.hive:hive-service:" + hiveVersion,
"hiveJdbc": "com.linkedin.hive:hive-jdbc:" + hiveVersion,
