Skip to content

Commit bd6912b

Browse files
zy-kkkdataroaring
authored andcommitted
[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.
1 parent 314a87d commit bd6912b

File tree

3 files changed

+9
-26
lines changed

3 files changed

+9
-26
lines changed

fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/BaseJdbcExecutor.java

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -105,20 +105,14 @@ public void close() throws Exception {
105105
try {
106106
stmt.cancel();
107107
} catch (SQLException e) {
108-
LOG.error("Error cancelling statement", e);
108+
LOG.warn("Cannot cancelling statement: ", e);
109109
}
110110
}
111111

112-
boolean shouldAbort = conn != null && resultSet != null;
113-
boolean aborted = false; // Used to record whether the abort operation is performed
114-
if (shouldAbort) {
115-
aborted = abortReadConnection(conn, resultSet);
116-
}
117-
118-
// If no abort operation is performed, the resource needs to be closed manually
119-
if (!aborted) {
120-
closeResources(resultSet, stmt, conn);
112+
if (conn != null && resultSet != null) {
113+
abortReadConnection(conn, resultSet);
121114
}
115+
closeResources(resultSet, stmt, conn);
122116
} finally {
123117
if (config.getConnectionPoolMinSize() == 0 && hikariDataSource != null) {
124118
hikariDataSource.close();
@@ -132,23 +126,16 @@ private void closeResources(AutoCloseable... closeables) {
132126
for (AutoCloseable closeable : closeables) {
133127
if (closeable != null) {
134128
try {
135-
if (closeable instanceof Connection) {
136-
if (!((Connection) closeable).isClosed()) {
137-
closeable.close();
138-
}
139-
} else {
140-
closeable.close();
141-
}
129+
closeable.close();
142130
} catch (Exception e) {
143-
LOG.error("Cannot close resource: ", e);
131+
LOG.warn("Cannot close resource: ", e);
144132
}
145133
}
146134
}
147135
}
148136

149-
protected boolean abortReadConnection(Connection connection, ResultSet resultSet)
137+
protected void abortReadConnection(Connection connection, ResultSet resultSet)
150138
throws SQLException {
151-
return false;
152139
}
153140

154141
public void cleanDataSource() {

fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/MySQLJdbcExecutor.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,13 @@ public MySQLJdbcExecutor(byte[] thriftParams) throws Exception {
5252
}
5353

5454
@Override
55-
protected boolean abortReadConnection(Connection connection, ResultSet resultSet)
55+
protected void abortReadConnection(Connection connection, ResultSet resultSet)
5656
throws SQLException {
5757
if (!resultSet.isAfterLast()) {
5858
// Abort connection before closing. Without this, the MySQL driver
5959
// attempts to drain the connection by reading all the results.
6060
connection.abort(MoreExecutors.directExecutor());
61-
return true;
6261
}
63-
return false;
6462
}
6563

6664
@Override

fe/be-java-extensions/jdbc-scanner/src/main/java/org/apache/doris/jdbc/SQLServerJdbcExecutor.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,13 @@ public SQLServerJdbcExecutor(byte[] thriftParams) throws Exception {
3838
}
3939

4040
@Override
41-
protected boolean abortReadConnection(Connection connection, ResultSet resultSet)
41+
protected void abortReadConnection(Connection connection, ResultSet resultSet)
4242
throws SQLException {
4343
if (!resultSet.isAfterLast()) {
4444
// Abort connection before closing. Without this, the SQLServer driver
4545
// attempts to drain the connection by reading all the results.
4646
connection.abort(MoreExecutors.directExecutor());
47-
return true;
4847
}
49-
return false;
5048
}
5149

5250
@Override

0 commit comments

Comments
 (0)