You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/10/27 16:29:51 UTC

[GitHub] [doris] Jibing-Li opened a new pull request, #13746: [enhancement](multi-catalog)Persist external catalog related metadata.

Jibing-Li opened a new pull request, #13746:
URL: https://github.com/apache/doris/pull/13746

   # Proposed changes
   
   Issue Number: close #xxx
   
   ## Problem summary
   
   Describe your changes.
   
   ## Checklist(Required)
   
   1. Does it affect the original behavior: 
       - [ ] Yes
       - [ ] No
       - [ ] I don't know
   2. Has unit tests been added:
       - [ ] Yes
       - [ ] No
       - [ ] No Need
   3. Has document been added or modified:
       - [ ] Yes
       - [ ] No
       - [ ] No Need
   4. Does it need to update dependencies:
       - [ ] Yes
       - [ ] No
   5. Are there any changes that cannot be rolled back:
       - [ ] Yes (If Yes, please explain WHY)
       - [ ] No
   
   ## Further comments
   
   If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] hello-stephen commented on pull request #13746: [enhancement](multi-catalog)Persist external catalog related metadata.

Posted by GitBox <gi...@apache.org>.
hello-stephen commented on PR #13746:
URL: https://github.com/apache/doris/pull/13746#issuecomment-1295019476

   TeamCity pipeline, clickbench performance test result:
    the sum of best hot time: 39.18 seconds
    load time: 566 seconds
    storage size: 17154644841 Bytes
    https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20221028134451_clickbench_pr_35465.html


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] morningman commented on a diff in pull request #13746: [enhancement](multi-catalog)Persist external catalog related metadata.

Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #13746:
URL: https://github.com/apache/doris/pull/13746#discussion_r1010181566


##########
fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java:
##########
@@ -406,6 +413,105 @@ public void replayAlterCatalogProps(CatalogLog log) {
         }
     }
 
