This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch branch-1.2
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.2 by this push:
new f7f9bc249b [Cherry-pick to branch-1.2] [#10253] Optimize driver
degisteration logic for JDBC catalog to avoid possible OOM problem (#10255)
(#10310)
f7f9bc249b is described below
commit f7f9bc249b135cb147e4bfb628ea4a646dc20952
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Mon Mar 9 19:56:40 2026 +0800
[Cherry-pick to branch-1.2] [#10253] Optimize driver degisteration logic
for JDBC catalog to avoid possible OOM problem (#10255) (#10310)
**Cherry-pick Information:**
- Original commit: 892167250926a5c6c8694d497da4b0866cd98a27
- Target branch: `branch-1.2`
- Status: ⚠️ **Has conflicts - manual resolution required**
Please review and resolve the conflicts before merging.
Co-authored-by: Qi Yu <[email protected]>
Co-authored-by: Copilot <[email protected]>
---
.../operations/ClickHouseCatalogOperations.java | 15 +----
.../hologres/HologresCatalogOperations.java | 25 ++++---
.../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, 129 insertions(+), 43 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/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/PostgreSQLCatalogOperations.java
b/catalogs-contrib/catalog-jdbc-hologres/src/main/java/org/apache/gravitino/catalog/hologres/HologresCatalogOperations.java
similarity index 76%
copy from
catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/PostgreSQLCatalogOperations.java
copy to
catalogs-contrib/catalog-jdbc-hologres/src/main/java/org/apache/gravitino/catalog/hologres/HologresCatalogOperations.java
index 527a27072a..2c2442569e 100644
---
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/PostgreSQLCatalogOperations.java
+++
b/catalogs-contrib/catalog-jdbc-hologres/src/main/java/org/apache/gravitino/catalog/hologres/HologresCatalogOperations.java
@@ -16,10 +16,11 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.gravitino.catalog.postgresql;
+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;
@@ -27,9 +28,15 @@ import
org.apache.gravitino.catalog.jdbc.converter.JdbcTypeConverter;
import org.apache.gravitino.catalog.jdbc.operation.JdbcDatabaseOperations;
import org.apache.gravitino.catalog.jdbc.operation.JdbcTableOperations;
-public class PostgreSQLCatalogOperations extends JdbcCatalogOperations {
+/**
+ * Hologres catalog operations implementation.
+ *
+ * <p>Since Hologres uses the PostgreSQL JDBC driver, this class handles the
driver deregistration
+ * properly to avoid memory leaks when the catalog is closed.
+ */
+public class HologresCatalogOperations extends JdbcCatalogOperations {
- public PostgreSQLCatalogOperations(
+ public HologresCatalogOperations(
JdbcExceptionConverter exceptionConverter,
JdbcTypeConverter jdbcTypeConverter,
JdbcDatabaseOperations databaseOperation,
@@ -44,15 +51,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/");
}
}
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/");
}
}
