This is an automated email from the ASF dual-hosted git repository.
dataroaring pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 93f9a2c5bcd [fix](tablet invert index) fix tablet invert index leaky
caused by auto partition (#33973)
93f9a2c5bcd is described below
commit 93f9a2c5bcd0cc3aa2803857c422978de1678dc4
Author: yujun <[email protected]>
AuthorDate: Thu Apr 25 11:13:01 2024 +0800
[fix](tablet invert index) fix tablet invert index leaky caused by auto
partition (#33973)
---
.../apache/doris/datasource/InternalCatalog.java | 24 ++++++----
.../apache/doris/alter/AddExistsPartitionTest.java | 56 ++++++++++++++++++++++
.../apache/doris/utframe/TestWithFeService.java | 3 +-
3 files changed, 72 insertions(+), 11 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 10983a955b7..dd52fad4f7f 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
@@ -1466,8 +1466,10 @@ public class InternalCatalog implements
CatalogIf<Database> {
// check partition name
if (olapTable.checkPartitionNameExist(partitionName)) {
if (singlePartitionDesc.isSetIfNotExists()) {
- LOG.info("add partition[{}] which already exists",
partitionName);
- return;
+ LOG.info("table[{}] add partition[{}] which already
exists", olapTable.getName(), partitionName);
+ if
(!DebugPointUtil.isEnable("InternalCatalog.addPartition.noCheckExists")) {
+ return;
+ }
} else {
ErrorReport.reportDdlException(ErrorCode.ERR_SAME_NAME_PARTITION,
partitionName);
}
@@ -1624,6 +1626,11 @@ public class InternalCatalog implements
CatalogIf<Database> {
if (!Strings.isNullOrEmpty(dataProperty.getStoragePolicy())) {
storagePolicy = dataProperty.getStoragePolicy();
}
+ Runnable failedCleanCallback = () -> {
+ for (Long tabletId : tabletIdSet) {
+ Env.getCurrentInvertedIndex().deleteTablet(tabletId);
+ }
+ };
try {
long partitionId = idGeneratorBuffer.getNextId();
List<Long> partitionIds = Lists.newArrayList(partitionId);
@@ -1646,8 +1653,9 @@ public class InternalCatalog implements
CatalogIf<Database> {
olapTable.checkNormalStateForAlter();
// check partition name
if (olapTable.checkPartitionNameExist(partitionName)) {
+ LOG.info("table[{}] add partition[{}] which already
exists", olapTable.getName(), partitionName);
if (singlePartitionDesc.isSetIfNotExists()) {
- LOG.info("add partition[{}] which already exists",
partitionName);
+ failedCleanCallback.run();
return;
} else {
ErrorReport.reportDdlException(ErrorCode.ERR_SAME_NAME_PARTITION,
partitionName);
@@ -1696,8 +1704,6 @@ public class InternalCatalog implements
CatalogIf<Database> {
}
}
-
-
if (metaChanged) {
throw new DdlException("Table[" + tableName + "]'s meta
has been changed. try again.");
}
@@ -1741,9 +1747,7 @@ public class InternalCatalog implements
CatalogIf<Database> {
olapTable.writeUnlock();
}
} catch (DdlException e) {
- for (Long tabletId : tabletIdSet) {
- Env.getCurrentInvertedIndex().deleteTablet(tabletId);
- }
+ failedCleanCallback.run();
throw e;
}
}
@@ -2844,10 +2848,10 @@ public class InternalCatalog implements
CatalogIf<Database> {
Env.getCurrentEnv().getEditLog().logColocateAddTable(info);
}
LOG.info("successfully create table[{};{}]", tableName,
tableId);
- // register or remove table from DynamicPartition after table
created
-
DynamicPartitionUtil.registerOrRemoveDynamicPartitionTable(db.getId(),
olapTable, false);
Env.getCurrentEnv().getDynamicPartitionScheduler()
.executeDynamicPartitionFirstTime(db.getId(),
olapTable.getId());
+ // register or remove table from DynamicPartition after table
created
+
DynamicPartitionUtil.registerOrRemoveDynamicPartitionTable(db.getId(),
olapTable, false);
Env.getCurrentEnv().getDynamicPartitionScheduler()
.createOrUpdateRuntimeInfo(tableId,
DynamicPartitionScheduler.LAST_UPDATE_TIME,
TimeUtils.getCurrentFormatTime());
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
new file mode 100644
index 00000000000..0d95ee30cde
--- /dev/null
+++
b/fe/fe-core/src/test/java/org/apache/doris/alter/AddExistsPartitionTest.java
@@ -0,0 +1,56 @@
+// 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.alter;
+
+import org.apache.doris.catalog.Env;
+import org.apache.doris.common.Config;
+import org.apache.doris.common.util.DebugPointUtil;
+import org.apache.doris.common.util.DebugPointUtil.DebugPoint;
+import org.apache.doris.utframe.TestWithFeService;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+public class AddExistsPartitionTest extends TestWithFeService {
+
+ @Override
+ protected void beforeCreatingConnectContext() throws Exception {
+ Config.enable_debug_points = true;
+ }
+
+ @Test
+ public void testAddExistsPartition() throws Exception {
+
DebugPointUtil.addDebugPoint("InternalCatalog.addPartition.noCheckExists", new
DebugPoint());
+ createDatabase("test");
+ createTable("CREATE TABLE test.tbl (k INT) DISTRIBUTED BY HASH(k) "
+ + " BUCKETS 5 PROPERTIES ( \"replication_num\" = \"" +
backendNum() + "\" )");
+ List<Long> backendIds = Env.getCurrentSystemInfo().getAllBackendIds();
+ for (long backendId : backendIds) {
+ Assertions.assertEquals(5,
Env.getCurrentInvertedIndex().getTabletIdsByBackendId(backendId).size());
+ }
+
+ 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());
+ }
+ }
+}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/utframe/TestWithFeService.java
b/fe/fe-core/src/test/java/org/apache/doris/utframe/TestWithFeService.java
index b590234a3e8..063ab21d8bc 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/utframe/TestWithFeService.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/utframe/TestWithFeService.java
@@ -583,7 +583,8 @@ public abstract class TestWithFeService {
connectContext.getState().reset();
StmtExecutor stmtExecutor = new StmtExecutor(connectContext, queryStr);
stmtExecutor.execute();
- if (connectContext.getState().getStateType() !=
QueryState.MysqlStateType.ERR) {
+ if (connectContext.getState().getStateType() !=
QueryState.MysqlStateType.ERR
+ && connectContext.getState().getErrorCode() == null) {
return stmtExecutor;
} else {
return null;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
