morningman commented on code in PR #41992:
URL: https://github.com/apache/doris/pull/41992#discussion_r1804157393


##########
fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/BaseJdbcExecutor.java:
##########
@@ -287,50 +294,70 @@ public boolean hasNext() throws JdbcExecutorException {
 
     private void init(JdbcDataSourceConfig config, String sql) throws 
JdbcExecutorException {
         ClassLoader oldClassLoader = 
Thread.currentThread().getContextClassLoader();
-        String hikariDataSourceKey = config.createCacheKey();
         try {
             ClassLoader parent = getClass().getClassLoader();
             ClassLoader classLoader = 
UdfUtils.getClassLoader(config.getJdbcDriverUrl(), parent);
             Thread.currentThread().setContextClassLoader(classLoader);
-            hikariDataSource = 
JdbcDataSource.getDataSource().getSource(hikariDataSourceKey);
-            if (hikariDataSource == null) {
-                synchronized (hikariDataSourceLock) {
-                    hikariDataSource = 
JdbcDataSource.getDataSource().getSource(hikariDataSourceKey);
-                    if (hikariDataSource == null) {
-                        long start = System.currentTimeMillis();
-                        HikariDataSource ds = new HikariDataSource();
-                        ds.setDriverClassName(config.getJdbcDriverClass());
-                        
ds.setJdbcUrl(SecurityChecker.getInstance().getSafeJdbcUrl(config.getJdbcUrl()));
-                        ds.setUsername(config.getJdbcUser());
-                        ds.setPassword(config.getJdbcPassword());
-                        ds.setMinimumIdle(config.getConnectionPoolMinSize()); 
// default 1
-                        
ds.setMaximumPoolSize(config.getConnectionPoolMaxSize()); // default 10
-                        
ds.setConnectionTimeout(config.getConnectionPoolMaxWaitTime()); // default 5000
-                        
ds.setMaxLifetime(config.getConnectionPoolMaxLifeTime()); // default 30 min
-                        
ds.setIdleTimeout(config.getConnectionPoolMaxLifeTime() / 2L); // default 15 min
-                        setValidationQuery(ds);
-                        if (config.isConnectionPoolKeepAlive()) {
-                            
ds.setKeepaliveTime(config.getConnectionPoolMaxLifeTime() / 5L); // default 6 
min
+            if (config.isEnableConnectionPool()) {
+                String hikariDataSourceKey = config.createCacheKey();
+                hikariDataSource = 
JdbcDataSource.getDataSource().getSource(hikariDataSourceKey);
+                if (hikariDataSource == null) {
+                    synchronized (hikariDataSourceLock) {
+                        hikariDataSource = 
JdbcDataSource.getDataSource().getSource(hikariDataSourceKey);
+                        if (hikariDataSource == null) {
+                            long start = System.currentTimeMillis();
+                            HikariDataSource ds = new HikariDataSource();
+                            ds.setDriverClassName(config.getJdbcDriverClass());
+                            
ds.setJdbcUrl(SecurityChecker.getInstance().getSafeJdbcUrl(config.getJdbcUrl()));
+                            ds.setUsername(config.getJdbcUser());
+                            ds.setPassword(config.getJdbcPassword());
+                            
ds.setMinimumIdle(config.getConnectionPoolMinSize()); // default 1
+                            
ds.setMaximumPoolSize(config.getConnectionPoolMaxSize()); // default 10
+                            
ds.setConnectionTimeout(config.getConnectionPoolMaxWaitTime()); // default 5000
+                            
ds.setMaxLifetime(config.getConnectionPoolMaxLifeTime()); // default 30 min
+                            
ds.setIdleTimeout(config.getConnectionPoolMaxLifeTime() / 2L); // default 15 min
+                            setValidationQuery(ds);
+                            if (config.isConnectionPoolKeepAlive()) {
+                                
ds.setKeepaliveTime(config.getConnectionPoolMaxLifeTime() / 5L); // default 6 
min
+                            }
+                            hikariDataSource = ds;
+                            
JdbcDataSource.getDataSource().putSource(hikariDataSourceKey, hikariDataSource);
+                            LOG.info("JdbcClient set"
+                                    + " ConnectionPoolMinSize = " + 
config.getConnectionPoolMinSize()
+                                    + ", ConnectionPoolMaxSize = " + 
config.getConnectionPoolMaxSize()
+                                    + ", ConnectionPoolMaxWaitTime = " + 
config.getConnectionPoolMaxWaitTime()
+                                    + ", ConnectionPoolMaxLifeTime = " + 
config.getConnectionPoolMaxLifeTime()
+                                    + ", ConnectionPoolKeepAlive = " + 
config.isConnectionPoolKeepAlive());
+                            LOG.info("init datasource [" + 
(config.getJdbcUrl() + config.getJdbcUser()) + "] cost: " + (
+                                    System.currentTimeMillis() - start) + " 
ms");
                         }
-                        hikariDataSource = ds;
-                        
JdbcDataSource.getDataSource().putSource(hikariDataSourceKey, hikariDataSource);
-                        LOG.info("JdbcClient set"
-                                + " ConnectionPoolMinSize = " + 
config.getConnectionPoolMinSize()
-                                + ", ConnectionPoolMaxSize = " + 
config.getConnectionPoolMaxSize()
-                                + ", ConnectionPoolMaxWaitTime = " + 
config.getConnectionPoolMaxWaitTime()
-                                + ", ConnectionPoolMaxLifeTime = " + 
config.getConnectionPoolMaxLifeTime()
-                                + ", ConnectionPoolKeepAlive = " + 
config.isConnectionPoolKeepAlive());
-                        LOG.info("init datasource [" + (config.getJdbcUrl() + 
config.getJdbcUser()) + "] cost: " + (
-                                System.currentTimeMillis() - start) + " ms");
                     }
                 }
-            }
 
-            long start = System.currentTimeMillis();
-            conn = hikariDataSource.getConnection();
-            LOG.info("get connection [" + (config.getJdbcUrl() + 
config.getJdbcUser()) + "] cost: " + (
-                    System.currentTimeMillis() - start)
-                    + " ms");
+                long start = System.currentTimeMillis();
+                conn = hikariDataSource.getConnection();
+                LOG.info("get connection with pool [" + (config.getJdbcUrl() + 
config.getJdbcUser()) + "] cost: " + (
+                        System.currentTimeMillis() - start)
+                        + " ms");
+            } else {
+                long start = System.currentTimeMillis();
+                Class<?> driverClass = 
Class.forName(config.getJdbcDriverClass(), true, classLoader);
+                Driver driverInstance = (Driver) 
driverClass.getDeclaredConstructor().newInstance();
+
+                Properties info = new Properties();
+                info.put("user", config.getJdbcUser());
+                info.put("password", config.getJdbcPassword());
+
+                conn = 
driverInstance.connect(SecurityChecker.getInstance().getSafeJdbcUrl(config.getJdbcUrl()),
 info);
+
+                LOG.info("get connection without pool [" + 
(config.getJdbcUrl() + config.getJdbcUser()) + "] cost: " + (
+                        System.currentTimeMillis() - start)
+                        + " ms");
+
+                if (conn == null) {
+                    throw new JdbcExecutorException("Failed to establish 
connection. Driver returned null.");

Review Comment:
   Can we get more detail error msg?



##########
fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/BaseJdbcExecutor.java:
##########
@@ -287,50 +294,70 @@ public boolean hasNext() throws JdbcExecutorException {
 
     private void init(JdbcDataSourceConfig config, String sql) throws 
JdbcExecutorException {
         ClassLoader oldClassLoader = 
Thread.currentThread().getContextClassLoader();
-        String hikariDataSourceKey = config.createCacheKey();
         try {
             ClassLoader parent = getClass().getClassLoader();
             ClassLoader classLoader = 
UdfUtils.getClassLoader(config.getJdbcDriverUrl(), parent);
             Thread.currentThread().setContextClassLoader(classLoader);
-            hikariDataSource = 
JdbcDataSource.getDataSource().getSource(hikariDataSourceKey);
-            if (hikariDataSource == null) {
-                synchronized (hikariDataSourceLock) {
-                    hikariDataSource = 
JdbcDataSource.getDataSource().getSource(hikariDataSourceKey);
-                    if (hikariDataSource == null) {
-                        long start = System.currentTimeMillis();
-                        HikariDataSource ds = new HikariDataSource();
-                        ds.setDriverClassName(config.getJdbcDriverClass());
-                        
ds.setJdbcUrl(SecurityChecker.getInstance().getSafeJdbcUrl(config.getJdbcUrl()));
-                        ds.setUsername(config.getJdbcUser());
-                        ds.setPassword(config.getJdbcPassword());
-                        ds.setMinimumIdle(config.getConnectionPoolMinSize()); 
// default 1
-                        
ds.setMaximumPoolSize(config.getConnectionPoolMaxSize()); // default 10
-                        
ds.setConnectionTimeout(config.getConnectionPoolMaxWaitTime()); // default 5000
-                        
ds.setMaxLifetime(config.getConnectionPoolMaxLifeTime()); // default 30 min
-                        
ds.setIdleTimeout(config.getConnectionPoolMaxLifeTime() / 2L); // default 15 min
-                        setValidationQuery(ds);
-                        if (config.isConnectionPoolKeepAlive()) {
-                            
ds.setKeepaliveTime(config.getConnectionPoolMaxLifeTime() / 5L); // default 6 
min
+            if (config.isEnableConnectionPool()) {
+                String hikariDataSourceKey = config.createCacheKey();
+                hikariDataSource = 
JdbcDataSource.getDataSource().getSource(hikariDataSourceKey);
+                if (hikariDataSource == null) {
+                    synchronized (hikariDataSourceLock) {
+                        hikariDataSource = 
JdbcDataSource.getDataSource().getSource(hikariDataSourceKey);
+                        if (hikariDataSource == null) {
+                            long start = System.currentTimeMillis();
+                            HikariDataSource ds = new HikariDataSource();
+                            ds.setDriverClassName(config.getJdbcDriverClass());
+                            
ds.setJdbcUrl(SecurityChecker.getInstance().getSafeJdbcUrl(config.getJdbcUrl()));
+                            ds.setUsername(config.getJdbcUser());
+                            ds.setPassword(config.getJdbcPassword());
+                            
ds.setMinimumIdle(config.getConnectionPoolMinSize()); // default 1
+                            
ds.setMaximumPoolSize(config.getConnectionPoolMaxSize()); // default 10
+                            
ds.setConnectionTimeout(config.getConnectionPoolMaxWaitTime()); // default 5000
+                            
ds.setMaxLifetime(config.getConnectionPoolMaxLifeTime()); // default 30 min
+                            
ds.setIdleTimeout(config.getConnectionPoolMaxLifeTime() / 2L); // default 15 min
+                            setValidationQuery(ds);
+                            if (config.isConnectionPoolKeepAlive()) {
+                                
ds.setKeepaliveTime(config.getConnectionPoolMaxLifeTime() / 5L); // default 6 
min
+                            }
+                            hikariDataSource = ds;
+                            
JdbcDataSource.getDataSource().putSource(hikariDataSourceKey, hikariDataSource);
+                            LOG.info("JdbcClient set"
+                                    + " ConnectionPoolMinSize = " + 
config.getConnectionPoolMinSize()
+                                    + ", ConnectionPoolMaxSize = " + 
config.getConnectionPoolMaxSize()
+                                    + ", ConnectionPoolMaxWaitTime = " + 
config.getConnectionPoolMaxWaitTime()
+                                    + ", ConnectionPoolMaxLifeTime = " + 
config.getConnectionPoolMaxLifeTime()
+                                    + ", ConnectionPoolKeepAlive = " + 
config.isConnectionPoolKeepAlive());
+                            LOG.info("init datasource [" + 
(config.getJdbcUrl() + config.getJdbcUser()) + "] cost: " + (
+                                    System.currentTimeMillis() - start) + " 
ms");
                         }
-                        hikariDataSource = ds;
-                        
JdbcDataSource.getDataSource().putSource(hikariDataSourceKey, hikariDataSource);
-                        LOG.info("JdbcClient set"
-                                + " ConnectionPoolMinSize = " + 
config.getConnectionPoolMinSize()
-                                + ", ConnectionPoolMaxSize = " + 
config.getConnectionPoolMaxSize()
-                                + ", ConnectionPoolMaxWaitTime = " + 
config.getConnectionPoolMaxWaitTime()
-                                + ", ConnectionPoolMaxLifeTime = " + 
config.getConnectionPoolMaxLifeTime()
-                                + ", ConnectionPoolKeepAlive = " + 
config.isConnectionPoolKeepAlive());
-                        LOG.info("init datasource [" + (config.getJdbcUrl() + 
config.getJdbcUser()) + "] cost: " + (
-                                System.currentTimeMillis() - start) + " ms");
                     }
                 }
-            }
 
-            long start = System.currentTimeMillis();
-            conn = hikariDataSource.getConnection();
-            LOG.info("get connection [" + (config.getJdbcUrl() + 
config.getJdbcUser()) + "] cost: " + (
-                    System.currentTimeMillis() - start)
-                    + " ms");
+                long start = System.currentTimeMillis();
+                conn = hikariDataSource.getConnection();
+                LOG.info("get connection with pool [" + (config.getJdbcUrl() + 
config.getJdbcUser()) + "] cost: " + (
+                        System.currentTimeMillis() - start)
+                        + " ms");
+            } else {
+                long start = System.currentTimeMillis();
+                Class<?> driverClass = 
Class.forName(config.getJdbcDriverClass(), true, classLoader);
+                Driver driverInstance = (Driver) 
driverClass.getDeclaredConstructor().newInstance();
+
+                Properties info = new Properties();
+                info.put("user", config.getJdbcUser());
+                info.put("password", config.getJdbcPassword());
+
+                conn = 
driverInstance.connect(SecurityChecker.getInstance().getSafeJdbcUrl(config.getJdbcUrl()),
 info);
+
+                LOG.info("get connection without pool [" + 
(config.getJdbcUrl() + config.getJdbcUser()) + "] cost: " + (

Review Comment:
   There will be too much log



##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcClient.java:
##########
@@ -168,15 +179,53 @@ public static String parseDbType(String jdbcUrl) {
     }
 
     public void closeClient() {
-        dataSource.close();
+        if (enableConnectionPool && dataSource != null) {
+            dataSource.close();
+        }
     }
 
     public Connection getConnection() throws JdbcClientException {
+        if (enableConnectionPool) {
+            return getConnectionWithPool();
+        } else {
+            return getConnectionWithoutPool();
+        }
+    }
+
+    private Connection getConnectionWithoutPool() throws JdbcClientException {
+        ClassLoader oldClassLoader = 
Thread.currentThread().getContextClassLoader();

Review Comment:
   Do we need to load the class for each `getConnection` call?



##########
regression-test/suites/external_table_p0/jdbc/non_connection_pool/test_oracle_jdbc_catalog_no_pool.groovy:
##########
@@ -0,0 +1,69 @@
+// Licensed to the Apache Software Foundation (ASF) under one

Review Comment:
   Also need to add case to test `alter property`



##########
fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java:
##########
@@ -122,10 +124,10 @@ public void onRefresh(boolean invalidCache) {
         }
     }
 
-    @Override
-    public void onRefreshCache(boolean invalidCache) {
-        onRefresh(invalidCache);
-    }
+    // @Override

Review Comment:
   remove 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-unsubscr...@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to