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

morningman pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/branch-2.1 by this push:
     new 47fa76e0f6b [2.1][fix](jdbc catalog) Optimize JDBC Connection Closing 
to Ensure Proper Resource Release (#43074)
47fa76e0f6b is described below

commit 47fa76e0f6b0522d87c7da04a86506fd42240566
Author: zy-kkk <zhongy...@gmail.com>
AuthorDate: Fri Nov 1 20:34:18 2024 +0800

    [2.1][fix](jdbc catalog) Optimize JDBC Connection Closing to Ensure Proper 
Resource Release (#43074)
    
    bp #43059
---
 .../org/apache/doris/jdbc/BaseJdbcExecutor.java    | 14 ++++--
 .../doris/datasource/jdbc/client/JdbcClient.java   | 54 ++++++++++++++--------
 .../datasource/jdbc/client/JdbcDB2Client.java      |  3 +-
 .../datasource/jdbc/client/JdbcGbaseClient.java    |  6 ++-
 .../datasource/jdbc/client/JdbcMySQLClient.java    | 21 +++++----
 .../datasource/jdbc/client/JdbcOracleClient.java   |  3 +-
 6 files changed, 67 insertions(+), 34 deletions(-)

diff --git 
a/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/BaseJdbcExecutor.java
 
b/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/BaseJdbcExecutor.java
index 79122fc1217..3a7342d6280 100644
--- 
a/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/BaseJdbcExecutor.java
+++ 
b/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/BaseJdbcExecutor.java
@@ -124,11 +124,17 @@ public abstract class BaseJdbcExecutor implements 
JdbcExecutor {
         }
     }
 
-    private void closeResources(AutoCloseable... closeables) {
-        for (AutoCloseable closeable : closeables) {
-            if (closeable != null) {
+    private void closeResources(Object... resources) {
+        for (Object resource : resources) {
+            if (resource != null) {
                 try {
-                    closeable.close();
+                    if (resource instanceof ResultSet) {
+                        ((ResultSet) resource).close();
+                    } else if (resource instanceof Statement) {
+                        ((Statement) resource).close();
+                    } else if (resource instanceof Connection) {
+                        ((Connection) resource).close();
+                    }
                 } catch (Exception e) {
                     LOG.warn("Cannot close resource: ", e);
                 }
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcClient.java
 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcClient.java
index 80a858e86a8..54f15f7404e 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcClient.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcClient.java
@@ -136,6 +136,7 @@ public abstract class JdbcClient {
             
dataSource.setConnectionTimeout(config.getConnectionPoolMaxWaitTime()); // 
default 5000
             dataSource.setMaxLifetime(config.getConnectionPoolMaxLifeTime()); 
// default 30 min
             dataSource.setIdleTimeout(config.getConnectionPoolMaxLifeTime() / 
2L); // default 15 min
+            dataSource.setConnectionTestQuery(getTestQuery());
             LOG.info("JdbcClient set"
                     + " ConnectionPoolMinSize = " + 
config.getConnectionPoolMinSize()
                     + ", ConnectionPoolMaxSize = " + 
config.getConnectionPoolMaxSize()
@@ -187,13 +188,19 @@ public abstract class JdbcClient {
         return conn;
     }
 
-    public void close(AutoCloseable... closeables) {
-        for (AutoCloseable closeable : closeables) {
-            if (closeable != null) {
+    public void close(Object... resources) {
+        for (Object resource : resources) {
+            if (resource != null) {
                 try {
-                    closeable.close();
-                } catch (Exception e) {
-                    throw new JdbcClientException("Can not close : ", e);
+                    if (resource instanceof ResultSet) {
+                        ((ResultSet) resource).close();
+                    } else if (resource instanceof Statement) {
+                        ((Statement) resource).close();
+                    } else if (resource instanceof Connection) {
+                        ((Connection) resource).close();
+                    }
+                } catch (SQLException e) {
+                    LOG.warn("Failed to close resource: {}", e.getMessage(), 
e);
                 }
             }
         }
@@ -205,9 +212,10 @@ public abstract class JdbcClient {
      * @param origStmt, the raw stmt string
      */
     public void executeStmt(String origStmt) {
-        Connection conn = getConnection();
+        Connection conn = null;
         Statement stmt = null;
         try {
+            conn = getConnection();
             stmt = conn.createStatement();
             int effectedRows = stmt.executeUpdate(origStmt);
             if (LOG.isDebugEnabled()) {
@@ -227,10 +235,12 @@ public abstract class JdbcClient {
      * @return List<Column>
      */
     public List<Column> getColumnsFromQuery(String query) {
-        Connection conn = getConnection();
+        Connection conn = null;
+        PreparedStatement pstmt = null;
         List<Column> columns = Lists.newArrayList();
         try {
-            PreparedStatement pstmt = conn.prepareStatement(query);
+            conn = getConnection();
+            pstmt = conn.prepareStatement(query);
             ResultSetMetaData metaData = pstmt.getMetaData();
             if (metaData == null) {
                 throw new JdbcClientException("Query not supported: Failed to 
get ResultSetMetaData from query: %s",
@@ -245,12 +255,11 @@ public abstract class JdbcClient {
         } catch (SQLException e) {
             throw new JdbcClientException("Failed to get columns from query: 
%s", e, query);
         } finally {
-            close(conn);
+            close(pstmt, conn);
         }
         return columns;
     }
 
-
     /**
      * Get schema from ResultSetMetaData
      *
@@ -273,10 +282,11 @@ public abstract class JdbcClient {
      * @return list of database names
      */
     public List<String> getDatabaseNameList() {
-        Connection conn = getConnection();
+        Connection conn = null;
         ResultSet rs = null;
         List<String> remoteDatabaseNames = Lists.newArrayList();
         try {
+            conn = getConnection();
             if (isOnlySpecifiedDatabase && includeDatabaseMap.isEmpty() && 
excludeDatabaseMap.isEmpty()) {
                 String currentDatabase = conn.getSchema();
                 remoteDatabaseNames.add(currentDatabase);
@@ -335,12 +345,13 @@ public abstract class JdbcClient {
      * get all columns of one table
      */
     public List<JdbcFieldSchema> getJdbcColumnsInfo(String localDbName, String 
localTableName) {
-        Connection conn = getConnection();
+        Connection conn = null;
         ResultSet rs = null;
         List<JdbcFieldSchema> tableSchema = Lists.newArrayList();
         String remoteDbName = getRemoteDatabaseName(localDbName);
         String remoteTableName = getRemoteTableName(localDbName, 
localTableName);
         try {
+            conn = getConnection();
             DatabaseMetaData databaseMetaData = conn.getMetaData();
             String catalogName = getCatalogName(conn);
             rs = getRemoteColumns(databaseMetaData, catalogName, remoteDbName, 
remoteTableName);
@@ -382,7 +393,7 @@ public abstract class JdbcClient {
         return jdbcLowerCaseMetaMatching.getRemoteColumnNames(localDbName, 
localTableName);
     }
 
-    // protected methods,for subclass to override
+    // protected methods, for subclass to override
     protected String getCatalogName(Connection conn) throws SQLException {
         return conn.getCatalog();
     }
@@ -393,9 +404,10 @@ public abstract class JdbcClient {
 
     protected void processTable(String remoteDbName, String remoteTableName, 
String[] tableTypes,
             Consumer<ResultSet> resultSetConsumer) {
-        Connection conn = getConnection();
+        Connection conn = null;
         ResultSet rs = null;
         try {
+            conn = getConnection();
             DatabaseMetaData databaseMetaData = conn.getMetaData();
             String catalogName = getCatalogName(conn);
             rs = databaseMetaData.getTables(catalogName, remoteDbName, 
remoteTableName, tableTypes);
@@ -467,15 +479,21 @@ public abstract class JdbcClient {
 
     public void testConnection() {
         String testQuery = getTestQuery();
-        try (Connection conn = getConnection();
-                Statement stmt = conn.createStatement();
-                ResultSet rs = stmt.executeQuery(testQuery)) {
+        Connection conn = null;
+        Statement stmt = null;
+        ResultSet rs = null;
+        try {
+            conn = getConnection();
+            stmt = conn.createStatement();
+            rs = stmt.executeQuery(testQuery);
             if (!rs.next()) {
                 throw new JdbcClientException(
                         "Failed to test connection in FE: query executed but 
returned no results.");
             }
         } catch (SQLException e) {
             throw new JdbcClientException("Failed to test connection in FE: " 
+ e.getMessage(), e);
+        } finally {
+            close(rs, stmt, conn);
         }
     }
 
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcDB2Client.java
 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcDB2Client.java
index dafb00ca9e8..a353b7ac361 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcDB2Client.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcDB2Client.java
@@ -41,10 +41,11 @@ public class JdbcDB2Client extends JdbcClient {
 
     @Override
     public List<String> getDatabaseNameList() {
-        Connection conn = getConnection();
+        Connection conn = null;
         ResultSet rs = null;
         List<String> remoteDatabaseNames = Lists.newArrayList();
         try {
+            conn = getConnection();
             if (isOnlySpecifiedDatabase && includeDatabaseMap.isEmpty() && 
excludeDatabaseMap.isEmpty()) {
                 String currentDatabase = conn.getSchema().trim();
                 remoteDatabaseNames.add(currentDatabase);
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcGbaseClient.java
 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcGbaseClient.java
index 5aaacb3e673..7ba393e0d0a 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcGbaseClient.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcGbaseClient.java
@@ -41,10 +41,11 @@ public class JdbcGbaseClient extends JdbcClient {
 
     @Override
     public List<String> getDatabaseNameList() {
-        Connection conn = getConnection();
+        Connection conn = null;
         ResultSet rs = null;
         List<String> remoteDatabaseNames = Lists.newArrayList();
         try {
+            conn = getConnection();
             if (isOnlySpecifiedDatabase && includeDatabaseMap.isEmpty() && 
excludeDatabaseMap.isEmpty()) {
                 String currentDatabase = conn.getCatalog();
                 remoteDatabaseNames.add(currentDatabase);
@@ -87,12 +88,13 @@ public class JdbcGbaseClient extends JdbcClient {
 
     @Override
     public List<JdbcFieldSchema> getJdbcColumnsInfo(String localDbName, String 
localTableName) {
-        Connection conn = getConnection();
+        Connection conn = null;
         ResultSet rs = null;
         List<JdbcFieldSchema> tableSchema = Lists.newArrayList();
         String remoteDbName = getRemoteDatabaseName(localDbName);
         String remoteTableName = getRemoteTableName(localDbName, 
localTableName);
         try {
+            conn = getConnection();
             DatabaseMetaData databaseMetaData = conn.getMetaData();
             String catalogName = getCatalogName(conn);
             rs = getRemoteColumns(databaseMetaData, catalogName, remoteDbName, 
remoteTableName);
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcMySQLClient.java
 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcMySQLClient.java
index 465a3c152ac..a8263f1621a 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcMySQLClient.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcMySQLClient.java
@@ -76,10 +76,11 @@ public class JdbcMySQLClient extends JdbcClient {
 
     @Override
     public List<String> getDatabaseNameList() {
-        Connection conn = getConnection();
+        Connection conn = null;
         ResultSet rs = null;
         List<String> remoteDatabaseNames = Lists.newArrayList();
         try {
+            conn = getConnection();
             if (isOnlySpecifiedDatabase && includeDatabaseMap.isEmpty() && 
excludeDatabaseMap.isEmpty()) {
                 String currentDatabase = conn.getCatalog();
                 remoteDatabaseNames.add(currentDatabase);
@@ -130,12 +131,13 @@ public class JdbcMySQLClient extends JdbcClient {
      */
     @Override
     public List<JdbcFieldSchema> getJdbcColumnsInfo(String localDbName, String 
localTableName) {
-        Connection conn = getConnection();
+        Connection conn = null;
         ResultSet rs = null;
         List<JdbcFieldSchema> tableSchema = Lists.newArrayList();
         String remoteDbName = getRemoteDatabaseName(localDbName);
         String remoteTableName = getRemoteTableName(localDbName, 
localTableName);
         try {
+            conn = getConnection();
             DatabaseMetaData databaseMetaData = conn.getMetaData();
             String catalogName = getCatalogName(conn);
             rs = getRemoteColumns(databaseMetaData, catalogName, remoteDbName, 
remoteTableName);
@@ -294,30 +296,33 @@ public class JdbcMySQLClient extends JdbcClient {
      * get all columns like DatabaseMetaData.getColumns in mysql-jdbc-connector
      */
     private Map<String, String> getColumnsDataTypeUseQuery(String 
remoteDbName, String remoteTableName) {
-        Connection conn = getConnection();
+        Connection conn = null;
+        Statement stmt = null;
         ResultSet resultSet = null;
-        Map<String, String> fieldtoType = Maps.newHashMap();
+        Map<String, String> fieldToType = Maps.newHashMap();
 
         StringBuilder queryBuf = new StringBuilder("SHOW FULL COLUMNS FROM ");
         queryBuf.append(remoteTableName);
         queryBuf.append(" FROM ");
         queryBuf.append(remoteDbName);
-        try (Statement stmt = conn.createStatement()) {
+        try {
+            conn = getConnection();
+            stmt = conn.createStatement();
             resultSet = stmt.executeQuery(queryBuf.toString());
             while (resultSet.next()) {
                 // get column name
                 String fieldName = resultSet.getString("Field");
                 // get original type name
                 String typeName = resultSet.getString("Type");
-                fieldtoType.put(fieldName, typeName);
+                fieldToType.put(fieldName, typeName);
             }
         } catch (SQLException e) {
             throw new JdbcClientException("failed to get jdbc columns info for 
remote table `%s.%s`: %s",
                     remoteDbName, remoteTableName, 
Util.getRootCauseMessage(e));
         } finally {
-            close(resultSet, conn);
+            close(resultSet, stmt, conn);
         }
-        return fieldtoType;
+        return fieldToType;
     }
 
     private Type dorisTypeToDoris(JdbcFieldSchema fieldSchema) {
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcOracleClient.java
 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcOracleClient.java
index d37b36cbf3d..9968de79ab3 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcOracleClient.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/client/JdbcOracleClient.java
@@ -50,12 +50,13 @@ public class JdbcOracleClient extends JdbcClient {
 
     @Override
     public List<JdbcFieldSchema> getJdbcColumnsInfo(String localDbName, String 
localTableName) {
-        Connection conn = getConnection();
+        Connection conn = null;
         ResultSet rs = null;
         List<JdbcFieldSchema> tableSchema = Lists.newArrayList();
         String remoteDbName = getRemoteDatabaseName(localDbName);
         String remoteTableName = getRemoteTableName(localDbName, 
localTableName);
         try {
+            conn = getConnection();
             DatabaseMetaData databaseMetaData = conn.getMetaData();
             String catalogName = getCatalogName(conn);
             String modifiedTableName;


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

Reply via email to