This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.1 by this push:
new 78b8f318957 branch-2.1 [Fix](Catalog) Close system resources when
dropping catalog ( #49621) (#49935)
78b8f318957 is described below
commit 78b8f31895777cd3d1d5d090c72d8260b9434fe9
Author: Calvin Kirs <[email protected]>
AuthorDate: Thu Apr 10 23:23:51 2025 +0800
branch-2.1 [Fix](Catalog) Close system resources when dropping catalog (
#49621) (#49935)
https://github.com/apache/doris/pull/49621
(cherry picked from commit
https://github.com/apache/doris/commit/902b95b1bfa144d089d88cdc8728943cfb1ab9da)
---
.../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 cd11d9424e8..3a4d32b88f6 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 f7785cf913b..875c8142a68 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
@@ -498,7 +498,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) {
@@ -508,6 +524,7 @@ public abstract class ExternalCatalog
synchronized (this.confLock) {
this.cachedConf = null;
}
+ onClose();
refreshOnlyCatalogCache(invalidCache);
}
@@ -721,6 +738,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 2c80901da4a..eca6d856cac 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
@@ -190,13 +190,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 b5322e292ee..4c06068aa3b 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
@@ -86,6 +86,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 74447fdd5a2..b09f9b241c9 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
@@ -124,12 +124,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 42fa769d033..4f2e8813883 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
@@ -31,6 +31,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;
@@ -113,7 +114,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 663eacb7098..1caedbf30ee 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
@@ -101,7 +101,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.assertTrue(infoDb.getTables().size() >= 33);
TestExternalDatabase testDb = (TestExternalDatabase)
test1.getDb("db1").get();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
