You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gobblin.apache.org by zi...@apache.org on 2022/05/26 17:11:05 UTC

[gobblin] branch master updated: [GOBBLIN-1648] Complete use of JDBC `DataSource` 'read-only' validation query by incorporating where previously omitted (#3509)

This is an automated email from the ASF dual-hosted git repository.

zihanli58 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 fcb477f16 [GOBBLIN-1648] Complete use of JDBC `DataSource` 'read-only' validation query by incorporating where previously omitted (#3509)
fcb477f16 is described below

commit fcb477f16bce98bdc4445edfbad89337315a014c
Author: Kip Kohn <ck...@linkedin.com>
AuthorDate: Thu May 26 10:11:00 2022 -0700

    [GOBBLIN-1648] Complete use of JDBC `DataSource` 'read-only' validation query by incorporating where previously omitted (#3509)
    
    * Complete use of JDBC `DataSource` 'read-only' validation query by incorporating where previously omitted
    
    * Add logging to demonstrate validation query successfully configured
---
 .../org/apache/gobblin/metastore/MysqlStateStore.java     | 13 ++++++++++---
 .../service/modules/db/ServiceDatabaseProviderImpl.java   | 12 +++++++++---
 .../org/apache/gobblin/util/jdbc/DataSourceProvider.java  | 15 +++++++++++++++
 .../apache/gobblin/util/jdbc}/MysqlDataSourceUtils.java   |  2 +-
 4 files changed, 35 insertions(+), 7 deletions(-)

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 d5a203b96..f75cddc5a 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
@@ -32,12 +32,16 @@ import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Timestamp;
+import java.time.Duration;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.zip.GZIPInputStream;
 import java.util.zip.GZIPOutputStream;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import org.apache.commons.dbcp.BasicDataSource;
 import org.apache.hadoop.io.Text;
 
@@ -54,10 +58,10 @@ import org.apache.gobblin.configuration.State;
 import org.apache.gobblin.metastore.metadata.StateStoreEntryManager;
 import org.apache.gobblin.metastore.predicates.StateStorePredicate;
 import org.apache.gobblin.metastore.predicates.StoreNamePredicate;
-import org.apache.gobblin.metastore.util.MysqlDataSourceUtils;
 import org.apache.gobblin.password.PasswordManager;
 import org.apache.gobblin.util.ConfigUtils;
 import org.apache.gobblin.util.io.StreamUtils;
+import org.apache.gobblin.util.jdbc.MysqlDataSourceUtils;
 
 /**
  * An implementation of {@link StateStore} backed by MySQL.
@@ -75,6 +79,7 @@ import org.apache.gobblin.util.io.StreamUtils;
  * @param <T> state object type
  **/
 public class MysqlStateStore<T extends State> implements StateStore<T> {
+  private static final Logger LOG = LoggerFactory.getLogger(MysqlStateStore.class);
 
   /** Specifies which 'Job State' query columns receive search evaluation (with SQL `LIKE` operator). */
   protected enum JobStateSearchColumns {
@@ -200,10 +205,12 @@ public class MysqlStateStore<T extends State> implements StateStore<T> {
     basicDataSource.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
-    basicDataSource.setValidationQuery(MysqlDataSourceUtils.QUERY_CONNECTION_IS_VALID_AND_NOT_READONLY);
+    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(60000);
+    basicDataSource.setTimeBetweenEvictionRunsMillis(Duration.ofSeconds(60).toMillis());
     basicDataSource.setUrl(config.getString(ConfigurationKeys.STATE_STORE_DB_URL_KEY));
     basicDataSource.setUsername(passwordManager.readPassword(
         config.getString(ConfigurationKeys.STATE_STORE_DB_USER_KEY)));
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 d36a81ece..b9c2d32e4 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
@@ -20,6 +20,9 @@ package org.apache.gobblin.service.modules.db;
 import java.time.Duration;
 import java.util.Objects;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import org.apache.commons.dbcp2.BasicDataSource;
 
 import com.typesafe.config.Config;
@@ -31,13 +34,14 @@ import lombok.Builder;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
 
-import org.apache.gobblin.metastore.util.MysqlDataSourceUtils;
+import org.apache.gobblin.util.jdbc.MysqlDataSourceUtils;
 import org.apache.gobblin.password.PasswordManager;
 import org.apache.gobblin.service.ServiceConfigKeys;
 import org.apache.gobblin.util.ConfigUtils;
 
 
 public class ServiceDatabaseProviderImpl implements ServiceDatabaseProvider {
+  private static final Logger LOG = LoggerFactory.getLogger(ServiceDatabaseProviderImpl.class);
 
   private final Configuration configuration;
   private BasicDataSource dataSource;
@@ -63,8 +67,10 @@ public class ServiceDatabaseProviderImpl implements ServiceDatabaseProvider {
     dataSource.setUsername(configuration.getUserName());
     dataSource.setPassword(configuration.getPassword());
 
-    // MySQL server can timeout a connection so we need to validate connections.
-    dataSource.setValidationQuery(MysqlDataSourceUtils.QUERY_CONNECTION_IS_VALID_AND_NOT_READONLY);
+    // 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
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 1d67136dd..21c94579f 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
@@ -17,10 +17,14 @@
 
 package org.apache.gobblin.util.jdbc;
 
+import java.time.Duration;
 import java.util.Properties;
 
 import javax.sql.DataSource;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import org.apache.commons.dbcp.BasicDataSource;
 
 import com.google.inject.Inject;
@@ -34,6 +38,7 @@ import org.apache.gobblin.password.PasswordManager;
  * A provider class for {@link javax.sql.DataSource}s.
  */
 public class DataSourceProvider implements Provider<DataSource> {
+  private static final Logger LOG = LoggerFactory.getLogger(DataSourceProvider.class);
 
   public static final String GOBBLIN_UTIL_JDBC_PREFIX = "gobblin.util.jdbc.";
   public static final String CONN_DRIVER = GOBBLIN_UTIL_JDBC_PREFIX + "conn.driver";
@@ -41,6 +46,7 @@ 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";
@@ -51,6 +57,15 @@ public class DataSourceProvider implements Provider<DataSource> {
   public DataSourceProvider(@Named("dataSourceProperties") Properties properties) {
     this.basicDataSource = new BasicDataSource();
     this.basicDataSource.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());
+    }
     this.basicDataSource.setUrl(properties.getProperty(CONN_URL));
     if (properties.containsKey(USERNAME) && properties.containsKey(PASSWORD)) {
       this.basicDataSource.setUsername(properties.getProperty(USERNAME));
diff --git a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/util/MysqlDataSourceUtils.java b/gobblin-utility/src/main/java/org/apache/gobblin/util/jdbc/MysqlDataSourceUtils.java
similarity index 97%
rename from gobblin-metastore/src/main/java/org/apache/gobblin/metastore/util/MysqlDataSourceUtils.java
rename to gobblin-utility/src/main/java/org/apache/gobblin/util/jdbc/MysqlDataSourceUtils.java
index 1c141a769..844f9272e 100644
--- a/gobblin-metastore/src/main/java/org/apache/gobblin/metastore/util/MysqlDataSourceUtils.java
+++ b/gobblin-utility/src/main/java/org/apache/gobblin/util/jdbc/MysqlDataSourceUtils.java
@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 
-package org.apache.gobblin.metastore.util;
+package org.apache.gobblin.util.jdbc;
 
 public final class MysqlDataSourceUtils {
   /**