Skip to content

Commit

Permalink
[fix](truncate) fix tablet invert index leaky (#37334)
Browse files Browse the repository at this point in the history 
FIX:
1. fix tablet invert index leaky due to truncate fail;
2. fix NPE due to partitions has changed during truncating;
  • Loading branch information
@yujun777 @dataroaring
yujun777 authored and dataroaring committed Jul 17, 2024
1 parent f1f9ea8 commit 78d8380
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 10 deletions.
31 changes: 23 additions & 8 deletions fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3356,6 +3356,11 @@ public void truncateTable(TruncateTableStmt truncateTableStmt) throws DdlExcepti
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);
}
};
Map<Integer, Integer> clusterKeyMap = new TreeMap<>();
for (int i = 0; i < olapTable.getBaseSchema().size(); i++) {
Column column = olapTable.getBaseSchema().get(i);
Expand Down Expand Up @@ -3408,26 +3413,27 @@ 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());

// 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<Partition> oldPartitions = Lists.newArrayList();
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");
}
}

Expand Down Expand Up @@ -3471,6 +3477,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.");
Expand All @@ -3485,8 +3495,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);
Expand Down
13 changes: 11 additions & 2 deletions fe/fe-core/src/test/java/org/apache/doris/alter/AddExistsPartitionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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<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 tbl"
+ " DISTRIBUTED BY HASH(k) BUCKETS 5";
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);
}
}
}
56 changes: 56 additions & 0 deletions fe/fe-core/src/test/java/org/apache/doris/catalog/TruncateTableTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,27 @@
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;
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 {
Expand All @@ -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
Expand Down Expand Up @@ -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<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);
Expand Down

0 comments on commit 78d8380

Please sign in to comment.