[opt](catalog) cache the Configuration object (#45433)(#45756) (#45759)

cherry-pick (#45433)(#45756)
apache · Dec 21, 2024 · 8861e67 · 8861e67
1 parent c4bee9a
commit 8861e67
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 10 deletions.
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
@@ -152,6 +152,9 @@ public abstract class ExternalCatalog
     protected MetaCache<ExternalDatabase<? extends ExternalTable>> metaCache;
     protected PreExecutionAuthenticator preExecutionAuthenticator;
 
+    private volatile Configuration cachedConf = null;
+    private byte[] confLock = new byte[0];
+
     public ExternalCatalog() {
     }
 
@@ -163,6 +166,20 @@ public ExternalCatalog(long catalogId, String name, InitCatalogLog.Type logType,
     }
 
     public Configuration getConfiguration() {
+        // build configuration is costly, so we cache it.
+        if (cachedConf != null) {
+            return cachedConf;
+        }
+        synchronized (confLock) {
+            if (cachedConf != null) {
+                return cachedConf;
+            }
+            cachedConf = buildConf();
+            return cachedConf;
+        }
+    }
+
+    private Configuration buildConf() {
         Configuration conf = DFSFileSystem.getHdfsConf(ifNotSetFallbackToSimpleAuth());
         Map<String, String> catalogProperties = catalogProperty.getHadoopProperties();
         for (Map.Entry<String, String> entry : catalogProperties.entrySet()) {
@@ -408,6 +425,10 @@ public void onRefresh(boolean invalidCache) {
             this.convertedProperties = null;
         }
 
+        synchronized (this.confLock) {
+            this.cachedConf = null;
+        }
+
         refreshOnlyCatalogCache(invalidCache);
     }
 
@@ -762,6 +783,7 @@ public void gsonPostProcess() throws IOException {
             }
         }
         this.propLock = new byte[0];
+        this.confLock = new byte[0];
         this.initialized = false;
         setDefaultPropsIfMissing(true);
         if (tableAutoAnalyzePolicy == null) {

diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HiveMetaStoreClientHelper.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HiveMetaStoreClientHelper.java
@@ -42,7 +42,6 @@
 import org.apache.doris.common.security.authentication.AuthenticationConfig;
 import org.apache.doris.common.security.authentication.HadoopAuthenticator;
 import org.apache.doris.datasource.ExternalCatalog;
-import org.apache.doris.fs.remote.dfs.DFSFileSystem;
 import org.apache.doris.thrift.TExprOpcode;
 
 import com.google.common.base.Strings;
@@ -843,11 +842,7 @@ public static HoodieTableMetaClient getHudiClient(HMSExternalTable table) {
     }
 
     public static Configuration getConfiguration(HMSExternalTable table) {
-        Configuration conf = DFSFileSystem.getHdfsConf(table.getCatalog().ifNotSetFallbackToSimpleAuth());
-        for (Map.Entry<String, String> entry : table.getHadoopProperties().entrySet()) {
-            conf.set(entry.getKey(), entry.getValue());
-        }
-        return conf;
+        return table.getCatalog().getConfiguration();
     }
 
     public static Optional<String> getSerdeProperty(Table table, String key) {

diff --git a/fe/fe-core/src/test/java/org/apache/doris/datasource/ExternalCatalogTest.java b/fe/fe-core/src/test/java/org/apache/doris/datasource/ExternalCatalogTest.java
@@ -22,26 +22,31 @@
 import org.apache.doris.catalog.Env;
 import org.apache.doris.catalog.PrimitiveType;
 import org.apache.doris.common.FeConstants;
+import org.apache.doris.common.FeMetaVersion;
 import org.apache.doris.datasource.hive.HMSExternalCatalog;
 import org.apache.doris.datasource.test.TestExternalCatalog;
-import org.apache.doris.mysql.privilege.Auth;
+import org.apache.doris.meta.MetaContext;
 import org.apache.doris.qe.ConnectContext;
 import org.apache.doris.qe.QueryState.MysqlStateType;
 import org.apache.doris.qe.StmtExecutor;
 import org.apache.doris.utframe.TestWithFeService;
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
+import org.apache.hadoop.conf.Configuration;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.File;
+import java.nio.file.Files;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
 public class ExternalCatalogTest extends TestWithFeService {
-    private static Auth auth;
-    private static Env env;
+    private Env env;
     private CatalogMgr mgr;
     private ConnectContext rootCtx;
 
@@ -51,7 +56,6 @@ protected void runBeforeAll() throws Exception {
         mgr = Env.getCurrentEnv().getCatalogMgr();
         rootCtx = createDefaultCtx();
         env = Env.getCurrentEnv();
-        auth = env.getAuth();
         // 1. create test catalog
         CreateCatalogStmt testCatalog = (CreateCatalogStmt) parseAndAnalyzeStmt(
                 "create catalog test1 properties(\n"
@@ -244,4 +248,32 @@ public Map<String, Map<String, List<Column>>> getMetadata() {
             return MOCKED_META;
         }
     }
+
+    @Test
+    public void testSerialization() throws Exception {
+        MetaContext metaContext = new MetaContext();
+        metaContext.setMetaVersion(FeMetaVersion.VERSION_CURRENT);
+        metaContext.setThreadLocalInfo();
+
+        // 1. Write objects to file
+        File file = new File("./external_catalog_persist_test.dat");
+        file.createNewFile();
+        DataOutputStream dos = new DataOutputStream(Files.newOutputStream(file.toPath()));
+
+        TestExternalCatalog ctl = (TestExternalCatalog) mgr.getCatalog("test1");
+        ctl.write(dos);
+        dos.flush();
+        dos.close();
+
+        // 2. Read objects from file
+        DataInputStream dis = new DataInputStream(Files.newInputStream(file.toPath()));
+
+        TestExternalCatalog ctl2 = (TestExternalCatalog) ExternalCatalog.read(dis);
+        Configuration conf = ctl2.getConfiguration();
+        Assertions.assertNotNull(conf);
+
+        // 3. delete files
+        dis.close();
+        file.delete();
+    }
 }