This is an automated email from the ASF dual-hosted git repository. dataroaring pushed a commit to branch branch-3.0 in repository https://gitbox.apache.org/repos/asf/doris.git
commit bd6912b27313308b812d6e3a283b9c6ecf582d5b Author: zy-kkk <[email protected]> AuthorDate: Wed Aug 21 10:36:57 2024 +0800 [improvement](jdbc catalog) Force all resources to be closed in the close method (#39423) Force all resources to be closed in the close method. In the previous logic, query errors or query cancellation will not force the connection to be closed, which will cause abnormal Hikari connection counts. Although forced connection closure will generate some error logs in some cases, we should have this bottom-line guarantee and refine the closing logic later. --- .../org/apache/doris/jdbc/BaseJdbcExecutor.java | 27 ++++++---------------- .../org/apache/doris/jdbc/MySQLJdbcExecutor.java | 4 +--- .../apache/doris/jdbc/SQLServerJdbcExecutor.java | 4 +--- 3 files changed, 9 insertions(+), 26 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 23ab66f5763..3e681fa1519 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 @@ -105,20 +105,14 @@ public abstract class BaseJdbcExecutor implements JdbcExecutor { try { stmt.cancel(); } catch (SQLException e) { - LOG.error("Error cancelling statement", e); + LOG.warn("Cannot cancelling statement: ", e); } } - boolean shouldAbort = conn != null && resultSet != null; - boolean aborted = false; // Used to record whether the abort operation is performed - if (shouldAbort) { - aborted = abortReadConnection(conn, resultSet); - } - - // If no abort operation is performed, the resource needs to be closed manually - if (!aborted) { - closeResources(resultSet, stmt, conn); + if (conn != null && resultSet != null) { + abortReadConnection(conn, resultSet); } + closeResources(resultSet, stmt, conn); } finally { if (config.getConnectionPoolMinSize() == 0 && hikariDataSource != null) { hikariDataSource.close(); @@ -132,23 +126,16 @@ public abstract class BaseJdbcExecutor implements JdbcExecutor { for (AutoCloseable closeable : closeables) { if (closeable != null) { try { - if (closeable instanceof Connection) { - if (!((Connection) closeable).isClosed()) { - closeable.close(); - } - } else { - closeable.close(); - } + closeable.close(); } catch (Exception e) { - LOG.error("Cannot close resource: ", e); + LOG.warn("Cannot close resource: ", e); } } } } - protected boolean abortReadConnection(Connection connection, ResultSet resultSet) + protected void abortReadConnection(Connection connection, ResultSet resultSet) throws SQLException { - return false; } public void cleanDataSource() { diff --git a/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/MySQLJdbcExecutor.java b/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/MySQLJdbcExecutor.java index 5cdd30a9751..60f190f1291 100644 --- a/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/MySQLJdbcExecutor.java +++ b/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/MySQLJdbcExecutor.java @@ -52,15 +52,13 @@ public class MySQLJdbcExecutor extends BaseJdbcExecutor { } @Override - protected boolean abortReadConnection(Connection connection, ResultSet resultSet) + protected void abortReadConnection(Connection connection, ResultSet resultSet) throws SQLException { if (!resultSet.isAfterLast()) { // Abort connection before closing. Without this, the MySQL driver // attempts to drain the connection by reading all the results. connection.abort(MoreExecutors.directExecutor()); - return true; } - return false; } @Override diff --git a/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/SQLServerJdbcExecutor.java b/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/SQLServerJdbcExecutor.java index efe47b2d075..6679395d551 100644 --- a/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/SQLServerJdbcExecutor.java +++ b/fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/SQLServerJdbcExecutor.java @@ -38,15 +38,13 @@ public class SQLServerJdbcExecutor extends BaseJdbcExecutor { } @Override - protected boolean abortReadConnection(Connection connection, ResultSet resultSet) + protected void abortReadConnection(Connection connection, ResultSet resultSet) throws SQLException { if (!resultSet.isAfterLast()) { // Abort connection before closing. Without this, the SQLServer driver // attempts to drain the connection by reading all the results. connection.abort(MoreExecutors.directExecutor()); - return true; } - return false; } @Override --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
