Skip to content

Commit

Permalink
DRILL-7639: Replace DBCP2 with HikariCP in RDBMS (JDBC) plugin
Browse files Browse the repository at this point in the history
  • Loading branch information
@arina-ielchiieva
arina-ielchiieva committed Mar 15, 2020
1 parent 63e64c2 commit 44b1f9a
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 86 deletions.
4 changes: 2 additions & 2 deletions contrib/storage-jdbc/pom.xml
View file
Expand Up @@ -43,8 +43,8 @@
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-dbcp2</artifactId>
<groupId>com.zaxxer</groupId>
<artifactId>HikariCP</artifactId>
</dependency>

<!-- Test dependencies -->
Expand Down
65 changes: 26 additions & 39 deletions contrib/storage-jdbc/src/main/java/org/apache/drill/exec/store/jdbc/JdbcStoragePlugin.java
View file
Expand Up @@ -17,20 +17,17 @@
*/
package org.apache.drill.exec.store.jdbc;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.util.Map;
import javax.sql.DataSource;
import java.util.Properties;
import java.util.Set;

import com.zaxxer.hikari.HikariConfig;
import com.zaxxer.hikari.HikariDataSource;
import org.apache.calcite.adapter.jdbc.JdbcSchema;
import org.apache.calcite.plan.RelOptRule;
import org.apache.calcite.schema.SchemaPlus;
import org.apache.calcite.sql.SqlDialect;
import org.apache.calcite.sql.SqlDialectFactoryImpl;
import org.apache.commons.dbcp2.BasicDataSource;
import org.apache.commons.lang3.StringUtils;
import org.apache.drill.common.AutoCloseables;
import org.apache.drill.common.exceptions.UserException;
import org.apache.drill.exec.ops.OptimizerRulesContext;
Expand All @@ -46,7 +43,7 @@ public class JdbcStoragePlugin extends AbstractStoragePlugin {
private static final Logger logger = LoggerFactory.getLogger(JdbcStoragePlugin.class);

private final JdbcStorageConfig config;
private final BasicDataSource dataSource;
private final HikariDataSource dataSource;
private final SqlDialect dialect;
private final DrillJdbcConvention convention;

Expand Down Expand Up @@ -95,44 +92,34 @@ public void close() {
}

/**
* Initializes {@link BasicDataSource} instance and configures it based on given
* Initializes {@link HikariDataSource} instance and configures it based on given
* storage plugin configuration.
* Basic parameters such as driver, url, user name and password are set using setters.
* Other source parameters are set dynamically by invoking setter based on given parameter name.
* If given parameter is absent, it will be ignored. If value is incorrect
* (for example, String is passed instead of int), data source initialization will fail.
* Parameter names should correspond to names available in documentation:
* <a href="https://commons.apache.org/proper/commons-dbcp/configuration.html">.
* Other source parameters are set dynamically through the properties. See the list
* of available Hikari properties: <a href="https://github.com/brettwooldridge/HikariCP">.
*
* @param config storage plugin config
* @return basic data source instance
* @throws UserException if unable to set source parameter
* @return Hikari data source instance
* @throws UserException if unable to configure Hikari data source
*/
@VisibleForTesting
static BasicDataSource initDataSource(JdbcStorageConfig config) {
BasicDataSource dataSource = new BasicDataSource();
dataSource.setDriverClassName(config.getDriver());
dataSource.setUrl(config.getUrl());
dataSource.setUsername(config.getUsername());
dataSource.setPassword(config.getPassword());

MethodHandles.Lookup publicLookup = MethodHandles.publicLookup();
for (Map.Entry<String, Object> entry : config.getSourceParameters().entrySet()) {
try {
Class<?> parameterType = dataSource.getClass().getDeclaredField(entry.getKey()).getType();
MethodType methodType = MethodType.methodType(void.class, parameterType);
MethodHandle methodHandle = publicLookup.findVirtual(dataSource.getClass(),
"set" + StringUtils.capitalize(entry.getKey()), methodType);
methodHandle.invokeWithArguments(dataSource, entry.getValue());
} catch (ReflectiveOperationException e) {
logger.warn("Unable to find / access setter for parameter {}: {}", entry.getKey(), e.getMessage());
} catch (Throwable e) {
throw UserException.connectionError()
.message("Unable to set value %s for parameter %s", entry.getKey(), entry.getValue())
.addContext("Error message:", e.getMessage())
.build(logger);
}
static HikariDataSource initDataSource(JdbcStorageConfig config) {
try {
Properties properties = new Properties();
properties.putAll(config.getSourceParameters());

HikariConfig hikariConfig = new HikariConfig(properties);

hikariConfig.setDriverClassName(config.getDriver());
hikariConfig.setJdbcUrl(config.getUrl());
hikariConfig.setUsername(config.getUsername());
hikariConfig.setPassword(config.getPassword());

return new HikariDataSource(hikariConfig);
} catch (RuntimeException e) {
throw UserException.connectionError(e)
.message("Unable to configure data source: %s", e.getMessage())
.build(logger);
}
return dataSource;
}
}
79 changes: 46 additions & 33 deletions .../exec/store/jdbc/TestBasicDataSource.java → ...drill/exec/store/jdbc/TestDataSource.java
View file
Expand Up @@ -17,86 +17,99 @@
*/
package org.apache.drill.exec.store.jdbc;

import org.apache.commons.dbcp2.BasicDataSource;
import com.zaxxer.hikari.HikariDataSource;
import org.apache.drill.common.exceptions.UserException;
import org.apache.drill.exec.proto.UserBitShared;
import org.apache.drill.test.BaseDirTestWatcher;
import org.apache.drill.test.BaseTest;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

import java.util.HashMap;
import java.util.Map;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;

public class TestBasicDataSource extends BaseTest {
public class TestDataSource extends BaseTest {

@Rule
public BaseDirTestWatcher dirTestWatcher = new BaseDirTestWatcher();

@Rule
public ExpectedException thrown = ExpectedException.none();

private static final String DRIVER = "org.h2.Driver";

private String url;

@Before
public void init() throws Exception {
url = "jdbc:h2:" + dirTestWatcher.getTmpDir().getCanonicalPath();
}

@Test
public void testInitWithoutUserAndPassword() throws Exception {
public void testInitWithoutUserAndPassword() {
JdbcStorageConfig config = new JdbcStorageConfig(
"driver", "url", null, null, false, null);
try (BasicDataSource dataSource = JdbcStoragePlugin.initDataSource(config)) {
assertEquals("driver", dataSource.getDriverClassName());
assertEquals("url", dataSource.getUrl());
DRIVER, url, null, null, false, null);
try (HikariDataSource dataSource = JdbcStoragePlugin.initDataSource(config)) {
assertEquals(DRIVER, dataSource.getDriverClassName());
assertEquals(url, dataSource.getJdbcUrl());
assertNull(dataSource.getUsername());
assertNull(dataSource.getPassword());
}
}

@Test
public void testInitWithUserAndPassword() throws Exception {
public void testInitWithUserAndPassword() {
JdbcStorageConfig config = new JdbcStorageConfig(
"driver", "url", "user", "password", false, null);
try (BasicDataSource dataSource = JdbcStoragePlugin.initDataSource(config)) {
DRIVER, url, "user", "password", false, null);
try (HikariDataSource dataSource = JdbcStoragePlugin.initDataSource(config)) {
assertEquals("user", dataSource.getUsername());
assertEquals("password", dataSource.getPassword());
}
}

@Test
public void testInitWithSourceParameters() throws Exception {
public void testInitWithSourceParameters() {
Map<String, Object> sourceParameters = new HashMap<>();
sourceParameters.put("maxIdle", 5);
sourceParameters.put("cacheState", false);
sourceParameters.put("validationQuery", "select * from information_schema.collations");
sourceParameters.put("minimumIdle", 5);
sourceParameters.put("autoCommit", false);
sourceParameters.put("connectionTestQuery", "select * from information_schema.collations");
sourceParameters.put("dataSource.cachePrepStmts", true);
sourceParameters.put("dataSource.prepStmtCacheSize", 250);
JdbcStorageConfig config = new JdbcStorageConfig(
"driver", "url", "user", "password", false, sourceParameters);
try (BasicDataSource dataSource = JdbcStoragePlugin.initDataSource(config)) {
assertEquals(5, dataSource.getMaxIdle());
assertFalse(dataSource.getCacheState());
assertEquals("select * from information_schema.collations", dataSource.getValidationQuery());
DRIVER, url, "user", "password", false, sourceParameters);
try (HikariDataSource dataSource = JdbcStoragePlugin.initDataSource(config)) {
assertEquals(5, dataSource.getMinimumIdle());
assertFalse(dataSource.isAutoCommit());
assertEquals("select * from information_schema.collations", dataSource.getConnectionTestQuery());
assertEquals(true, dataSource.getDataSourceProperties().get("cachePrepStmts"));
assertEquals(250, dataSource.getDataSourceProperties().get("prepStmtCacheSize"));
}
}

@Test
public void testInitWithIncorrectSourceParameterName() throws Exception {
public void testInitWithIncorrectSourceParameterName() {
Map<String, Object> sourceParameters = new HashMap<>();
sourceParameters.put("maxIdle", 5);
sourceParameters.put("abc", "abc");
sourceParameters.put("cacheState", false);
sourceParameters.put("validationQuery", null);
JdbcStorageConfig config = new JdbcStorageConfig(
"driver", "url", "user", "password", false, sourceParameters);
try (BasicDataSource dataSource = JdbcStoragePlugin.initDataSource(config)) {
// "abc" parameter will be ignored
assertEquals(5, dataSource.getMaxIdle());
assertFalse(dataSource.getCacheState());
assertNull(dataSource.getValidationQuery());
}
DRIVER, url, "user", "password", false, sourceParameters);

thrown.expect(UserException.class);
thrown.expectMessage(UserBitShared.DrillPBError.ErrorType.CONNECTION.name());

JdbcStoragePlugin.initDataSource(config);
}

@Test
public void testInitWithIncorrectSourceParameterValue() {
Map<String, Object> sourceParameters = new HashMap<>();
sourceParameters.put("maxIdle", "abc");
sourceParameters.put("minimumIdle", "abc");
JdbcStorageConfig config = new JdbcStorageConfig(
"driver", "url", "user", "password", false, sourceParameters);
DRIVER, url, "user", "password", false, sourceParameters);

thrown.expect(UserException.class);
thrown.expectMessage(UserBitShared.DrillPBError.ErrorType.CONNECTION.name());
Expand Down
3 changes: 1 addition & 2 deletions ...b/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcPluginWithH2IT.java
View file
Expand Up @@ -63,8 +63,7 @@ public static void init() throws Exception {
RunScript.execute(connection, fileReader);
}
Map<String, Object> sourceParameters = new HashMap<>();
sourceParameters.put("maxIdle", 5);
sourceParameters.put("maxTotal", 5);
sourceParameters.put("minimumIdle", 1);
JdbcStorageConfig jdbcStorageConfig = new JdbcStorageConfig("org.h2.Driver", connString, "root", "root", true, sourceParameters);
jdbcStorageConfig.setEnabled(true);
cluster.defineStoragePlugin("h2", jdbcStorageConfig);
Expand Down
14 changes: 4 additions & 10 deletions pom.xml
View file
Expand Up @@ -111,7 +111,7 @@
<javax.el.version>3.0.0</javax.el.version>
<surefire.version>3.0.0-M4</surefire.version>
<commons.compress.version>1.19</commons.compress.version>
<commons.dbcp2.version>2.7.0</commons.dbcp2.version>
<hikari.version>3.4.2</hikari.version>
</properties>

<scm>
Expand Down Expand Up @@ -1873,15 +1873,9 @@
<version>${commons.compress.version}</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-dbcp2</artifactId>
<version>${commons.dbcp2.version}</version>
<exclusions>
<exclusion>
<groupId>commons-logging</groupId>
<artifactId>commons-logging</artifactId>
</exclusion>
</exclusions>
<groupId>com.zaxxer</groupId>
<artifactId>HikariCP</artifactId>
<version>${hikari.version}</version>
</dependency>
</dependencies>
</dependencyManagement>
Expand Down

0 comments on commit 44b1f9a

Please sign in to comment.