+    public void replayInitCatalog(InitCatalogLog log) {
+        writeLock();
+        try {
+            ExternalCatalog catalog = (ExternalCatalog) idToCatalog.get(log.getCatalogId());
+            for (int i = 0; i < log.getRefreshCount(); i++) {
+                ExternalDatabase db = catalog.getDbNullable(log.getRefreshDbIds().get(i));
+                db.setUnInitialized();
+            }
+            switch (log.getType()) {
+                case HMS:
+                    for (int i = 0; i < log.getCreateCount(); i++) {
+                        catalog.addDb(new HMSExternalDatabase(
+                                catalog, log.getCreateDbIds().get(i), log.getCreateDbNames().get(i)));
+                    }
+                    break;
+                case ES:
+                    for (int i = 0; i < log.getCreateCount(); i++) {
+                        catalog.addDb(new EsExternalDatabase(
+                                catalog, log.getCreateDbIds().get(i), log.getCreateDbNames().get(i)));
+                    }
+                    break;
+                default:
+                    break;
+            }
+            catalog.setInitialized(true);
+        } finally {
+            writeUnlock();
+        }
+    }
+
+    public void replayInitExternalDb(InitDatabaseLog log) {
+        writeLock();
+        try {
+            ExternalCatalog catalog = (ExternalCatalog) idToCatalog.get(log.getCatalogId());
+            ExternalDatabase db = catalog.getDbNullable(log.getDbId());
+            for (int i = 0; i < log.getRefreshCount(); i++) {
+                ExternalTable table = db.getTableNullable(log.getRefreshTableIds().get(i));
+                table.setUnInitialized();
+            }
+            switch (log.getType()) {
+                case HMS:
+                    for (int i = 0; i < log.getCreateCount(); i++) {
+                        db.addTable(new HMSExternalTable(log.getCreateTableIds().get(i),
+                                log.getCreateTableNames().get(i), db.getFullName(), (HMSExternalCatalog) catalog));
+                    }
+                    break;
+                case ES:
+                    for (int i = 0; i < log.getCreateCount(); i++) {
+                        db.addTable(new EsExternalTable(log.getCreateTableIds().get(i),
+                                log.getCreateTableNames().get(i), db.getFullName(), (EsExternalCatalog) catalog));
+                    }
+                    break;
+                default:
+                    break;
+            }
+            db.setInitialized();
+        } finally {
+            writeUnlock();
+        }
+    }
+
+    public void replayInitExternalTable(InitTableLog log) {
+        writeLock();

Review Comment:
   Sink to the `Database` class



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/external/EsExternalDatabase.java:
##########
@@ -53,15 +60,46 @@ public EsExternalDatabase(ExternalCatalog extCatalog, long id, String name) {
         super(extCatalog, id, name);
     }
 
-    private synchronized void makeSureInitialized() {
+    public void addTable(ExternalTable table) {
+        if (!(table instanceof HMSExternalTable)) {
+            LOG.warn("Table {} is not HMSExternalTable.", table.getName());
+            return;
+        }
+        tableNameToId.put(table.getName(), table.getId());
+        idToTbl.put(table.getId(), (EsExternalTable) table);
+    }
+
+    public void setTableExtCatalog(ExternalCatalog extCatalog) {
+        for (EsExternalTable table : idToTbl.values()) {
+            table.setCatalog(extCatalog);
+        }
+    }
+
+    public synchronized void makeSureInitialized() {
         if (!initialized) {
-            init();
+            if (!Env.getCurrentEnv().isMaster()) {
+                tableNameToId = null;
+                idToTbl = null;
+                // Forward to master and wait the journal to replay.
+                MasterCatalogExecutor remoteExecutor = new MasterCatalogExecutor();
+                try {
+                    remoteExecutor.forward(extCatalog.getName(), name, null);
+                } catch (Exception e) {
+                    LOG.warn("Failed to forward init db {} operation to master. {}", name, e.getMessage());
+                }
+                return;
+            }
+            InitDatabaseLog initDatabaseLog = new InitDatabaseLog();
+            init(initDatabaseLog);
             initialized = true;
+            Env.getCurrentEnv().getEditLog().logInitExternalDb(initDatabaseLog);

Review Comment:
   I think we can put the line 92~95 into the `init()` method



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/external/EsExternalDatabase.java:
##########
@@ -53,10 +55,35 @@ public EsExternalDatabase(ExternalCatalog extCatalog, long id, String name) {
         super(extCatalog, id, name);
     }
 
-    private synchronized void makeSureInitialized() {
+    public void addTable(ExternalTable table) {
+        if (!(table instanceof HMSExternalTable)) {

Review Comment:
   should be `EsTable`?



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:
##########
@@ -156,5 +161,10 @@ private void refreshExternalCtlTable(String dbName, String tableName, CatalogIf
             throw new DdlException("Table " + tableName + " does not exist in db " + dbName);
         }
         ((ExternalTable) table).setUnInitialized();
+        ExternalObjectLog log = new ExternalObjectLog();
+        log.setCatalogName(catalog.getName());

Review Comment:
   Better use `id` instead of `name` in edit log



##########
fe/fe-core/src/main/java/org/apache/doris/datasource/EsExternalCatalog.java:
##########
@@ -138,28 +136,67 @@ private void validate(Map<String, String> properties) throws DdlException {
      * Datasource can't be init when creating because the external datasource may depend on third system.
      * So you have to make sure the client of third system is initialized before any method was called.
      */
-    private synchronized void makeSureInitialized() {
+    @Override
+    public synchronized void makeSureInitialized() {
+        if (!objectCreated) {
+            try {
+                validate(catalogProperty.getProperties());
+            } catch (DdlException e) {
+                LOG.warn("validate error", e);
+            }
+            esRestClient = new EsRestClient(this.nodes, this.username, this.password, this.enableSsl);
+            objectCreated = true;
+        }
         if (!initialized) {
-            init();
+            if (!Env.getCurrentEnv().isMaster()) {
+                dbNameToId = null;
+                idToDb = null;
+                // Forward to master and wait the journal to replay.
+                MasterCatalogExecutor remoteExecutor = new MasterCatalogExecutor();
+                try {
+                    remoteExecutor.forward(name, null, null);
+                } catch (Exception e) {
+                    LOG.warn("Failed to forward init catalog {} operation to master. {}", name, e.getMessage());
+                }
+                return;
+            }
+            InitCatalogLog initCatalogLog = new InitCatalogLog();
+            init(initCatalogLog);
             initialized = true;
+            Env.getCurrentEnv().getEditLog().logInitCatalog(initCatalogLog);

Review Comment:
   Move line 163~166 into the `init()` method.



##########
fe/fe-core/src/main/java/org/apache/doris/datasource/EsExternalCatalog.java:
##########
@@ -138,28 +136,67 @@ private void validate(Map<String, String> properties) throws DdlException {
      * Datasource can't be init when creating because the external datasource may depend on third system.
      * So you have to make sure the client of third system is initialized before any method was called.
      */
-    private synchronized void makeSureInitialized() {
+    @Override
+    public synchronized void makeSureInitialized() {
+        if (!objectCreated) {
+            try {
+                validate(catalogProperty.getProperties());
+            } catch (DdlException e) {
+                LOG.warn("validate error", e);
+            }
+            esRestClient = new EsRestClient(this.nodes, this.username, this.password, this.enableSsl);
+            objectCreated = true;
+        }
         if (!initialized) {
-            init();
+            if (!Env.getCurrentEnv().isMaster()) {
+                dbNameToId = null;
+                idToDb = null;
+                // Forward to master and wait the journal to replay.
+                MasterCatalogExecutor remoteExecutor = new MasterCatalogExecutor();
+                try {
+                    remoteExecutor.forward(name, null, null);
+                } catch (Exception e) {
+                    LOG.warn("Failed to forward init catalog {} operation to master. {}", name, e.getMessage());
+                }
+                return;
+            }
+            InitCatalogLog initCatalogLog = new InitCatalogLog();
+            init(initCatalogLog);
             initialized = true;
+            Env.getCurrentEnv().getEditLog().logInitCatalog(initCatalogLog);
+        }
+    }
+
+    @Override
+    public void addDb(ExternalDatabase db) {
+        if (!(db instanceof HMSExternalDatabase)) {

Review Comment:
   ```suggestion
           if (!(db instanceof EsExternalDatabase)) {
   ```



##########
fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java:
##########
@@ -406,6 +413,105 @@ public void replayAlterCatalogProps(CatalogLog log) {
         }
     }
 
+    public void replayInitCatalog(InitCatalogLog log) {
+        writeLock();
+        try {
+            ExternalCatalog catalog = (ExternalCatalog) idToCatalog.get(log.getCatalogId());
+            for (int i = 0; i < log.getRefreshCount(); i++) {
+                ExternalDatabase db = catalog.getDbNullable(log.getRefreshDbIds().get(i));
+                db.setUnInitialized();
+            }
+            switch (log.getType()) {
+                case HMS:
+                    for (int i = 0; i < log.getCreateCount(); i++) {
+                        catalog.addDb(new HMSExternalDatabase(
+                                catalog, log.getCreateDbIds().get(i), log.getCreateDbNames().get(i)));
+                    }
+                    break;
+                case ES:
+                    for (int i = 0; i < log.getCreateCount(); i++) {
+                        catalog.addDb(new EsExternalDatabase(
+                                catalog, log.getCreateDbIds().get(i), log.getCreateDbNames().get(i)));
+                    }
+                    break;
+                default:
+                    break;
+            }
+            catalog.setInitialized(true);
+        } finally {
+            writeUnlock();
+        }
+    }
+
+    public void replayInitExternalDb(InitDatabaseLog log) {
+        writeLock();

Review Comment:
   The lock scope is too large for initializing a database.
   I think we can sink the `replayInitExternalDb()` to the Database class.



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/external/EsExternalDatabase.java:
##########
@@ -53,15 +60,46 @@ public EsExternalDatabase(ExternalCatalog extCatalog, long id, String name) {
         super(extCatalog, id, name);
     }
 
-    private synchronized void makeSureInitialized() {
+    public void addTable(ExternalTable table) {
+        if (!(table instanceof HMSExternalTable)) {
+            LOG.warn("Table {} is not HMSExternalTable.", table.getName());
+            return;
+        }
+        tableNameToId.put(table.getName(), table.getId());
+        idToTbl.put(table.getId(), (EsExternalTable) table);
+    }
+
+    public void setTableExtCatalog(ExternalCatalog extCatalog) {
+        for (EsExternalTable table : idToTbl.values()) {
+            table.setCatalog(extCatalog);
+        }
+    }
+
+    public synchronized void makeSureInitialized() {
         if (!initialized) {
-            init();
+            if (!Env.getCurrentEnv().isMaster()) {
+                tableNameToId = null;
+                idToTbl = null;
+                // Forward to master and wait the journal to replay.
+                MasterCatalogExecutor remoteExecutor = new MasterCatalogExecutor();
+                try {
+                    remoteExecutor.forward(extCatalog.getName(), name, null);

Review Comment:
   Better to use ID instead of NAME



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] morningman commented on a diff in pull request #13746: [enhancement](multi-catalog)Persist external catalog related metadata.

Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #13746:
URL: https://github.com/apache/doris/pull/13746#discussion_r1007527277


##########
fe/fe-core/src/main/java/org/apache/doris/qe/MasterCatalogExecutor.java:
##########
@@ -0,0 +1,76 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.qe;
+
+import org.apache.doris.common.ClientPool;
+import org.apache.doris.thrift.FrontendService;
+import org.apache.doris.thrift.TInitExternalCtlMetaRequest;
+import org.apache.doris.thrift.TInitExternalCtlMetaResult;
+import org.apache.doris.thrift.TNetworkAddress;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+public class MasterCatalogExecutor {

Review Comment:
   Add some comment for this class



##########
gensrc/thrift/FrontendService.thrift:
##########
@@ -678,6 +678,17 @@ struct TWaitingTxnStatusResult {
     2: optional i32 txn_status_id
 }
 
+struct TInitExternalCtlMetaRequest {
+    1: optional string catalogName
+    2: optional string dbName
+    3: optional string tableName
+}
+
+struct TInitExternalCtlMetaResult {
+    1: required i64 maxJournalId;

Review Comment:
   ```suggestion
       1: optional i64 maxJournalId;
   ```



##########
fe/fe-core/src/main/java/org/apache/doris/datasource/HMSExternalCatalog.java:
##########
@@ -105,10 +106,24 @@ private void init() {
      * Catalog can't be init when creating because the external catalog may depend on third system.
      * So you have to make sure the client of third system is initialized before any method was called.
      */
-    private synchronized void makeSureInitialized() {
+    @Override
+    public synchronized void makeSureInitialized() {
         if (!initialized) {
+            if (!Env.getCurrentEnv().isMaster()) {
+                // Forward to master and wait the journal to replay.
+                MasterCatalogExecutor remoteExecutor = new MasterCatalogExecutor();
+                try {
+                    remoteExecutor.forward(name, null, null);
+                } catch (Exception e) {
+                    LOG.warn("Failed to forward init operation to master. {}", e.getMessage());
+                }
+                return;
+            }
             init();
             initialized = true;
+            ExternalObjectInitLog log = new ExternalObjectInitLog();

Review Comment:
   Missing database info?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] morningman merged pull request #13746: [enhancement](multi-catalog)Persist external catalog related metadata.

Posted by GitBox <gi...@apache.org>.
morningman merged PR #13746:
URL: https://github.com/apache/doris/pull/13746


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] hello-stephen commented on pull request #13746: [enhancement](multi-catalog)Persist external catalog related metadata.

Posted by GitBox <gi...@apache.org>.
hello-stephen commented on PR #13746:
URL: https://github.com/apache/doris/pull/13746#issuecomment-1295185137

   TeamCity pipeline, clickbench performance test result:
    the sum of best hot time: 38.83 seconds
    load time: 554 seconds
    storage size: 17154644878 Bytes
    https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20221028161416_clickbench_pr_35509.html


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] hello-stephen commented on pull request #13746: [enhancement](multi-catalog)Persist external catalog related metadata.

Posted by GitBox <gi...@apache.org>.
hello-stephen commented on PR #13746:
URL: https://github.com/apache/doris/pull/13746#issuecomment-1294494588

   TeamCity pipeline, clickbench performance test result:
    the sum of best hot time: 40.2 seconds
    load time: 560 seconds
    storage size: 17154655374 Bytes
    https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20221028055207_clickbench_pr_35170.html


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] hello-stephen commented on pull request #13746: [enhancement](multi-catalog)Persist external catalog related metadata.

Posted by GitBox <gi...@apache.org>.
hello-stephen commented on PR #13746:
URL: https://github.com/apache/doris/pull/13746#issuecomment-1294527584

   TeamCity pipeline, clickbench performance test result:
    the sum of best hot time: 38.38 seconds
    load time: 559 seconds
    storage size: 17154699320 Bytes
    https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20221028063708_clickbench_pr_35189.html


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] github-actions[bot] commented on pull request #13746: [enhancement](multi-catalog)Persist external catalog related metadata.

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13746:
URL: https://github.com/apache/doris/pull/13746#issuecomment-1302530150

   PR approved by anyone and no changes requested.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] hello-stephen commented on pull request #13746: [enhancement](multi-catalog)Persist external catalog related metadata.

Posted by GitBox <gi...@apache.org>.
hello-stephen commented on PR #13746:
URL: https://github.com/apache/doris/pull/13746#issuecomment-1295241525

   TeamCity pipeline, clickbench performance test result:
    the sum of best hot time: 39.06 seconds
    load time: 560 seconds
    storage size: 17154644851 Bytes
    https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20221028170437_clickbench_pr_35529.html


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] github-actions[bot] commented on pull request #13746: [enhancement](multi-catalog)Persist external catalog related metadata.

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13746:
URL: https://github.com/apache/doris/pull/13746#issuecomment-1302530092

   PR approved by at least one committer and no changes requested.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [doris] morningman commented on a diff in pull request #13746: [enhancement](multi-catalog)Persist external catalog related metadata.

Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #13746:
URL: https://github.com/apache/doris/pull/13746#discussion_r1011352564


##########
fe/fe-core/src/main/java/org/apache/doris/catalog/external/EsExternalDatabase.java:
##########
@@ -60,13 +60,24 @@ public EsExternalDatabase(ExternalCatalog extCatalog, long id, String name) {
         super(extCatalog, id, name);
     }
 
-    public void addTable(ExternalTable table) {
-        if (!(table instanceof EsExternalTable)) {
-            LOG.warn("Table {} is not EsExternalTable.", table.getName());
-            return;
+    public synchronized void replayInitDb(InitDatabaseLog log, ExternalCatalog catalog) {

Review Comment:
   Do we need `synchronized`?



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/external/EsExternalDatabase.java:
##########
@@ -135,12 +145,14 @@ public Set<String> getTableNamesWithLock() {
     @Override
     public List<EsExternalTable> getTables() {
         makeSureInitialized();
+        MasterCatalogExecutor.waitJournalReplay(replayToJournalId);

Review Comment:
   Why not move `MasterCatalogExecutor.waitJournalReplay(replayToJournalId);` into `makeSureInitialized()`?



##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java:
##########
@@ -85,12 +93,20 @@ public abstract class ExternalCatalog implements CatalogIf<ExternalDatabase>, Wr
 
     public abstract void makeSureInitialized();
 
-    public abstract void addDb(ExternalDatabase db);
-
     public void setInitialized(boolean initialized) {
         this.initialized = initialized;
     }
 
+    protected void waitJournalReplay() {
+        if (!Env.getCurrentEnv().isMaster()) {
+            try {
+                ConnectContext.get().getEnv().getJournalObservable().waitOn(replayToJournalId, 3000);

Review Comment:
   I think this should be done in `MasterCatalogExecutor`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org