Skip to content

Commit

Permalink
[branch-2.0](tablet invert index) fix tablet invert index leaky cause…
Browse files Browse the repository at this point in the history
…d by auto partition #33973 (#34514)
  • Loading branch information
yujun777 authored May 9, 2024
1 parent 68cae7e commit 06d3bb1
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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(),
Expand All @@ -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);
Expand Down Expand Up @@ -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.");
}
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
@@ -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<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 p1 VALUES LESS THAN ('200')";
Assertions.assertNotNull(getSqlStmtExecutor(addPartitionSql));
for (long backendId : backendIds) {
Assertions.assertEquals(5, Env.getCurrentInvertedIndex().getTabletIdsByBackendId(backendId).size());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 06d3bb1

Please sign in to comment.