From 06d3bb153f9673bed3ce080c183ab05c832329af Mon Sep 17 00:00:00 2001 From: yujun Date: Thu, 9 May 2024 20:25:58 +0800 Subject: [PATCH] [branch-2.0](tablet invert index) fix tablet invert index leaky caused by auto partition #33973 (#34514) --- .../doris/datasource/InternalCatalog.java | 25 ++++---- .../doris/alter/AddExistsPartitionTest.java | 59 +++++++++++++++++++ .../doris/utframe/TestWithFeService.java | 3 +- 3 files changed, 76 insertions(+), 11 deletions(-) create mode 100644 fe/fe-core/src/test/java/org/apache/doris/alter/AddExistsPartitionTest.java 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 bdf091194ef4b9..6c525b45645631 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 @@ -123,6 +123,7 @@ import org.apache.doris.common.UserException; import org.apache.doris.common.io.CountingDataOutputStream; import org.apache.doris.common.util.DbUtil; +import org.apache.doris.common.util.DebugPointUtil; import org.apache.doris.common.util.DynamicPartitionUtil; import org.apache.doris.common.util.IdGeneratorUtil; import org.apache.doris.common.util.MetaLockUtils; @@ -1405,8 +1406,10 @@ public void addPartition(Database db, String tableName, AddPartitionClause addPa // 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); } @@ -1567,6 +1570,11 @@ public void addPartition(Database db, String tableName, AddPartitionClause addPa if (!Strings.isNullOrEmpty(dataProperty.getStoragePolicy())) { storagePolicy = dataProperty.getStoragePolicy(); } + Runnable failedCleanCallback = () -> { + for (Long tabletId : tabletIdSet) { + Env.getCurrentInvertedIndex().deleteTablet(tabletId); + } + }; try { long partitionId = idGeneratorBuffer.getNextId(); Partition partition = createPartitionWithIndices(db.getClusterName(), db.getId(), olapTable.getId(), @@ -1593,8 +1601,9 @@ public void addPartition(Database db, String tableName, AddPartitionClause addPa 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); @@ -1643,8 +1652,6 @@ public void addPartition(Database db, String tableName, AddPartitionClause addPa } } - - if (metaChanged) { throw new DdlException("Table[" + tableName + "]'s meta has been changed. try again."); } @@ -1684,9 +1691,7 @@ public void addPartition(Database db, String tableName, AddPartitionClause addPa olapTable.writeUnlock(); } } catch (DdlException e) { - for (Long tabletId : tabletIdSet) { - Env.getCurrentInvertedIndex().deleteTablet(tabletId); - } + failedCleanCallback.run(); throw e; } } @@ -2622,10 +2627,10 @@ private void createOlapTable(Database db, CreateTableStmt stmt) throws UserExcep 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 00000000000000..bd3d03c6c46de2 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/alter/AddExistsPartitionTest.java @@ -0,0 +1,59 @@ +// 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, s INT SUM DEFAULT '0')" + + " AGGREGATE KEY(`k`) " + + " PARTITION BY RANGE(`k`)" + + " ( PARTITION p1 VALUES LESS THAN ('100') )" + + " DISTRIBUTED BY HASH(`k`) BUCKETS 5" + + " PROPERTIES ( \"replication_num\" = \"" + backendNum() + "\" )"); + List 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 p1 VALUES LESS THAN ('200')"; + 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 c27bdbac4c3fe9..41eb4e6c4e1462 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 @@ -553,7 +553,8 @@ public StmtExecutor getSqlStmtExecutor(String queryStr) throws Exception { 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;