You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2021/12/25 15:32:06 UTC

[GitHub] [dolphinscheduler] Narcasserun opened a new pull request #7624: [feature-7623][plugin] Refactored data source plug-in loading

Narcasserun opened a new pull request #7624:
URL: https://github.com/apache/dolphinscheduler/pull/7624


   - Support kerberos, hadoop multi-cluster environment
   - Hadoop, hive dependent classpath isolation to avoid dependency conflicts
   - hive,mysql,sqlserver plugin packag


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [dolphinscheduler] KonsonChow commented on a change in pull request #7624: [feature-7623][plugin] Refactored data source plug-in loading

Posted by GitBox <gi...@apache.org>.
KonsonChow commented on a change in pull request #7624:
URL: https://github.com/apache/dolphinscheduler/pull/7624#discussion_r790273465



##########
File path: dolphinscheduler-datasource-plugin/dolphinscheduler-datasource-api/src/main/java/org/apache/dolphinscheduler/plugin/datasource/api/plugin/DataSourceClientProvider.java
##########
@@ -17,57 +17,153 @@
 
 package org.apache.dolphinscheduler.plugin.datasource.api.plugin;
 
-import org.apache.dolphinscheduler.plugin.datasource.api.utils.DataSourceUtils;
-import org.apache.dolphinscheduler.spi.datasource.BaseConnectionParam;
-import org.apache.dolphinscheduler.spi.datasource.ConnectionParam;
-import org.apache.dolphinscheduler.spi.datasource.DataSourceChannel;
+import org.apache.dolphinscheduler.plugin.datasource.api.exception.DataSourceException;
+import org.apache.dolphinscheduler.plugin.datasource.api.utils.ClassLoaderUtils;
+import org.apache.dolphinscheduler.plugin.datasource.api.utils.ThreadContextClassLoader;
+import org.apache.dolphinscheduler.spi.datasource.DataSourceChannelFactory;
 import org.apache.dolphinscheduler.spi.datasource.DataSourceClient;
+import org.apache.dolphinscheduler.spi.datasource.JdbcConnectionParam;
 import org.apache.dolphinscheduler.spi.enums.DbType;
+import org.apache.dolphinscheduler.spi.utils.StringUtils;
 
