From 8bada3c1be3ccc0708a8d8c72c0405aab9643ac1 Mon Sep 17 00:00:00 2001 From: yujun777 Date: Mon, 8 Jul 2024 17:43:49 +0800 Subject: [PATCH 1/9] fix no drop table --- .../doris/datasource/InternalCatalog.java | 28 +++++++++++---- .../doris/persist/ColocatePersistInfo.java | 4 +++ .../test_create_table_exception.groovy | 36 ++++++++++++------- 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java index ea9d8e7610f353..9a8a2bb5e8f76b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java @@ -1001,7 +1001,7 @@ public boolean unprotectDropTable(Database db, Table table, boolean isForceDrop, return true; } - public void dropTable(Database db, long tableId, boolean isForceDrop, + private void dropTable(Database db, long tableId, boolean isForceDrop, Long recycleTime) throws MetaNotFoundException { Table table = db.getTableOrMetaException(tableId); db.writeLock(); @@ -2804,6 +2804,8 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx // if failed in any step, use this set to do clear things Set tabletIdSet = new HashSet<>(); // create partition + boolean editlogCreateTable = false; + boolean editlogColocateAddTable = false; try { if (partitionInfo.getType() == PartitionType.UNPARTITIONED) { if (properties != null && !properties.isEmpty()) { @@ -2935,11 +2937,9 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx if (!result.first) { ErrorReport.reportDdlException(ErrorCode.ERR_TABLE_EXISTS_ERROR, tableName); } - if (DebugPointUtil.isEnable("FE.createOlapTable.exception")) { - LOG.info("debug point FE.createOlapTable.exception, throw e"); - // not commit, not log edit - throw new DdlException("debug point FE.createOlapTable.exception"); - } + + // if table not exists, then db.createTableWithLock will write an editlog. + editlogCreateTable = !result.second; if (result.second) { if (Env.getCurrentColocateIndex().isColocateTable(tableId)) { @@ -2960,6 +2960,7 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx ColocatePersistInfo info = ColocatePersistInfo.createForAddTable(groupId, tableId, backendsPerBucketSeq); Env.getCurrentEnv().getEditLog().logColocateAddTable(info); + editlogColocateAddTable = true; } LOG.info("successfully create table[{};{}]", tableName, tableId); Env.getCurrentEnv().getDynamicPartitionScheduler() @@ -2970,17 +2971,30 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx .createOrUpdateRuntimeInfo(tableId, DynamicPartitionScheduler.LAST_UPDATE_TIME, TimeUtils.getCurrentFormatTime()); } + + if (DebugPointUtil.isEnable("FE.createOlapTable.exception")) { + LOG.info("debug point FE.createOlapTable.exception, throw e"); + throw new DdlException("debug point FE.createOlapTable.exception"); + } } catch (DdlException e) { LOG.warn("create table failed {} - {}", tabletIdSet, e.getMessage()); for (Long tabletId : tabletIdSet) { Env.getCurrentInvertedIndex().deleteTablet(tabletId); } - // only remove from memory, because we have not persist it if (Env.getCurrentColocateIndex().isColocateTable(tableId)) { + GroupId groupId = Env.getCurrentColocateIndex().getGroup(tableId); Env.getCurrentColocateIndex().removeTable(tableId); + if (editlogColocateAddTable) { + ColocatePersistInfo info = ColocatePersistInfo.createForRemoveTable(groupId, tableId); + Env.getCurrentEnv().getEditLog().logColocateRemoveTable(info); + } } try { dropTable(db, tableId, true, 0L); + if (editlogCreateTable) { + DropInfo info = new DropInfo(db.getId(), tableId, olapTable.getName(), -1L, true, 0L); + Env.getCurrentEnv().getEditLog().logDropTable(info); + } } catch (Exception ex) { LOG.warn("drop table", ex); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/ColocatePersistInfo.java b/fe/fe-core/src/main/java/org/apache/doris/persist/ColocatePersistInfo.java index 429d4e0e1a6b94..c3380cdfba2ab4 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/persist/ColocatePersistInfo.java +++ b/fe/fe-core/src/main/java/org/apache/doris/persist/ColocatePersistInfo.java @@ -62,6 +62,10 @@ public static ColocatePersistInfo createForAddTable(GroupId groupId, long tableI return new ColocatePersistInfo(groupId, tableId, backendsPerBucketSeq, new ReplicaAllocation()); } + public static ColocatePersistInfo createForRemoveTable(GroupId groupId, long tableId) { + return new ColocatePersistInfo(groupId, tableId, Maps.newHashMap(), new ReplicaAllocation()); + } + public static ColocatePersistInfo createForBackendsPerBucketSeq(GroupId groupId, Map>> backendsPerBucketSeq) { return new ColocatePersistInfo(groupId, -1L, backendsPerBucketSeq, new ReplicaAllocation()); diff --git a/regression-test/suites/partition_p0/test_create_table_exception.groovy b/regression-test/suites/partition_p0/test_create_table_exception.groovy index 49cadcd3af4bf9..46c2fa9b2b9796 100644 --- a/regression-test/suites/partition_p0/test_create_table_exception.groovy +++ b/regression-test/suites/partition_p0/test_create_table_exception.groovy @@ -31,9 +31,9 @@ suite("test_create_table_exception") { def table3 = "dynamic_partition_table" try { GetDebugPoint().enableDebugPointForAllFEs('FE.createOlapTable.exception', null) - def createTable = { -> + def createTable = { tableIdx -> try_sql """ - CREATE TABLE $table1 ( + CREATE TABLE ${table1}_${tableIdx} ( `k1` int(11) NULL, `k2` int(11) NULL ) @@ -41,12 +41,13 @@ suite("test_create_table_exception") { COMMENT 'OLAP' DISTRIBUTED BY HASH(`k1`) BUCKETS 10 PROPERTIES ( + "colocate_with" = "col_grp_${tableIdx}", "replication_num"="3" ); """ try_sql """ - CREATE TABLE IF NOT EXISTS $table2 ( + CREATE TABLE IF NOT EXISTS ${table2}_${tableIdx} ( lo_orderdate int(11) NOT NULL COMMENT "", lo_orderkey bigint(20) NOT NULL COMMENT "", lo_linenumber bigint(20) NOT NULL COMMENT "", @@ -79,7 +80,7 @@ suite("test_create_table_exception") { """ try_sql """ - CREATE TABLE $table3 ( + CREATE TABLE ${table3}_${tableIdx} ( time date, key1 int, key2 int, @@ -109,19 +110,30 @@ suite("test_create_table_exception") { ); """ } - createTable() + createTable(1) def result = sql """show tables;""" assertEquals(result.size(), 0) + + def checkResult = { -> + def tables = sql """show tables;""" + log.info(tables.toString()) + assertEquals(3, tables.size()) + + def groups = sql """ show proc "/colocation_group" """ + log.info(groups) + assertEquals(1, groups.size()) + } + GetDebugPoint().disableDebugPointForAllFEs('FE.createOlapTable.exception') - createTable() - result = sql """show tables;""" - log.info(result.toString()) - assertEquals(result.size(), 3) + createTable(2) + checkResult() + + cluster.restartFrontends(cluster.getMasterFe().index) + sleep 30_000 + reconnectFe() + checkResult() } finally { GetDebugPoint().disableDebugPointForAllFEs('FE.createOlapTable.exception') - sql """drop table if exists ${table1}""" - sql """drop table if exists ${table2}""" - sql """drop table if exists ${table3}""" } } } From af0ff334995d76781628f83db792ccc1b18ace4f Mon Sep 17 00:00:00 2001 From: yujun777 Date: Mon, 8 Jul 2024 18:44:09 +0800 Subject: [PATCH 2/9] no need log drop colocate --- .../java/org/apache/doris/datasource/InternalCatalog.java | 7 +------ .../java/org/apache/doris/persist/ColocatePersistInfo.java | 4 ---- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java index 9a8a2bb5e8f76b..462352a70ccbb6 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java @@ -2805,7 +2805,6 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx Set tabletIdSet = new HashSet<>(); // create partition boolean editlogCreateTable = false; - boolean editlogColocateAddTable = false; try { if (partitionInfo.getType() == PartitionType.UNPARTITIONED) { if (properties != null && !properties.isEmpty()) { @@ -2960,7 +2959,6 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx ColocatePersistInfo info = ColocatePersistInfo.createForAddTable(groupId, tableId, backendsPerBucketSeq); Env.getCurrentEnv().getEditLog().logColocateAddTable(info); - editlogColocateAddTable = true; } LOG.info("successfully create table[{};{}]", tableName, tableId); Env.getCurrentEnv().getDynamicPartitionScheduler() @@ -2984,10 +2982,7 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx if (Env.getCurrentColocateIndex().isColocateTable(tableId)) { GroupId groupId = Env.getCurrentColocateIndex().getGroup(tableId); Env.getCurrentColocateIndex().removeTable(tableId); - if (editlogColocateAddTable) { - ColocatePersistInfo info = ColocatePersistInfo.createForRemoveTable(groupId, tableId); - Env.getCurrentEnv().getEditLog().logColocateRemoveTable(info); - } + // edit log write DropTableInfo will also remove colocate group } try { dropTable(db, tableId, true, 0L); diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/ColocatePersistInfo.java b/fe/fe-core/src/main/java/org/apache/doris/persist/ColocatePersistInfo.java index c3380cdfba2ab4..429d4e0e1a6b94 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/persist/ColocatePersistInfo.java +++ b/fe/fe-core/src/main/java/org/apache/doris/persist/ColocatePersistInfo.java @@ -62,10 +62,6 @@ public static ColocatePersistInfo createForAddTable(GroupId groupId, long tableI return new ColocatePersistInfo(groupId, tableId, backendsPerBucketSeq, new ReplicaAllocation()); } - public static ColocatePersistInfo createForRemoveTable(GroupId groupId, long tableId) { - return new ColocatePersistInfo(groupId, tableId, Maps.newHashMap(), new ReplicaAllocation()); - } - public static ColocatePersistInfo createForBackendsPerBucketSeq(GroupId groupId, Map>> backendsPerBucketSeq) { return new ColocatePersistInfo(groupId, -1L, backendsPerBucketSeq, new ReplicaAllocation()); From 71471f331f384a0a396acc8c9bf3342d9a8e9c57 Mon Sep 17 00:00:00 2001 From: yujun777 Date: Mon, 8 Jul 2024 18:46:01 +0800 Subject: [PATCH 3/9] update --- .../java/org/apache/doris/datasource/InternalCatalog.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java index 462352a70ccbb6..e5fcbf1cec7fab 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java @@ -2937,9 +2937,6 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx ErrorReport.reportDdlException(ErrorCode.ERR_TABLE_EXISTS_ERROR, tableName); } - // if table not exists, then db.createTableWithLock will write an editlog. - editlogCreateTable = !result.second; - if (result.second) { if (Env.getCurrentColocateIndex().isColocateTable(tableId)) { // if this is a colocate table, its table id is already added to colocate group @@ -2951,6 +2948,9 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx } LOG.info("duplicate create table[{};{}], skip next steps", tableName, tableId); } else { + // if table not exists, then db.createTableWithLock will write an editlog. + editlogCreateTable = true; + // we have added these index to memory, only need to persist here if (Env.getCurrentColocateIndex().isColocateTable(tableId)) { GroupId groupId = Env.getCurrentColocateIndex().getGroup(tableId); From a0d5c3e533082130e1b2342113e6f242f25ac166 Mon Sep 17 00:00:00 2001 From: yujun777 Date: Mon, 8 Jul 2024 18:52:17 +0800 Subject: [PATCH 4/9] update --- .../main/java/org/apache/doris/datasource/InternalCatalog.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java index e5fcbf1cec7fab..444d351bcce24d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java @@ -2980,9 +2980,9 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx Env.getCurrentInvertedIndex().deleteTablet(tabletId); } if (Env.getCurrentColocateIndex().isColocateTable(tableId)) { + // edit log write DropTableInfo will also remove colocate group GroupId groupId = Env.getCurrentColocateIndex().getGroup(tableId); Env.getCurrentColocateIndex().removeTable(tableId); - // edit log write DropTableInfo will also remove colocate group } try { dropTable(db, tableId, true, 0L); From a49e8de10efcaaa573973aaa44d878faf3245db6 Mon Sep 17 00:00:00 2001 From: yujun777 Date: Mon, 8 Jul 2024 19:09:37 +0800 Subject: [PATCH 5/9] update --- .../main/java/org/apache/doris/datasource/InternalCatalog.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java index 444d351bcce24d..31bf95bcc634bb 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java @@ -2979,9 +2979,8 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx for (Long tabletId : tabletIdSet) { Env.getCurrentInvertedIndex().deleteTablet(tabletId); } + // edit log write DropTableInfo will also remove colocate group if (Env.getCurrentColocateIndex().isColocateTable(tableId)) { - // edit log write DropTableInfo will also remove colocate group - GroupId groupId = Env.getCurrentColocateIndex().getGroup(tableId); Env.getCurrentColocateIndex().removeTable(tableId); } try { From 94103d05f494d7e28c17f6cfd5fc1033d49ff56e Mon Sep 17 00:00:00 2001 From: yujun777 Date: Mon, 8 Jul 2024 19:29:33 +0800 Subject: [PATCH 6/9] update --- .../suites/partition_p0/test_create_table_exception.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/regression-test/suites/partition_p0/test_create_table_exception.groovy b/regression-test/suites/partition_p0/test_create_table_exception.groovy index 46c2fa9b2b9796..1b17c15fc12c1a 100644 --- a/regression-test/suites/partition_p0/test_create_table_exception.groovy +++ b/regression-test/suites/partition_p0/test_create_table_exception.groovy @@ -116,11 +116,11 @@ suite("test_create_table_exception") { def checkResult = { -> def tables = sql """show tables;""" - log.info(tables.toString()) + log.info("tables=" + tables) assertEquals(3, tables.size()) def groups = sql """ show proc "/colocation_group" """ - log.info(groups) + log.info("groups=" + groups) assertEquals(1, groups.size()) } From 87b7136d4aedb6d913629a9d901fc0dbd6f98577 Mon Sep 17 00:00:00 2001 From: yujun777 Date: Mon, 8 Jul 2024 19:30:17 +0800 Subject: [PATCH 7/9] update --- .../suites/partition_p0/test_create_table_exception.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/regression-test/suites/partition_p0/test_create_table_exception.groovy b/regression-test/suites/partition_p0/test_create_table_exception.groovy index 1b17c15fc12c1a..5bf35be2c8b1fe 100644 --- a/regression-test/suites/partition_p0/test_create_table_exception.groovy +++ b/regression-test/suites/partition_p0/test_create_table_exception.groovy @@ -129,7 +129,7 @@ suite("test_create_table_exception") { checkResult() cluster.restartFrontends(cluster.getMasterFe().index) - sleep 30_000 + sleep 32_000 reconnectFe() checkResult() } finally { From 472787ec34d37e91da2cf915f865ff3769ae3c4e Mon Sep 17 00:00:00 2001 From: yujun777 Date: Mon, 8 Jul 2024 20:04:49 +0800 Subject: [PATCH 8/9] udpate --- .../org/apache/doris/datasource/InternalCatalog.java | 3 ++- .../partition_p0/test_create_table_exception.groovy | 11 ++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java index 31bf95bcc634bb..398c3c1dbeaf1b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java @@ -2979,7 +2979,8 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx for (Long tabletId : tabletIdSet) { Env.getCurrentInvertedIndex().deleteTablet(tabletId); } - // edit log write DropTableInfo will also remove colocate group + // edit log write DropTableInfo will result in deleting colocate group, + // but follow fe may need wait 30s (recycle bin mgr run every 30s). if (Env.getCurrentColocateIndex().isColocateTable(tableId)) { Env.getCurrentColocateIndex().removeTable(tableId); } diff --git a/regression-test/suites/partition_p0/test_create_table_exception.groovy b/regression-test/suites/partition_p0/test_create_table_exception.groovy index 5bf35be2c8b1fe..96f097c76705f2 100644 --- a/regression-test/suites/partition_p0/test_create_table_exception.groovy +++ b/regression-test/suites/partition_p0/test_create_table_exception.groovy @@ -128,12 +128,17 @@ suite("test_create_table_exception") { createTable(2) checkResult() + sleep 1000 cluster.restartFrontends(cluster.getMasterFe().index) sleep 32_000 - reconnectFe() - checkResult() + def newMasterFe = cluster.getMasterFe() + def newMasterFeUrl = "jdbc:mysql://${newMasterFe.host}:${newMasterFe.queryPort}/?useLocalSessionState=false&allowLoadLocalInfile=false" + newMasterFeUrl = context.config.buildUrlWithDb(newMasterFeUrl, context.dbName) + connect('root', '', newMasterFeUrl) { + checkResult() + } + } finally { - GetDebugPoint().disableDebugPointForAllFEs('FE.createOlapTable.exception') } } } From 9261095ba197b88ae2c5d3570a77d006e9eb8d96 Mon Sep 17 00:00:00 2001 From: yujun777 Date: Mon, 8 Jul 2024 21:43:45 +0800 Subject: [PATCH 9/9] update --- .../apache/doris/datasource/InternalCatalog.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java index 398c3c1dbeaf1b..e13316c3e7655e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java @@ -1001,13 +1001,13 @@ public boolean unprotectDropTable(Database db, Table table, boolean isForceDrop, return true; } - private void dropTable(Database db, long tableId, boolean isForceDrop, + private void dropTable(Database db, long tableId, boolean isForceDrop, boolean isReplay, Long recycleTime) throws MetaNotFoundException { Table table = db.getTableOrMetaException(tableId); db.writeLock(); table.writeLock(); try { - unprotectDropTable(db, table, isForceDrop, true, recycleTime); + unprotectDropTable(db, table, isForceDrop, isReplay, recycleTime); Env.getCurrentEnv().getQueryStats().clear(Env.getCurrentInternalCatalog().getId(), db.getId(), tableId); Env.getCurrentEnv().getAnalysisManager().removeTableStats(table.getId()); } finally { @@ -1018,7 +1018,7 @@ private void dropTable(Database db, long tableId, boolean isForceDrop, public void replayDropTable(Database db, long tableId, boolean isForceDrop, Long recycleTime) throws MetaNotFoundException { - dropTable(db, tableId, isForceDrop, recycleTime); + dropTable(db, tableId, isForceDrop, true, recycleTime); } public void replayEraseTable(long tableId) { @@ -2804,7 +2804,7 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx // if failed in any step, use this set to do clear things Set tabletIdSet = new HashSet<>(); // create partition - boolean editlogCreateTable = false; + boolean hadLogEditCreateTable = false; try { if (partitionInfo.getType() == PartitionType.UNPARTITIONED) { if (properties != null && !properties.isEmpty()) { @@ -2949,7 +2949,7 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx LOG.info("duplicate create table[{};{}], skip next steps", tableName, tableId); } else { // if table not exists, then db.createTableWithLock will write an editlog. - editlogCreateTable = true; + hadLogEditCreateTable = true; // we have added these index to memory, only need to persist here if (Env.getCurrentColocateIndex().isColocateTable(tableId)) { @@ -2985,8 +2985,8 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx Env.getCurrentColocateIndex().removeTable(tableId); } try { - dropTable(db, tableId, true, 0L); - if (editlogCreateTable) { + dropTable(db, tableId, true, false, 0L); + if (hadLogEditCreateTable) { DropInfo info = new DropInfo(db.getId(), tableId, olapTable.getName(), -1L, true, 0L); Env.getCurrentEnv().getEditLog().logDropTable(info); }