This is an automated email from the ASF dual-hosted git repository.
dataroaring pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.0 by this push:
new 17e5177f711 [fix](truncate) fix tablet invert index leaky #37334
(#37411)
17e5177f711 is described below
commit 17e5177f711e78df6086f4f8b994a2aca8d08e4f
Author: yujun <[email protected]>
AuthorDate: Mon Jul 8 23:33:01 2024 +0800
[fix](truncate) fix tablet invert index leaky #37334 (#37411)
cherry pick from #37334
---
.../apache/doris/datasource/InternalCatalog.java | 31 ++++++++----
.../apache/doris/alter/AddExistsPartitionTest.java | 13 ++++-
.../apache/doris/catalog/TruncateTableTest.java | 56 ++++++++++++++++++++++
3 files changed, 90 insertions(+), 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 1b2090378d7..8c027412fcf 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
@@ -2982,6 +2982,11 @@ public class InternalCatalog implements
CatalogIf<Database> {
List<Partition> newPartitions = Lists.newArrayList();
// tabletIdSet to save all newly created tablet ids.
Set<Long> tabletIdSet = Sets.newHashSet();
+ Runnable failedCleanCallback = () -> {
+ for (Long tabletId : tabletIdSet) {
+ Env.getCurrentInvertedIndex().deleteTablet(tabletId);
+ }
+ };
try {
long bufferSize =
IdGeneratorUtil.getBufferSizeForTruncateTable(copiedTbl,
origPartitions.values());
IdGeneratorBuffer idGeneratorBuffer =
@@ -3017,9 +3022,7 @@ public class InternalCatalog implements
CatalogIf<Database> {
}
} 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());
@@ -3027,15 +3030,18 @@ public class InternalCatalog implements
CatalogIf<Database> {
// 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();
+ boolean hasWriteLock = false;
try {
+ olapTable = (OlapTable)
db.getTableOrDdlException(copiedTbl.getId());
+ olapTable.writeLockOrDdlException();
+ hasWriteLock = true;
olapTable.checkNormalStateForAlter();
// check partitions
for (Map.Entry<String, Long> 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");
}
}
@@ -3079,6 +3085,10 @@ public class InternalCatalog implements
CatalogIf<Database> {
}
}
}
+ 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.");
@@ -3093,8 +3103,13 @@ public class InternalCatalog implements
CatalogIf<Database> {
newPartitions,
truncateEntireTable,
truncateTableStmt.toSqlWithoutTable());
Env.getCurrentEnv().getEditLog().logTruncateTable(info);
+ } catch (DdlException e) {
+ failedCleanCallback.run();
+ throw e;
} finally {
- olapTable.writeUnlock();
+ if (hasWriteLock) {
+ olapTable.writeUnlock();
+ }
}
if (truncateEntireTable) {
// Drop the whole table stats after truncate the entire table
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 bd3d03c6c46..0197e93c24f 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;
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 {
@@ -46,14 +50,19 @@ public class AddExistsPartitionTest extends
TestWithFeService {
+ " DISTRIBUTED BY HASH(`k`) BUCKETS 5"
+ " PROPERTIES ( \"replication_num\" = \"" + backendNum() +
"\" )");
List<Long> backendIds = Env.getCurrentSystemInfo().getAllBackendIds();
+ Map<Long, Set<Long>> oldBackendTablets = Maps.newHashMap();
for (long backendId : backendIds) {
- Assertions.assertEquals(5,
Env.getCurrentInvertedIndex().getTabletIdsByBackendId(backendId).size());
+ Set<Long> 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 p1 VALUES LESS THAN ('200')";
Assertions.assertNotNull(getSqlStmtExecutor(addPartitionSql));
for (long backendId : backendIds) {
- Assertions.assertEquals(5,
Env.getCurrentInvertedIndex().getTabletIdsByBackendId(backendId).size());
+ Set<Long> 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 e5708eded29..5160a0feba0 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.CreateTableStmt;
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 org.junit.Test;
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
@@ -164,6 +175,51 @@ public class TruncateTableTest {
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("default_cluster: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<Long> backendIds =
Env.getCurrentSystemInfo().getAllBackendIds();
+ Map<Long, Set<Long>> oldBackendTablets = Maps.newHashMap();
+ for (long backendId : backendIds) {
+ Set<Long> 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<Long> 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);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
