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 68d7db6256cce9..dc5ea892649b18 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 @@ -3331,6 +3331,11 @@ public void truncateTable(TruncateTableStmt truncateTableStmt) throws DdlExcepti List newPartitions = Lists.newArrayList(); // tabletIdSet to save all newly created tablet ids. Set tabletIdSet = Sets.newHashSet(); + Runnable failedCleanCallback = () -> { + for (Long tabletId : tabletIdSet) { + Env.getCurrentInvertedIndex().deleteTablet(tabletId); + } + }; Map clusterKeyMap = new TreeMap<>(); for (int i = 0; i < olapTable.getBaseSchema().size(); i++) { Column column = olapTable.getBaseSchema().get(i); @@ -3383,9 +3388,7 @@ public void truncateTable(TruncateTableStmt truncateTableStmt) throws DdlExcepti } catch (DdlException e) { // create partition failed, remove all newly created tablets - for (Long tabletId : tabletIdSet) { - Env.getCurrentInvertedIndex().deleteTablet(tabletId); - } + failedCleanCallback.run(); throw e; } Preconditions.checkState(origPartitions.size() == newPartitions.size()); @@ -3393,16 +3396,19 @@ public void truncateTable(TruncateTableStmt truncateTableStmt) throws DdlExcepti // all partitions are created successfully, try to replace the old partitions. // before replacing, we need to check again. // Things may be changed outside the table lock. - olapTable = (OlapTable) db.getTableOrDdlException(copiedTbl.getId()); - olapTable.writeLockOrDdlException(); List oldPartitions = Lists.newArrayList(); + boolean hasWriteLock = false; try { + olapTable = (OlapTable) db.getTableOrDdlException(copiedTbl.getId()); + olapTable.writeLockOrDdlException(); + hasWriteLock = true; olapTable.checkNormalStateForAlter(); // check partitions for (Map.Entry entry : origPartitions.entrySet()) { - Partition partition = copiedTbl.getPartition(entry.getValue()); + Partition partition = olapTable.getPartition(entry.getValue()); if (partition == null || !partition.getName().equalsIgnoreCase(entry.getKey())) { - throw new DdlException("Partition [" + entry.getKey() + "] is changed"); + throw new DdlException("Partition [" + entry.getKey() + "] is changed" + + " during truncating table, please retry"); } } @@ -3446,6 +3452,10 @@ public void truncateTable(TruncateTableStmt truncateTableStmt) throws DdlExcepti } } } + if (DebugPointUtil.isEnable("InternalCatalog.truncateTable.metaChanged")) { + metaChanged = true; + LOG.warn("debug set truncate table meta changed"); + } if (metaChanged) { throw new DdlException("Table[" + copiedTbl.getName() + "]'s meta has been changed. try again."); @@ -3460,8 +3470,13 @@ public void truncateTable(TruncateTableStmt truncateTableStmt) throws DdlExcepti newPartitions, truncateEntireTable, truncateTableStmt.toSqlWithoutTable()); Env.getCurrentEnv().getEditLog().logTruncateTable(info); + } catch (DdlException e) { + failedCleanCallback.run(); + throw e; } finally { - olapTable.writeUnlock(); + if (hasWriteLock) { + olapTable.writeUnlock(); + } } erasePartitionDropBackendReplicas(oldPartitions); diff --git a/fe/fe-core/src/test/java/org/apache/doris/alter/AddExistsPartitionTest.java b/fe/fe-core/src/test/java/org/apache/doris/alter/AddExistsPartitionTest.java index 0d95ee30cdeb20..841dd099d22d1c 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/alter/AddExistsPartitionTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/alter/AddExistsPartitionTest.java @@ -23,10 +23,14 @@ import org.apache.doris.common.util.DebugPointUtil.DebugPoint; import org.apache.doris.utframe.TestWithFeService; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import java.util.List; +import java.util.Map; +import java.util.Set; public class AddExistsPartitionTest extends TestWithFeService { @@ -42,15 +46,20 @@ public void testAddExistsPartition() throws Exception { createTable("CREATE TABLE test.tbl (k INT) DISTRIBUTED BY HASH(k) " + " BUCKETS 5 PROPERTIES ( \"replication_num\" = \"" + backendNum() + "\" )"); List backendIds = Env.getCurrentSystemInfo().getAllBackendIds(); + Map> oldBackendTablets = Maps.newHashMap(); for (long backendId : backendIds) { - Assertions.assertEquals(5, Env.getCurrentInvertedIndex().getTabletIdsByBackendId(backendId).size()); + Set tablets = Sets.newHashSet(Env.getCurrentInvertedIndex().getTabletIdsByBackendId(backendId)); + Assertions.assertEquals(5, tablets.size()); + oldBackendTablets.put(backendId, tablets); } String addPartitionSql = "ALTER TABLE test.tbl ADD PARTITION IF NOT EXISTS tbl" + " DISTRIBUTED BY HASH(k) BUCKETS 5"; Assertions.assertNotNull(getSqlStmtExecutor(addPartitionSql)); for (long backendId : backendIds) { - Assertions.assertEquals(5, Env.getCurrentInvertedIndex().getTabletIdsByBackendId(backendId).size()); + Set tablets = Sets.newHashSet(Env.getCurrentInvertedIndex().getTabletIdsByBackendId(backendId)); + Assertions.assertEquals(5, tablets.size()); + Assertions.assertEquals(oldBackendTablets.get(backendId), tablets); } } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/catalog/TruncateTableTest.java b/fe/fe-core/src/test/java/org/apache/doris/catalog/TruncateTableTest.java index 3d7a4bff1bd177..8c742787bcb2a1 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/catalog/TruncateTableTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/TruncateTableTest.java @@ -23,11 +23,18 @@ import org.apache.doris.analysis.ShowStmt; import org.apache.doris.analysis.ShowTabletStmt; import org.apache.doris.analysis.TruncateTableStmt; +import org.apache.doris.common.Config; +import org.apache.doris.common.DdlException; +import org.apache.doris.common.ExceptionChecker; +import org.apache.doris.common.util.DebugPointUtil; +import org.apache.doris.common.util.DebugPointUtil.DebugPoint; import org.apache.doris.qe.ConnectContext; import org.apache.doris.qe.ShowExecutor; import org.apache.doris.qe.ShowResultSet; import org.apache.doris.utframe.UtFrameUtils; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; @@ -35,6 +42,8 @@ import java.io.File; import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.UUID; public class TruncateTableTest { @@ -44,6 +53,8 @@ public class TruncateTableTest { @BeforeClass public static void setup() throws Exception { + Config.disable_balance = true; + Config.enable_debug_points = true; UtFrameUtils.createDorisCluster(runningDir); connectContext = UtFrameUtils.createDefaultCtx(); // create database @@ -165,6 +176,51 @@ public void testTruncateTable() throws Exception { checkShowTabletResultNum("test.tbl", "p20210904", 5); } + @Test + public void testTruncateTableFailed() throws Exception { + String createTableStr = "create table test.tbl2(d1 date, k1 int, k2 bigint)" + + "duplicate key(d1, k1) " + + "PARTITION BY RANGE(d1)" + + "(PARTITION p20210901 VALUES [('2021-09-01'), ('2021-09-02')))" + + "distributed by hash(k1) buckets 2 " + + "properties('replication_num' = '1');"; + createTable(createTableStr); + String partitionName = "p20210901"; + Database db = Env.getCurrentInternalCatalog().getDbNullable("test"); + OlapTable tbl2 = db.getOlapTableOrDdlException("tbl2"); + Assert.assertNotNull(tbl2); + Partition p20210901 = tbl2.getPartition(partitionName); + Assert.assertNotNull(p20210901); + long partitionId = p20210901.getId(); + p20210901.setVisibleVersionAndTime(2L, System.currentTimeMillis()); + + try { + List backendIds = Env.getCurrentSystemInfo().getAllBackendIds(); + Map> oldBackendTablets = Maps.newHashMap(); + for (long backendId : backendIds) { + Set tablets = Sets.newHashSet(Env.getCurrentInvertedIndex().getTabletIdsByBackendId(backendId)); + oldBackendTablets.put(backendId, tablets); + } + + DebugPointUtil.addDebugPoint("InternalCatalog.truncateTable.metaChanged", new DebugPoint()); + + String truncateStr = "truncate table test.tbl2 partition (" + partitionName + ");"; + TruncateTableStmt truncateTableStmt = (TruncateTableStmt) UtFrameUtils.parseAndAnalyzeStmt( + truncateStr, connectContext); + ExceptionChecker.expectThrowsWithMsg(DdlException.class, + "Table[tbl2]'s meta has been changed. try again", + () -> Env.getCurrentEnv().truncateTable(truncateTableStmt)); + + Assert.assertEquals(partitionId, tbl2.getPartition(partitionName).getId()); + for (long backendId : backendIds) { + Set tablets = Sets.newHashSet(Env.getCurrentInvertedIndex().getTabletIdsByBackendId(backendId)); + Assert.assertEquals(oldBackendTablets.get(backendId), tablets); + } + } finally { + DebugPointUtil.removeDebugPoint("InternalCatalog.truncateTable.metaChanged"); + } + } + private static void createDb(String sql) throws Exception { CreateDbStmt createDbStmt = (CreateDbStmt) UtFrameUtils.parseAndAnalyzeStmt(sql, connectContext); Env.getCurrentEnv().createDb(createDbStmt);