This is an automated email from the ASF dual-hosted git repository.
zykkk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new eaaf5b4b373 [fix](jdbc catalog) Optimize JDBC Connection Closing to
Ensure ProperResource Release (#43059)
eaaf5b4b373 is described below
commit eaaf5b4b3731c6a665165702604aabab2a7cbed8
Author: zy-kkk <zhongy...@gmail.com>
AuthorDate: Fri Nov 1 21:24:57 2024 +0800
[fix](jdbc catalog) Optimize JDBC Connection Closing to Ensure
ProperResource Release (#43059)
This pull request optimizes the JDBC connection and resource management
in `JdbcClient` and related methods. The main changes are as follows:
- **Manual Resource Closing**: Explicitly place resource acquisition and
closing within a `try-catch-finally` structure. In the `finally` block,
manually close resources like `Connection`, `Statement`, and `ResultSet`
to ensure that resources are properly released even if an exception
occurs.
- **Remove try-with-resources**: All usages of `try-with-resources` have
been removed, switching to manual resource management instead.
- **Improve Exception Handling**: When closing resources, if an
exception occurs, log a warning but do not throw a new exception. This
avoids masking the original exception during the resource release phase.
- **Unified Resource Management**: Ensure consistent resource
acquisition and closing methods across all functions, improving code
readability and maintainability.
Functions modified include but are not limited to:
- `executeStmt`
- `getColumnsFromQuery`
- `getDatabaseNameList`
- `getJdbcColumnsInfo`
- `processTable`
- `getColumnsDataTypeUseQuery`
These changes enhance the robustness of the code, prevent potential
resource leaks, and ensure that all JDBC connections and resources are
properly closed without using `try-with-resources`.
---
.../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 03e5ca1fa7c..e05a7baa008 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
@@ -125,11 +125,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 8c4ada01774..458142ff518 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
@@ -137,6 +137,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()
@@ -188,13 +189,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);
}
}
}
@@ -206,9 +213,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()) {
@@ -228,10 +236,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",
@@ -246,12 +256,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
*
@@ -274,10 +283,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);
@@ -336,12 +346,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);
@@ -383,7 +394,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();
}
@@ -394,9 +405,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);
@@ -468,15 +480,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
