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
The following commit(s) were added to refs/heads/branch-3.0 by this push:
new ab89bb6d53c branch-3.0: [Fix](Catalog) Close system resources when
dropping catalog (#49621) (#49936)
ab89bb6d53c is described below
commit ab89bb6d53c39d3fd102c88bb8ea764224e8e281
Author: Calvin Kirs <[email protected]>
AuthorDate: Thu Apr 24 09:25:22 2025 +0800
branch-3.0: [Fix](Catalog) Close system resources when dropping catalog
(#49621) (#49936)
bp #49621
---
.../org/apache/doris/datasource/CatalogIf.java | 2 +-
.../org/apache/doris/datasource/CatalogMgr.java | 2 +-
.../apache/doris/datasource/ExternalCatalog.java | 25 +++++++++++++++++++++-
.../doris/datasource/hive/HMSExternalCatalog.java | 15 +++++++++++--
.../datasource/iceberg/IcebergExternalCatalog.java | 8 +++++++
.../datasource/iceberg/IcebergMetadataOps.java | 3 +++
.../doris/datasource/jdbc/JdbcExternalCatalog.java | 8 ++-----
.../mysql/privilege/AccessControllerManager.java | 8 ++++++-
.../doris/datasource/RefreshCatalogTest.java | 2 +-
9 files changed, 60 insertions(+), 13 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogIf.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogIf.java
index a11061afa38..d528dd84821 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogIf.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogIf.java
@@ -94,7 +94,7 @@ public interface CatalogIf<T extends DatabaseIf> {
default void notifyPropertiesUpdated(Map<String, String> updatedProps) {
if (this instanceof ExternalCatalog) {
- ((ExternalCatalog) this).onRefresh(false);
+ ((ExternalCatalog) this).resetToUninitialized(false);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java
index 8ed93b78fd0..26bfad011e7 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java
@@ -124,7 +124,7 @@ public class CatalogMgr implements Writable,
GsonPostProcessable {
idToCatalog.put(catalog.getId(), catalog);
String catalogName = catalog.getName();
if (!catalogName.equals(InternalCatalog.INTERNAL_CATALOG_NAME)) {
- ((ExternalCatalog) catalog).onRefresh(false);
+ ((ExternalCatalog) catalog).resetToUninitialized(false);
}
if (!Strings.isNullOrEmpty(catalog.getResource())) {
Resource resource =
Env.getCurrentEnv().getResourceMgr().getResource(catalog.getResource());
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
index 0b6124591da..9908e539b4b 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
@@ -494,7 +494,23 @@ public abstract class ExternalCatalog
return remoteToLocalPairs;
}
- public void onRefresh(boolean invalidCache) {
+ /**
+ * Resets the Catalog state to uninitialized, releases resources held by
{@code initLocalObjectsImpl()}
+ * <p>
+ * This method is typically invoked during operations such as {@code
CREATE CATALOG}
+ * and {@code MODIFY CATALOG}. It marks the object as uninitialized,
clears cached
+ * configurations, and ensures that resources allocated during {@link
#initLocalObjectsImpl()}
+ * are properly released via {@link #onClose()}
+ * </p>
+ * <p>
+ * The {@code onClose()} method is responsible for cleaning up resources
that were initialized
+ * in {@code initLocalObjectsImpl()}, preventing potential resource leaks.
+ * </p>
+ *
+ * @param invalidCache if {@code true}, the catalog cache will be
invalidated
+ * and reloaded during the refresh process.
+ */
+ public void resetToUninitialized(boolean invalidCache) {
this.objectCreated = false;
this.initialized = false;
synchronized (this.propLock) {
@@ -504,6 +520,7 @@ public abstract class ExternalCatalog
synchronized (this.confLock) {
this.cachedConf = null;
}
+ onClose();
refreshOnlyCatalogCache(invalidCache);
}
@@ -717,6 +734,12 @@ public abstract class ExternalCatalog
@Override
public void onClose() {
removeAccessController();
+ if (null != preExecutionAuthenticator) {
+ preExecutionAuthenticator = null;
+ }
+ if (null != transactionManager) {
+ transactionManager = null;
+ }
CatalogIf.super.onClose();
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java
index abd099894a3..505436903ce 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java
@@ -197,13 +197,24 @@ public class HMSExternalCatalog extends ExternalCatalog {
}
@Override
- public void onRefresh(boolean invalidCache) {
- super.onRefresh(invalidCache);
+ public void resetToUninitialized(boolean invalidCache) {
+ super.resetToUninitialized(invalidCache);
if (metadataOps != null) {
metadataOps.close();
}
}
+ @Override
+ public void onClose() {
+ super.onClose();
+ if (null != fileSystemExecutor) {
+ fileSystemExecutor.shutdown();
+ }
+ if (null != icebergMetadataOps) {
+ icebergMetadataOps.close();
+ }
+ }
+
@Override
public List<String> listTableNames(SessionContext ctx, String dbName) {
makeSureInitialized();
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalCatalog.java
index 0fa69825a01..523c31c3f74 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalCatalog.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalCatalog.java
@@ -82,6 +82,14 @@ public abstract class IcebergExternalCatalog extends
ExternalCatalog {
return metadataOps.listTableNames(dbName);
}
+ @Override
+ public void onClose() {
+ super.onClose();
+ if (null != catalog) {
+ catalog = null;
+ }
+ }
+
protected void initS3Param(Configuration conf) {
Map<String, String> properties = catalogProperty.getHadoopProperties();
conf.set(Constants.AWS_CREDENTIALS_PROVIDER,
PropertyConverter.getAWSCredentialsProviders(properties));
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java
index bf07284a6d8..787d706132e 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java
@@ -87,6 +87,9 @@ public class IcebergMetadataOps implements
ExternalMetadataOps {
@Override
public void close() {
+ if (catalog != null) {
+ catalog = null;
+ }
}
@Override
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java
index 03554dafbcb..b63685d9fb3 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java
@@ -125,12 +125,8 @@ public class JdbcExternalCatalog extends ExternalCatalog {
}
@Override
- public void onRefresh(boolean invalidCache) {
- super.onRefresh(invalidCache);
- if (jdbcClient != null) {
- jdbcClient.closeClient();
- jdbcClient = null;
- }
+ public void resetToUninitialized(boolean invalidCache) {
+ super.resetToUninitialized(invalidCache);
this.identifierMapping = new JdbcIdentifierMapping(
(Env.isTableNamesCaseInsensitive() ||
Env.isStoredTableNamesLowerCase()),
Boolean.parseBoolean(getLowerCaseMetaNames()),
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/AccessControllerManager.java
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/AccessControllerManager.java
index 23845982679..86aad9af71e 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/AccessControllerManager.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/AccessControllerManager.java
@@ -32,6 +32,7 @@ import org.apache.doris.qe.ConnectContext;
import com.google.common.base.Preconditions;
import com.google.common.collect.Maps;
+import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -114,7 +115,12 @@ public class AccessControllerManager {
}
public void removeAccessController(String ctl) {
- ctlToCtlAccessController.remove(ctl);
+ if (StringUtils.isBlank(ctl)) {
+ return;
+ }
+ if (ctlToCtlAccessController.containsKey(ctl)) {
+ ctlToCtlAccessController.remove(ctl);
+ }
LOG.info("remove access controller for catalog {}", ctl);
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/RefreshCatalogTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/RefreshCatalogTest.java
index 89994c36142..39a6394477e 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/RefreshCatalogTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/RefreshCatalogTest.java
@@ -102,7 +102,7 @@ public class RefreshCatalogTest extends TestWithFeService {
Thread.sleep(5000);
// there are test1.db1 , test1.db2 , test1.db3, information_schema,
mysql
List<String> dbNames2 = test1.getDbNames();
- Assertions.assertEquals(5, dbNames2.size());
+ Assertions.assertEquals(4, dbNames2.size());
ExternalInfoSchemaDatabase infoDb = (ExternalInfoSchemaDatabase)
test1.getDb(InfoSchemaDb.DATABASE_NAME).get();
Assertions.assertEquals(SchemaTable.TABLE_MAP.size(),
infoDb.getTables().size());
TestExternalDatabase testDb = (TestExternalDatabase)
test1.getDb("db1").get();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
