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 <guoqi...@selectdb.com>
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: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to