This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 8921672509 [#10253] Optimize driver degisteration logic for JDBC
catalog to avoid possible OOM problem (#10255)
8921672509 is described below
commit 892167250926a5c6c8694d497da4b0866cd98a27
Author: Qi Yu <[email protected]>
AuthorDate: Mon Mar 9 18:07:52 2026 +0800
[#10253] Optimize driver degisteration logic for JDBC catalog to avoid
possible OOM problem (#10255)
### What changes were proposed in this pull request?
This pull request refactors the JDBC driver deregistration logic across
multiple catalog implementations to centralize and standardize the
process. The main change is moving driver deregistration from individual
catalog subclasses to the base `JdbcCatalogOperations` class, reducing
code duplication and improving maintainability. Additionally, the pull
request updates related tests and adapts the OceanBase catalog to use a
more appropriate operations class.
### Why are the changes needed?
To avoid resource leakage
Fix: #10253
### Does this PR introduce _any_ user-facing change?
N/A
### How was this patch tested?
UTs
---------
Co-authored-by: Copilot <[email protected]>
---
.../operations/ClickHouseCatalogOperations.java | 15 +----
.../hologres/HologresCatalogOperations.java | 13 +---
.../catalog/oceanbase/OceanBaseCatalog.java | 4 +-
.../catalog/jdbc/JdbcCatalogOperations.java | 24 +++++++
.../MySQLProtocolCompatibleCatalogOperations.java | 13 ++--
.../catalog/jdbc/TestJdbcCatalogOperations.java | 78 ++++++++++++++++++++++
.../postgresql/PostgreSQLCatalogOperations.java | 13 +---
7 files changed, 120 insertions(+), 40 deletions(-)
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseCatalogOperations.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseCatalogOperations.java
index 01631f3337..fb9a3d7140 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseCatalogOperations.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseCatalogOperations.java
@@ -21,6 +21,7 @@ package org.apache.gravitino.catalog.clickhouse.operations;
import java.sql.Driver;
import java.sql.DriverManager;
+import java.sql.SQLException;
import org.apache.gravitino.catalog.jdbc.JdbcCatalogOperations;
import
org.apache.gravitino.catalog.jdbc.converter.JdbcColumnDefaultValueConverter;
import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter;
@@ -45,17 +46,7 @@ public class ClickHouseCatalogOperations extends
JdbcCatalogOperations {
}
@Override
- public void close() {
- super.close();
-
- // deregister ClickHouse JDBC driver to prevent memory leak in
long-running applications
- try {
- Driver clickhouseDriver =
-
DriverManager.getDriver("jdbc:clickhouse://dummy_address:8443/default");
- deregisterDriver(clickhouseDriver);
- } catch (java.sql.SQLException e) {
- // log and ignore
- System.err.println("Failed to deregister ClickHouse JDBC driver: " +
e.getMessage());
- }
+ protected Driver getDriver() throws SQLException {
+ return
DriverManager.getDriver("jdbc:clickhouse://dummy_address:8443/default");
}
}
diff --git
a/catalogs-contrib/catalog-jdbc-hologres/src/main/java/org/apache/gravitino/catalog/hologres/HologresCatalogOperations.java
b/catalogs-contrib/catalog-jdbc-hologres/src/main/java/org/apache/gravitino/catalog/hologres/HologresCatalogOperations.java
index 135e1bd02e..2c2442569e 100644
---
a/catalogs-contrib/catalog-jdbc-hologres/src/main/java/org/apache/gravitino/catalog/hologres/HologresCatalogOperations.java
+++
b/catalogs-contrib/catalog-jdbc-hologres/src/main/java/org/apache/gravitino/catalog/hologres/HologresCatalogOperations.java
@@ -20,6 +20,7 @@ package org.apache.gravitino.catalog.hologres;
import java.sql.Driver;
import java.sql.DriverManager;
+import java.sql.SQLException;
import org.apache.gravitino.catalog.jdbc.JdbcCatalogOperations;
import
org.apache.gravitino.catalog.jdbc.converter.JdbcColumnDefaultValueConverter;
import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter;
@@ -50,15 +51,7 @@ public class HologresCatalogOperations extends
JdbcCatalogOperations {
}
@Override
- public void close() {
- super.close();
- try {
- // Unload the PostgreSQL driver, only unload the driver if it is loaded
by
- // IsolatedClassLoader. Hologres uses PostgreSQL JDBC driver.
- Driver pgDriver =
DriverManager.getDriver("jdbc:postgresql://dummy_address:12345/");
- deregisterDriver(pgDriver);
- } catch (Exception e) {
- LOG.warn("Failed to deregister PostgreSQL driver for Hologres", e);
- }
+ protected Driver getDriver() throws SQLException {
+ return DriverManager.getDriver("jdbc:postgresql://dummy_address:12345/");
}
}
diff --git
a/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/OceanBaseCatalog.java
b/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/OceanBaseCatalog.java
index 1231926441..200d501de5 100644
---
a/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/OceanBaseCatalog.java
+++
b/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/OceanBaseCatalog.java
@@ -20,7 +20,7 @@ package org.apache.gravitino.catalog.oceanbase;
import java.util.Map;
import org.apache.gravitino.catalog.jdbc.JdbcCatalog;
-import org.apache.gravitino.catalog.jdbc.JdbcCatalogOperations;
+import
org.apache.gravitino.catalog.jdbc.MySQLProtocolCompatibleCatalogOperations;
import
org.apache.gravitino.catalog.jdbc.converter.JdbcColumnDefaultValueConverter;
import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter;
import org.apache.gravitino.catalog.jdbc.operation.JdbcDatabaseOperations;
@@ -42,7 +42,7 @@ public class OceanBaseCatalog extends JdbcCatalog {
@Override
protected CatalogOperations newOps(Map<String, String> config) {
- return new JdbcCatalogOperations(
+ return new MySQLProtocolCompatibleCatalogOperations(
createExceptionConverter(),
createJdbcTypeConverter(),
createJdbcDatabaseOperations(),
diff --git
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalogOperations.java
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalogOperations.java
index b70abc354d..e9716749fa 100644
---
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalogOperations.java
+++
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/JdbcCatalogOperations.java
@@ -102,6 +102,7 @@ public class JdbcCatalogOperations implements
CatalogOperations, SupportsSchemas
private final TableOperation tableOperation;
private DataSource dataSource;
+ private String jdbcUrl;
private final JdbcColumnDefaultValueConverter columnDefaultValueConverter;
@@ -170,6 +171,7 @@ public class JdbcCatalogOperations implements
CatalogOperations, SupportsSchemas
resultConf.putAll(gravitinoConfig);
JdbcConfig jdbcConfig = new JdbcConfig(resultConf);
+ this.jdbcUrl = jdbcConfig.getJdbcUrl();
this.dataSource = DataSourceUtils.createDataSource(jdbcConfig);
checkJDBCDriverVersion();
@@ -199,6 +201,28 @@ public class JdbcCatalogOperations implements
CatalogOperations, SupportsSchemas
metricsSystem.unregister(catalogMetricsSource);
}
DataSourceUtils.closeDataSource(dataSource);
+ try {
+ Driver driver = getDriver();
+ if (driver != null) {
+ deregisterDriver(driver);
+ }
+ } catch (SQLException e) {
+ LOG.warn("Failed to deregister JDBC driver", e);
+ }
+ }
+
+ /**
+ * Gets the JDBC driver for the provided JDBC URL.
+ *
+ * @return The JDBC driver instance if the JDBC URL is not blank; null if
the JDBC URL is blank.
+ * @throws SQLException if failing to get driver from JDBC URL.
+ */
+ protected Driver getDriver() throws SQLException {
+ if (StringUtils.isBlank(jdbcUrl)) {
+ return null;
+ }
+
+ return DriverManager.getDriver(jdbcUrl);
}
/**
diff --git
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/MySQLProtocolCompatibleCatalogOperations.java
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/MySQLProtocolCompatibleCatalogOperations.java
index b4ee565dce..2a5bddaa99 100644
---
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/MySQLProtocolCompatibleCatalogOperations.java
+++
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/MySQLProtocolCompatibleCatalogOperations.java
@@ -21,6 +21,7 @@ package org.apache.gravitino.catalog.jdbc;
import java.sql.Driver;
import java.sql.DriverManager;
+import java.sql.SQLException;
import
org.apache.gravitino.catalog.jdbc.converter.JdbcColumnDefaultValueConverter;
import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter;
import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter;
@@ -70,13 +71,13 @@ public class MySQLProtocolCompatibleCatalogOperations
extends JdbcCatalogOperati
.getMethod("uncheckedShutdown")
.invoke(null);
LOG.info("AbandonedConnectionCleanupThread has been shutdown...");
-
- // Unload the MySQL driver, only Unload the driver if it is loaded by
- // IsolatedClassLoader.
- Driver mysqlDriver =
DriverManager.getDriver("jdbc:mysql://dumpy_address");
- deregisterDriver(mysqlDriver);
} catch (Exception e) {
- LOG.warn("Failed to shutdown AbandonedConnectionCleanupThread or
deregister MySQL driver", e);
+ LOG.warn("Failed to shutdown AbandonedConnectionCleanupThread", e);
}
}
+
+ @Override
+ protected Driver getDriver() throws SQLException {
+ return DriverManager.getDriver("jdbc:mysql://dummy_address");
+ }
}
diff --git
a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogOperations.java
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogOperations.java
index 0b082d5916..d7f711e20f 100644
---
a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogOperations.java
+++
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/TestJdbcCatalogOperations.java
@@ -20,6 +20,8 @@ package org.apache.gravitino.catalog.jdbc;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
+import java.sql.Driver;
+import java.sql.DriverManager;
import java.sql.SQLException;
import java.util.HashMap;
import javax.sql.DataSource;
@@ -27,6 +29,8 @@ import org.apache.commons.dbcp2.BasicDataSource;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.catalog.jdbc.config.JdbcConfig;
+import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter;
+import org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter;
import
org.apache.gravitino.catalog.jdbc.converter.SqliteColumnDefaultValueConverter;
import org.apache.gravitino.catalog.jdbc.converter.SqliteExceptionConverter;
import org.apache.gravitino.catalog.jdbc.converter.SqliteTypeConverter;
@@ -75,4 +79,78 @@ public class TestJdbcCatalogOperations {
Assertions.assertFalse(((BasicDataSource) dataSource).getTestOnBorrow());
((BasicDataSource) dataSource).close();
}
+
+ @Test
+ public void testCloseDeregisterDriver() throws SQLException {
+ TestableJdbcCatalogOperations catalogOperations =
+ new TestableJdbcCatalogOperations(
+ new SqliteExceptionConverter(),
+ new SqliteTypeConverter(),
+ new SqliteDatabaseOperations("/illegal/path"),
+ new SqliteTableOperations(),
+ new SqliteColumnDefaultValueConverter());
+
catalogOperations.setDriver(DriverManager.getDriver("jdbc:sqlite::memory:"));
+
+ Assertions.assertDoesNotThrow(catalogOperations::close);
+ Assertions.assertTrue(catalogOperations.isDeregisterCalled());
+ }
+
+ @Test
+ public void testCloseIgnoreGetDriverException() {
+ TestableJdbcCatalogOperations catalogOperations =
+ new TestableJdbcCatalogOperations(
+ new SqliteExceptionConverter(),
+ new SqliteTypeConverter(),
+ new SqliteDatabaseOperations("/illegal/path"),
+ new SqliteTableOperations(),
+ new SqliteColumnDefaultValueConverter());
+ catalogOperations.setThrowExceptionInGetDriver(true);
+
+ Assertions.assertDoesNotThrow(catalogOperations::close);
+ }
+
+ private static class TestableJdbcCatalogOperations extends
JdbcCatalogOperations {
+ private Driver driver;
+ private boolean deregisterCalled;
+ private boolean throwExceptionInGetDriver;
+
+ private TestableJdbcCatalogOperations(
+ JdbcExceptionConverter exceptionConverter,
+ JdbcTypeConverter jdbcTypeConverter,
+ SqliteDatabaseOperations databaseOperation,
+ SqliteTableOperations tableOperation,
+ SqliteColumnDefaultValueConverter columnDefaultValueConverter) {
+ super(
+ exceptionConverter,
+ jdbcTypeConverter,
+ databaseOperation,
+ tableOperation,
+ columnDefaultValueConverter);
+ }
+
+ @Override
+ protected Driver getDriver() throws SQLException {
+ if (throwExceptionInGetDriver) {
+ throw new SQLException("failed to get driver");
+ }
+ return driver;
+ }
+
+ @Override
+ public void deregisterDriver(Driver driver) {
+ this.deregisterCalled = true;
+ }
+
+ private void setDriver(Driver driver) {
+ this.driver = driver;
+ }
+
+ private void setThrowExceptionInGetDriver(boolean
throwExceptionInGetDriver) {
+ this.throwExceptionInGetDriver = throwExceptionInGetDriver;
+ }
+
+ private boolean isDeregisterCalled() {
+ return deregisterCalled;
+ }
+ }
}
diff --git
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/PostgreSQLCatalogOperations.java
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/PostgreSQLCatalogOperations.java
index 527a27072a..b04ac0dad0 100644
---
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/PostgreSQLCatalogOperations.java
+++
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/PostgreSQLCatalogOperations.java
@@ -20,6 +20,7 @@ package org.apache.gravitino.catalog.postgresql;
import java.sql.Driver;
import java.sql.DriverManager;
+import java.sql.SQLException;
import org.apache.gravitino.catalog.jdbc.JdbcCatalogOperations;
import
org.apache.gravitino.catalog.jdbc.converter.JdbcColumnDefaultValueConverter;
import org.apache.gravitino.catalog.jdbc.converter.JdbcExceptionConverter;
@@ -44,15 +45,7 @@ public class PostgreSQLCatalogOperations extends
JdbcCatalogOperations {
}
@Override
- public void close() {
- super.close();
- try {
- // Unload the PostgreSQL driver, only Unload the driver if it is loaded
by
- // IsolatedClassLoader.
- Driver pgDriver =
DriverManager.getDriver("jdbc:postgresql://dummy_address:12345/");
- deregisterDriver(pgDriver);
- } catch (Exception e) {
- LOG.warn("Failed to deregister PostgreSQL driver", e);
- }
+ protected Driver getDriver() throws SQLException {
+ return DriverManager.getDriver("jdbc:postgresql://dummy_address:12345/");
}
}