-import java.sql.Connection;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
+import java.io.FilenameFilter;
+import java.net.MalformedURLException;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.sql.Driver;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.ServiceLoader;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Sets;
+
 public class DataSourceClientProvider {
     private static final Logger logger = LoggerFactory.getLogger(DataSourceClientProvider.class);
 
-    private static final Map<String, DataSourceClient> uniqueId2dataSourceClientMap = new ConcurrentHashMap<>();
+    private static final JdbcDriverManager jdbcDriverManagerInstance = JdbcDriverManager.getInstance();
 
-    private DataSourcePluginManager dataSourcePluginManager;
+    public static DataSourceClient createDataSourceClient(JdbcConnectionParam connectionParam) {
+        logger.info("Creating the createDataSourceClient. JdbcUrl: {} ", connectionParam.getJdbcUrl());
 
-    private DataSourceClientProvider() {
-        initDataSourcePlugin();
-    }
+        //Check jdbc driver location
+        checkDriverLocation(connectionParam);
+
+        logger.info("Creating the ClassLoader for the jdbc driver and plugin.");
+        ClassLoader driverClassLoader = getDriverClassLoader(connectionParam);
 
-    private static class DataSourceClientProviderHolder {
-        private static final DataSourceClientProvider INSTANCE = new DataSourceClientProvider();
+        try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(driverClassLoader)) {
+            return createDataSourceClientWithClassLoader(connectionParam);
+        }
     }
 
-    public static DataSourceClientProvider getInstance() {
-        return DataSourceClientProviderHolder.INSTANCE;
+    protected static void checkDriverLocation(JdbcConnectionParam connectionParam) {
+        final String driverLocation = connectionParam.getDriverLocation();
+        if (StringUtils.isBlank(driverLocation)) {
+            logger.warn("No jdbc driver provide,will use randomly driver jar for {}.", connectionParam.getDbType().getDescp());
+            connectionParam.setDriverLocation(jdbcDriverManagerInstance.getDefaultDriverPluginPath(connectionParam.getDbType().getDescp()));
+        }
     }
 
-    public Connection getConnection(DbType dbType, ConnectionParam connectionParam) {
-        BaseConnectionParam baseConnectionParam = (BaseConnectionParam) connectionParam;
-        String datasourceUniqueId = DataSourceUtils.getDatasourceUniqueId(baseConnectionParam, dbType);
-        logger.info("getConnection datasourceUniqueId {}", datasourceUniqueId);
+    protected static ClassLoader getDriverClassLoader(JdbcConnectionParam connectionParam) {
+        FilenameFilter filenameFilter = (dir, name) -> name != null && name.endsWith(".jar");
+        ClassLoader threadClassLoader = Thread.currentThread().getContextClassLoader();
+        ClassLoader classLoader;
+
+        HashSet<String> paths = Sets.newHashSet();
+        if (StringUtils.isNotBlank(connectionParam.getDriverLocation())) {
+            logger.info("Driver location: {}", connectionParam.getDriverLocation());
+            paths.add(connectionParam.getDriverLocation());
+        }
+        try {
+            classLoader = ClassLoaderUtils.getCustomClassLoader(paths, threadClassLoader, filenameFilter);
+        } catch (final MalformedURLException e) {
+            throw DataSourceException.getInstance("Invalid jdbc driver location.", e);
+        }
+
+        //try loading jdbc driver
+        loadJdbcDriver(classLoader, connectionParam);
+
+        DbType dbType = connectionParam.getDbType();
+        String pluginPath = JdbcDriverManager.getInstance().getPluginPath(dbType);
+        logger.info("Plugin location: {}", pluginPath);
+        paths.add(pluginPath);
+
+        if (dbType == DbType.HIVE || dbType == DbType.SPARK) {
+            try {
+                Class.forName("org.apache.hadoop.conf.Configuration", true, classLoader);
+                Class.forName("org.apache.hadoop.security.UserGroupInformation", true, classLoader);
+                Class.forName("org.apache.hadoop.fs.FileSystem", true, classLoader);

Review comment:
       org.apache.dolphinscheduler.common.utils.HadoopUtils introduces Hadoop related classes, and PluginClassLoader does not override the findClass method, then Can dependencies be loaded correctly here?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [dolphinscheduler] Narcasserun commented on a change in pull request #7624: [feature-7623][plugin] Refactored data source plug-in loading

Posted by GitBox <gi...@apache.org>.
Narcasserun commented on a change in pull request #7624:
URL: https://github.com/apache/dolphinscheduler/pull/7624#discussion_r775761409



##########
File path: dolphinscheduler-datasource-plugin/dolphinscheduler-datasource-api/src/main/java/org/apache/dolphinscheduler/plugin/datasource/api/plugin/DataSourceClientProvider.java
##########
@@ -17,57 +17,153 @@
 
 package org.apache.dolphinscheduler.plugin.datasource.api.plugin;
 
-import org.apache.dolphinscheduler.plugin.datasource.api.utils.DataSourceUtils;
-import org.apache.dolphinscheduler.spi.datasource.BaseConnectionParam;
-import org.apache.dolphinscheduler.spi.datasource.ConnectionParam;
-import org.apache.dolphinscheduler.spi.datasource.DataSourceChannel;
+import org.apache.dolphinscheduler.plugin.datasource.api.exception.DataSourceException;
+import org.apache.dolphinscheduler.plugin.datasource.api.utils.ClassLoaderUtils;
+import org.apache.dolphinscheduler.plugin.datasource.api.utils.ThreadContextClassLoader;
+import org.apache.dolphinscheduler.spi.datasource.DataSourceChannelFactory;
 import org.apache.dolphinscheduler.spi.datasource.DataSourceClient;
+import org.apache.dolphinscheduler.spi.datasource.JdbcConnectionParam;
 import org.apache.dolphinscheduler.spi.enums.DbType;
+import org.apache.dolphinscheduler.spi.utils.StringUtils;
 
-import java.sql.Connection;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
+import java.io.FilenameFilter;
+import java.net.MalformedURLException;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.sql.Driver;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.ServiceLoader;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Sets;
+
 public class DataSourceClientProvider {
     private static final Logger logger = LoggerFactory.getLogger(DataSourceClientProvider.class);
 
-    private static final Map<String, DataSourceClient> uniqueId2dataSourceClientMap = new ConcurrentHashMap<>();
+    private static final JdbcDriverManager jdbcDriverManagerInstance = JdbcDriverManager.getInstance();
 
-    private DataSourcePluginManager dataSourcePluginManager;
+    public static DataSourceClient createDataSourceClient(JdbcConnectionParam connectionParam) {
+        logger.info("Creating the createDataSourceClient. JdbcUrl: {} ", connectionParam.getJdbcUrl());
 
-    private DataSourceClientProvider() {
-        initDataSourcePlugin();
-    }
+        //Check jdbc driver location
+        checkDriverLocation(connectionParam);
+
+        logger.info("Creating the ClassLoader for the jdbc driver and plugin.");
+        ClassLoader driverClassLoader = getDriverClassLoader(connectionParam);
 
-    private static class DataSourceClientProviderHolder {
-        private static final DataSourceClientProvider INSTANCE = new DataSourceClientProvider();
+        try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(driverClassLoader)) {
+            return createDataSourceClientWithClassLoader(connectionParam);
+        }
     }
 
-    public static DataSourceClientProvider getInstance() {
-        return DataSourceClientProviderHolder.INSTANCE;
+    protected static void checkDriverLocation(JdbcConnectionParam connectionParam) {
+        final String driverLocation = connectionParam.getDriverLocation();
+        if (StringUtils.isBlank(driverLocation)) {
+            logger.warn("No jdbc driver provide,will use randomly driver jar for {}.", connectionParam.getDbType().getDescp());
+            connectionParam.setDriverLocation(jdbcDriverManagerInstance.getDefaultDriverPluginPath(connectionParam.getDbType().getDescp()));
+        }
     }
 
-    public Connection getConnection(DbType dbType, ConnectionParam connectionParam) {
-        BaseConnectionParam baseConnectionParam = (BaseConnectionParam) connectionParam;
-        String datasourceUniqueId = DataSourceUtils.getDatasourceUniqueId(baseConnectionParam, dbType);
-        logger.info("getConnection datasourceUniqueId {}", datasourceUniqueId);
+    protected static ClassLoader getDriverClassLoader(JdbcConnectionParam connectionParam) {
+        FilenameFilter filenameFilter = (dir, name) -> name != null && name.endsWith(".jar");
+        ClassLoader threadClassLoader = Thread.currentThread().getContextClassLoader();
+        ClassLoader classLoader;
+
+        HashSet<String> paths = Sets.newHashSet();
+        if (StringUtils.isNotBlank(connectionParam.getDriverLocation())) {
+            logger.info("Driver location: {}", connectionParam.getDriverLocation());
+            paths.add(connectionParam.getDriverLocation());
+        }
+        try {
+            classLoader = ClassLoaderUtils.getCustomClassLoader(paths, threadClassLoader, filenameFilter);
+        } catch (final MalformedURLException e) {
+            throw DataSourceException.getInstance("Invalid jdbc driver location.", e);
+        }
+
+        //try loading jdbc driver
+        loadJdbcDriver(classLoader, connectionParam);
+
+        DbType dbType = connectionParam.getDbType();
+        String pluginPath = JdbcDriverManager.getInstance().getPluginPath(dbType);
+        logger.info("Plugin location: {}", pluginPath);
+        paths.add(pluginPath);
+
+        if (dbType == DbType.HIVE || dbType == DbType.SPARK) {
+            try {
+                Class.forName("org.apache.hadoop.conf.Configuration", true, classLoader);
+                Class.forName("org.apache.hadoop.security.UserGroupInformation", true, classLoader);
+                Class.forName("org.apache.hadoop.fs.FileSystem", true, classLoader);
+            } catch (ClassNotFoundException cnf) {
+                cnf.printStackTrace();

Review comment:
       yes, I'll revise it




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [dolphinscheduler] codecov-commenter edited a comment on pull request #7624: [feature-7623][plugin] Refactored data source plug-in loading

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7624:
URL: https://github.com/apache/dolphinscheduler/pull/7624#issuecomment-1001158869


   # [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/7624?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#7624](https://codecov.io/gh/apache/dolphinscheduler/pull/7624?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61d1fd3) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/0d861fe46af595156342b56ef2edf3a2f24a0e4c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0d861fe) will **decrease** coverage by `0.48%`.
   > The diff coverage is `5.39%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/graphs/tree.svg?width=650&height=150&src=pr&token=bv9iXXRLi9&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/dolphinscheduler/pull/7624?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##                dev    #7624      +/-   ##
   ============================================
   - Coverage     41.22%   40.74%   -0.49%     
   + Complexity     3671     3520     -151     
   ============================================
     Files           641      615      -26     
     Lines         26539    26012     -527     
     Branches       2967     2948      -19     
   ============================================
   - Hits          10941    10598     -343     
   + Misses        14569    14413     -156     
   + Partials       1029     1001      -28     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/7624?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...scheduler/api/controller/DataSourceController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvRGF0YVNvdXJjZUNvbnRyb2xsZXIuamF2YQ==) | `52.17% <0.00%> (+2.17%)` | :arrow_up: |
   | [...pache/dolphinscheduler/common/utils/JSONUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3V0aWxzL0pTT05VdGlscy5qYXZh) | `72.41% <0.00%> (-0.85%)` | :arrow_down: |
   | [.../datasource/api/client/CommonDataSourceClient.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9kYXRhc291cmNlL2FwaS9jbGllbnQvQ29tbW9uRGF0YVNvdXJjZUNsaWVudC5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [.../datasource/api/exception/DataSourceException.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9kYXRhc291cmNlL2FwaS9leGNlcHRpb24vRGF0YVNvdXJjZUV4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...atasource/api/plugin/DataSourceClientProvider.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9kYXRhc291cmNlL2FwaS9wbHVnaW4vRGF0YVNvdXJjZUNsaWVudFByb3ZpZGVyLmphdmE=) | `0.00% <0.00%> (-10.00%)` | :arrow_down: |
   | [...lugin/datasource/api/plugin/JdbcDriverManager.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9kYXRhc291cmNlL2FwaS9wbHVnaW4vSmRiY0RyaXZlck1hbmFnZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [.../plugin/datasource/api/utils/ClassLoaderUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9kYXRhc291cmNlL2FwaS91dGlscy9DbGFzc0xvYWRlclV0aWxzLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...plugin/datasource/api/utils/PluginClassLoader.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9kYXRhc291cmNlL2FwaS91dGlscy9QbHVnaW5DbGFzc0xvYWRlci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...datasource/api/utils/ThreadContextClassLoader.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9kYXRhc291cmNlL2FwaS91dGlscy9UaHJlYWRDb250ZXh0Q2xhc3NMb2FkZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...source/clickhouse/ClickHouseDataSourceChannel.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtY2xpY2tob3VzZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9wbHVnaW4vZGF0YXNvdXJjZS9jbGlja2hvdXNlL0NsaWNrSG91c2VEYXRhU291cmNlQ2hhbm5lbC5qYXZh) | `50.00% <0.00%> (ø)` | |
   | ... and [27 more](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/7624?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/7624?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [0d861fe...61d1fd3](https://codecov.io/gh/apache/dolphinscheduler/pull/7624?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #7624: [feature-7623][plugin] Refactored data source plug-in loading

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #7624:
URL: https://github.com/apache/dolphinscheduler/pull/7624#issuecomment-1001159483


   SonarCloud Quality Gate failed.&nbsp; &nbsp; ![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=SECURITY_HOTSPOT) [![E](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E-16px.png 'E')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=SECURITY_HOTSPOT) [2 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=CODE_SMELL) [19 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=CODE_SMELL)
   
   [![5.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '5.5%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=7624&metric=new_coverage&view=list) [5.5% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=7624&metric=new_coverage&view=list)  
   [![2.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '2.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=7624&metric=new_duplicated_lines_density&view=list) [2.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=7624&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [dolphinscheduler] KonsonChow commented on a change in pull request #7624: [feature-7623][plugin] Refactored data source plug-in loading

Posted by GitBox <gi...@apache.org>.
KonsonChow commented on a change in pull request #7624:
URL: https://github.com/apache/dolphinscheduler/pull/7624#discussion_r790273465



##########
File path: dolphinscheduler-datasource-plugin/dolphinscheduler-datasource-api/src/main/java/org/apache/dolphinscheduler/plugin/datasource/api/plugin/DataSourceClientProvider.java
##########
@@ -17,57 +17,153 @@
 
 package org.apache.dolphinscheduler.plugin.datasource.api.plugin;
 
-import org.apache.dolphinscheduler.plugin.datasource.api.utils.DataSourceUtils;
-import org.apache.dolphinscheduler.spi.datasource.BaseConnectionParam;
-import org.apache.dolphinscheduler.spi.datasource.ConnectionParam;
-import org.apache.dolphinscheduler.spi.datasource.DataSourceChannel;
+import org.apache.dolphinscheduler.plugin.datasource.api.exception.DataSourceException;
+import org.apache.dolphinscheduler.plugin.datasource.api.utils.ClassLoaderUtils;
+import org.apache.dolphinscheduler.plugin.datasource.api.utils.ThreadContextClassLoader;
+import org.apache.dolphinscheduler.spi.datasource.DataSourceChannelFactory;
 import org.apache.dolphinscheduler.spi.datasource.DataSourceClient;
+import org.apache.dolphinscheduler.spi.datasource.JdbcConnectionParam;
 import org.apache.dolphinscheduler.spi.enums.DbType;
+import org.apache.dolphinscheduler.spi.utils.StringUtils;
 
-import java.sql.Connection;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
+import java.io.FilenameFilter;
+import java.net.MalformedURLException;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.sql.Driver;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.ServiceLoader;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Sets;
+
 public class DataSourceClientProvider {
     private static final Logger logger = LoggerFactory.getLogger(DataSourceClientProvider.class);
 
-    private static final Map<String, DataSourceClient> uniqueId2dataSourceClientMap = new ConcurrentHashMap<>();
+    private static final JdbcDriverManager jdbcDriverManagerInstance = JdbcDriverManager.getInstance();
 
-    private DataSourcePluginManager dataSourcePluginManager;
+    public static DataSourceClient createDataSourceClient(JdbcConnectionParam connectionParam) {
+        logger.info("Creating the createDataSourceClient. JdbcUrl: {} ", connectionParam.getJdbcUrl());
 
-    private DataSourceClientProvider() {
-        initDataSourcePlugin();
-    }
+        //Check jdbc driver location
+        checkDriverLocation(connectionParam);
+
+        logger.info("Creating the ClassLoader for the jdbc driver and plugin.");
+        ClassLoader driverClassLoader = getDriverClassLoader(connectionParam);
 
-    private static class DataSourceClientProviderHolder {
-        private static final DataSourceClientProvider INSTANCE = new DataSourceClientProvider();
+        try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(driverClassLoader)) {
+            return createDataSourceClientWithClassLoader(connectionParam);
+        }
     }
 
-    public static DataSourceClientProvider getInstance() {
-        return DataSourceClientProviderHolder.INSTANCE;
+    protected static void checkDriverLocation(JdbcConnectionParam connectionParam) {
+        final String driverLocation = connectionParam.getDriverLocation();
+        if (StringUtils.isBlank(driverLocation)) {
+            logger.warn("No jdbc driver provide,will use randomly driver jar for {}.", connectionParam.getDbType().getDescp());
+            connectionParam.setDriverLocation(jdbcDriverManagerInstance.getDefaultDriverPluginPath(connectionParam.getDbType().getDescp()));
+        }
     }
 
-    public Connection getConnection(DbType dbType, ConnectionParam connectionParam) {
-        BaseConnectionParam baseConnectionParam = (BaseConnectionParam) connectionParam;
-        String datasourceUniqueId = DataSourceUtils.getDatasourceUniqueId(baseConnectionParam, dbType);
-        logger.info("getConnection datasourceUniqueId {}", datasourceUniqueId);
+    protected static ClassLoader getDriverClassLoader(JdbcConnectionParam connectionParam) {
+        FilenameFilter filenameFilter = (dir, name) -> name != null && name.endsWith(".jar");
+        ClassLoader threadClassLoader = Thread.currentThread().getContextClassLoader();
+        ClassLoader classLoader;
+
+        HashSet<String> paths = Sets.newHashSet();
+        if (StringUtils.isNotBlank(connectionParam.getDriverLocation())) {
+            logger.info("Driver location: {}", connectionParam.getDriverLocation());
+            paths.add(connectionParam.getDriverLocation());
+        }
+        try {
+            classLoader = ClassLoaderUtils.getCustomClassLoader(paths, threadClassLoader, filenameFilter);
+        } catch (final MalformedURLException e) {
+            throw DataSourceException.getInstance("Invalid jdbc driver location.", e);
+        }
+
+        //try loading jdbc driver
+        loadJdbcDriver(classLoader, connectionParam);
+
+        DbType dbType = connectionParam.getDbType();
+        String pluginPath = JdbcDriverManager.getInstance().getPluginPath(dbType);
+        logger.info("Plugin location: {}", pluginPath);
+        paths.add(pluginPath);
+
+        if (dbType == DbType.HIVE || dbType == DbType.SPARK) {
+            try {
+                Class.forName("org.apache.hadoop.conf.Configuration", true, classLoader);
+                Class.forName("org.apache.hadoop.security.UserGroupInformation", true, classLoader);
+                Class.forName("org.apache.hadoop.fs.FileSystem", true, classLoader);

Review comment:
       org.apache.dolphinscheduler.common.utils.HadoopUtils introduces Hadoop related classes, and PluginClassLoader does not override the findClass method. Can dependencies be loaded correctly here?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #7624: [feature-7623][plugin] Refactored data source plug-in loading

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #7624:
URL: https://github.com/apache/dolphinscheduler/pull/7624#issuecomment-1001162088


   SonarCloud Quality Gate failed.&nbsp; &nbsp; ![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=SECURITY_HOTSPOT) [![E](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E-16px.png 'E')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=SECURITY_HOTSPOT) [2 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=CODE_SMELL) [19 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=CODE_SMELL)
   
   [![5.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '5.5%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=7624&metric=new_coverage&view=list) [5.5% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=7624&metric=new_coverage&view=list)  
   [![2.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '2.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=7624&metric=new_duplicated_lines_density&view=list) [2.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=7624&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [dolphinscheduler] kezhenxu94 commented on pull request #7624: [feature-7623][plugin] Refactored data source plug-in loading

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on pull request #7624:
URL: https://github.com/apache/dolphinscheduler/pull/7624#issuecomment-1002906975


   Looks like the PMC/committers don't have a consensus in https://github.com/apache/dolphinscheduler/issues/7623 yet?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [dolphinscheduler] sonarcloud[bot] removed a comment on pull request #7624: [feature-7623][plugin] Refactored data source plug-in loading

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #7624:
URL: https://github.com/apache/dolphinscheduler/pull/7624#issuecomment-1001159483


   SonarCloud Quality Gate failed.&nbsp; &nbsp; ![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=SECURITY_HOTSPOT) [![E](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E-16px.png 'E')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=SECURITY_HOTSPOT) [2 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=CODE_SMELL) [19 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=7624&resolved=false&types=CODE_SMELL)
   
   [![5.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '5.5%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=7624&metric=new_coverage&view=list) [5.5% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=7624&metric=new_coverage&view=list)  
   [![2.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '2.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=7624&metric=new_duplicated_lines_density&view=list) [2.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=7624&metric=new_duplicated_lines_density&view=list)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [dolphinscheduler] Narcasserun commented on a change in pull request #7624: [feature-7623][plugin] Refactored data source plug-in loading

Posted by GitBox <gi...@apache.org>.
Narcasserun commented on a change in pull request #7624:
URL: https://github.com/apache/dolphinscheduler/pull/7624#discussion_r775761898



##########
File path: dolphinscheduler-datasource-plugin/dolphinscheduler-datasource-api/src/main/java/org/apache/dolphinscheduler/plugin/datasource/api/utils/ThreadContextClassLoader.java
##########
@@ -15,33 +15,21 @@
  * limitations under the License.
  */
 
-package org.apache.dolphinscheduler.common.utils;
+package org.apache.dolphinscheduler.plugin.datasource.api.utils;
 
-import org.junit.Assert;
-import org.junit.Test;
+import java.io.Closeable;
 
-/**
- * hive conf utils test
- */
-public class HiveConfUtilsTest {
-
-    /**
-     * test is hive conf var
-     */
-    @Test
-    public void testIsHiveConfVar() {
-
-        String conf = "hive.exec.script.wrapper=123";
-        boolean hiveConfVar = HiveConfUtils.isHiveConfVar(conf);
-        Assert.assertTrue(hiveConfVar);
+public class ThreadContextClassLoader
+        implements Closeable {
+    private final ClassLoader threadContextClassLoader;
 
-        conf = "hive.test.v1=v1";
-        hiveConfVar = HiveConfUtils.isHiveConfVar(conf);
-        Assert.assertFalse(hiveConfVar);
-
-        conf = "tez.queue.name=tezQueue";
-        hiveConfVar = HiveConfUtils.isHiveConfVar(conf);
-        Assert.assertTrue(hiveConfVar);
+    public ThreadContextClassLoader(ClassLoader newThreadContextClassLoader) {

Review comment:
       It will be set when creating a data source




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [dolphinscheduler] Narcasserun commented on pull request #7624: [feature-7623][plugin] Refactored data source plug-in loading

Posted by GitBox <gi...@apache.org>.
Narcasserun commented on pull request #7624:
URL: https://github.com/apache/dolphinscheduler/pull/7624#issuecomment-1001909572


   > It's a big pr, thanks for your great job! BTW, I think it should change the docs of plug-in loading too.
   
   After completing the code, I will supplement the documentation


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #7624: [feature-7623][plugin] Refactored data source plug-in loading

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7624:
URL: https://github.com/apache/dolphinscheduler/pull/7624#issuecomment-1001158869


   # [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/7624?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#7624](https://codecov.io/gh/apache/dolphinscheduler/pull/7624?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (36a3f00) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/0d861fe46af595156342b56ef2edf3a2f24a0e4c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0d861fe) will **decrease** coverage by `0.45%`.
   > The diff coverage is `5.39%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/graphs/tree.svg?width=650&height=150&src=pr&token=bv9iXXRLi9&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/dolphinscheduler/pull/7624?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##                dev    #7624      +/-   ##
   ============================================
   - Coverage     41.22%   40.77%   -0.46%     
   + Complexity     3671     3520     -151     
   ============================================
     Files           641      615      -26     
     Lines         26539    26012     -527     
     Branches       2967     2948      -19     
   ============================================
   - Hits          10941    10606     -335     
   + Misses        14569    14406     -163     
   + Partials       1029     1000      -29     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/7624?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...scheduler/api/controller/DataSourceController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvRGF0YVNvdXJjZUNvbnRyb2xsZXIuamF2YQ==) | `52.17% <0.00%> (+2.17%)` | :arrow_up: |
   | [...pache/dolphinscheduler/common/utils/JSONUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3V0aWxzL0pTT05VdGlscy5qYXZh) | `72.41% <0.00%> (-0.85%)` | :arrow_down: |
   | [.../datasource/api/client/CommonDataSourceClient.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9kYXRhc291cmNlL2FwaS9jbGllbnQvQ29tbW9uRGF0YVNvdXJjZUNsaWVudC5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [.../datasource/api/exception/DataSourceException.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9kYXRhc291cmNlL2FwaS9leGNlcHRpb24vRGF0YVNvdXJjZUV4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...atasource/api/plugin/DataSourceClientProvider.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9kYXRhc291cmNlL2FwaS9wbHVnaW4vRGF0YVNvdXJjZUNsaWVudFByb3ZpZGVyLmphdmE=) | `0.00% <0.00%> (-10.00%)` | :arrow_down: |
   | [...lugin/datasource/api/plugin/JdbcDriverManager.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9kYXRhc291cmNlL2FwaS9wbHVnaW4vSmRiY0RyaXZlck1hbmFnZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [.../plugin/datasource/api/utils/ClassLoaderUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9kYXRhc291cmNlL2FwaS91dGlscy9DbGFzc0xvYWRlclV0aWxzLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...plugin/datasource/api/utils/PluginClassLoader.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9kYXRhc291cmNlL2FwaS91dGlscy9QbHVnaW5DbGFzc0xvYWRlci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...datasource/api/utils/ThreadContextClassLoader.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3BsdWdpbi9kYXRhc291cmNlL2FwaS91dGlscy9UaHJlYWRDb250ZXh0Q2xhc3NMb2FkZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...source/clickhouse/ClickHouseDataSourceChannel.java](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1kYXRhc291cmNlLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLWRhdGFzb3VyY2UtY2xpY2tob3VzZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9wbHVnaW4vZGF0YXNvdXJjZS9jbGlja2hvdXNlL0NsaWNrSG91c2VEYXRhU291cmNlQ2hhbm5lbC5qYXZh) | `50.00% <0.00%> (ø)` | |
   | ... and [30 more](https://codecov.io/gh/apache/dolphinscheduler/pull/7624/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/7624?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/7624?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [0d861fe...36a3f00](https://codecov.io/gh/apache/dolphinscheduler/pull/7624?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [dolphinscheduler] caishunfeng commented on a change in pull request #7624: [feature-7623][plugin] Refactored data source plug-in loading

Posted by GitBox <gi...@apache.org>.
caishunfeng commented on a change in pull request #7624:
URL: https://github.com/apache/dolphinscheduler/pull/7624#discussion_r775241949



##########
File path: dolphinscheduler-datasource-plugin/dolphinscheduler-datasource-api/src/main/java/org/apache/dolphinscheduler/plugin/datasource/api/plugin/DataSourceClientProvider.java
##########
@@ -17,57 +17,153 @@
 
 package org.apache.dolphinscheduler.plugin.datasource.api.plugin;
 
-import org.apache.dolphinscheduler.plugin.datasource.api.utils.DataSourceUtils;
-import org.apache.dolphinscheduler.spi.datasource.BaseConnectionParam;
-import org.apache.dolphinscheduler.spi.datasource.ConnectionParam;
-import org.apache.dolphinscheduler.spi.datasource.DataSourceChannel;
+import org.apache.dolphinscheduler.plugin.datasource.api.exception.DataSourceException;
+import org.apache.dolphinscheduler.plugin.datasource.api.utils.ClassLoaderUtils;
+import org.apache.dolphinscheduler.plugin.datasource.api.utils.ThreadContextClassLoader;
+import org.apache.dolphinscheduler.spi.datasource.DataSourceChannelFactory;
 import org.apache.dolphinscheduler.spi.datasource.DataSourceClient;
+import org.apache.dolphinscheduler.spi.datasource.JdbcConnectionParam;
 import org.apache.dolphinscheduler.spi.enums.DbType;
+import org.apache.dolphinscheduler.spi.utils.StringUtils;
 
-import java.sql.Connection;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
+import java.io.FilenameFilter;
+import java.net.MalformedURLException;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.sql.Driver;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.ServiceLoader;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Sets;
+
 public class DataSourceClientProvider {
     private static final Logger logger = LoggerFactory.getLogger(DataSourceClientProvider.class);
 
-    private static final Map<String, DataSourceClient> uniqueId2dataSourceClientMap = new ConcurrentHashMap<>();
+    private static final JdbcDriverManager jdbcDriverManagerInstance = JdbcDriverManager.getInstance();
 
-    private DataSourcePluginManager dataSourcePluginManager;
+    public static DataSourceClient createDataSourceClient(JdbcConnectionParam connectionParam) {
+        logger.info("Creating the createDataSourceClient. JdbcUrl: {} ", connectionParam.getJdbcUrl());
 
-    private DataSourceClientProvider() {
-        initDataSourcePlugin();
-    }
+        //Check jdbc driver location
+        checkDriverLocation(connectionParam);
+
+        logger.info("Creating the ClassLoader for the jdbc driver and plugin.");
+        ClassLoader driverClassLoader = getDriverClassLoader(connectionParam);
 
-    private static class DataSourceClientProviderHolder {
-        private static final DataSourceClientProvider INSTANCE = new DataSourceClientProvider();
+        try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(driverClassLoader)) {
+            return createDataSourceClientWithClassLoader(connectionParam);
+        }
     }
 
-    public static DataSourceClientProvider getInstance() {
-        return DataSourceClientProviderHolder.INSTANCE;
+    protected static void checkDriverLocation(JdbcConnectionParam connectionParam) {
+        final String driverLocation = connectionParam.getDriverLocation();
+        if (StringUtils.isBlank(driverLocation)) {
+            logger.warn("No jdbc driver provide,will use randomly driver jar for {}.", connectionParam.getDbType().getDescp());
+            connectionParam.setDriverLocation(jdbcDriverManagerInstance.getDefaultDriverPluginPath(connectionParam.getDbType().getDescp()));
+        }
     }
 
-    public Connection getConnection(DbType dbType, ConnectionParam connectionParam) {
-        BaseConnectionParam baseConnectionParam = (BaseConnectionParam) connectionParam;
-        String datasourceUniqueId = DataSourceUtils.getDatasourceUniqueId(baseConnectionParam, dbType);
-        logger.info("getConnection datasourceUniqueId {}", datasourceUniqueId);
+    protected static ClassLoader getDriverClassLoader(JdbcConnectionParam connectionParam) {
+        FilenameFilter filenameFilter = (dir, name) -> name != null && name.endsWith(".jar");
+        ClassLoader threadClassLoader = Thread.currentThread().getContextClassLoader();
+        ClassLoader classLoader;
+
+        HashSet<String> paths = Sets.newHashSet();
+        if (StringUtils.isNotBlank(connectionParam.getDriverLocation())) {
+            logger.info("Driver location: {}", connectionParam.getDriverLocation());
+            paths.add(connectionParam.getDriverLocation());
+        }
+        try {
+            classLoader = ClassLoaderUtils.getCustomClassLoader(paths, threadClassLoader, filenameFilter);
+        } catch (final MalformedURLException e) {
+            throw DataSourceException.getInstance("Invalid jdbc driver location.", e);
+        }
+
+        //try loading jdbc driver
+        loadJdbcDriver(classLoader, connectionParam);
+
+        DbType dbType = connectionParam.getDbType();
+        String pluginPath = JdbcDriverManager.getInstance().getPluginPath(dbType);
+        logger.info("Plugin location: {}", pluginPath);
+        paths.add(pluginPath);
+
+        if (dbType == DbType.HIVE || dbType == DbType.SPARK) {
+            try {
+                Class.forName("org.apache.hadoop.conf.Configuration", true, classLoader);
+                Class.forName("org.apache.hadoop.security.UserGroupInformation", true, classLoader);
+                Class.forName("org.apache.hadoop.fs.FileSystem", true, classLoader);
+            } catch (ClassNotFoundException cnf) {
+                cnf.printStackTrace();

Review comment:
       Is it better to add error log?

##########
File path: dolphinscheduler-datasource-plugin/dolphinscheduler-datasource-api/src/main/java/org/apache/dolphinscheduler/plugin/datasource/api/utils/ThreadContextClassLoader.java
##########
@@ -15,33 +15,21 @@
  * limitations under the License.
  */
 
-package org.apache.dolphinscheduler.common.utils;
+package org.apache.dolphinscheduler.plugin.datasource.api.utils;
 
-import org.junit.Assert;
-import org.junit.Test;
+import java.io.Closeable;
 
-/**
- * hive conf utils test
- */
-public class HiveConfUtilsTest {
-
-    /**
-     * test is hive conf var
-     */
-    @Test
-    public void testIsHiveConfVar() {
-
-        String conf = "hive.exec.script.wrapper=123";
-        boolean hiveConfVar = HiveConfUtils.isHiveConfVar(conf);
-        Assert.assertTrue(hiveConfVar);
+public class ThreadContextClassLoader
+        implements Closeable {
+    private final ClassLoader threadContextClassLoader;
 
-        conf = "hive.test.v1=v1";
-        hiveConfVar = HiveConfUtils.isHiveConfVar(conf);
-        Assert.assertFalse(hiveConfVar);
-
-        conf = "tez.queue.name=tezQueue";
-        hiveConfVar = HiveConfUtils.isHiveConfVar(conf);
-        Assert.assertTrue(hiveConfVar);
+    public ThreadContextClassLoader(ClassLoader newThreadContextClassLoader) {

Review comment:
       It seems that the class loader is thread level? 
   What's the matter if some datasource thread was created but not set classLoader?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [dolphinscheduler] Narcasserun commented on pull request #7624: [feature-7623][plugin] Refactored data source plug-in loading

Posted by GitBox <gi...@apache.org>.
Narcasserun commented on pull request #7624:
URL: https://github.com/apache/dolphinscheduler/pull/7624#issuecomment-1003023855


   > Looks like the PMC/committers don't have a consensus in #7623 yet?
   
   I think we should pay more attention to what value this feature can bring to DS. It is wrong to blindly introduce new technologies and build cars behind closed doors. This feature is to isolate the configuration loading related to Hadoop, hive and Kerberos, which has nothing to do with the plug-in loading method of alarm, task and registry. Maybe it's limited from my own point of view @kezhenxu94 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org