From 2bc97604e30fafa347ce9f1969517b9497e3b2ca Mon Sep 17 00:00:00 2001
From: zy-kkk <zhongyk10@gmail.com>
Date: Wed, 21 Aug 2024 10:36:57 +0800
Subject: [PATCH] [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.
---
 .../apache/doris/jdbc/BaseJdbcExecutor.java   | 27 +++++--------------
 .../apache/doris/jdbc/MySQLJdbcExecutor.java  |  4 +--
 .../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 08f5e65181e4dd..d1cd0c9d33e78d 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
@@ -103,20 +103,14 @@ public void close() throws Exception {
                 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();
@@ -130,23 +124,16 @@ private void closeResources(AutoCloseable... closeables) {
         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 5cdd30a9751594..60f190f129147d 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 MySQLJdbcExecutor(byte[] thriftParams) throws Exception {
     }
 
     @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 efe47b2d07505e..6679395d5510c6 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 SQLServerJdbcExecutor(byte[] thriftParams) throws Exception {
     }
 
     @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