From eaa5a2b704e55e22192306e8dbb1badf8a243af3 Mon Sep 17 00:00:00 2001 From: Yongzao <532741407@qq.com> Date: Wed, 22 Jul 2026 19:17:15 +0800 Subject: [PATCH 1/5] [ConfigNode] Fence region creation during database deletion --- .../confignode/i18n/ConfigNodeMessages.java | 12 + .../confignode/i18n/ManagerMessages.java | 5 + .../confignode/i18n/ConfigNodeMessages.java | 12 + .../confignode/i18n/ManagerMessages.java | 5 + .../consensus/request/ConfigPhysicalPlan.java | 4 + .../request/ConfigPhysicalPlanType.java | 1 + .../BatchRemoveRegionCreateTasksPlan.java | 82 +++++ .../confignode/manager/ProcedureManager.java | 93 +++-- .../manager/partition/PartitionManager.java | 337 +++++++++++++++--- .../manager/schema/ClusterSchemaManager.java | 128 ++++--- .../executor/ConfigPlanExecutor.java | 4 + .../persistence/partition/PartitionInfo.java | 123 +++++-- .../procedure/env/ConfigNodeProcedureEnv.java | 38 +- .../impl/AbstractDatabaseProcedure.java | 67 ++++ .../region/CreateRegionGroupsProcedure.java | 85 +++-- .../region/RemoveRegionGroupProcedure.java | 18 +- .../impl/schema/DeleteDatabaseProcedure.java | 67 ++-- .../DatabaseLifecycleLockManager.java | 279 +++++++++++++++ .../request/ConfigPhysicalPlanSerDeTest.java | 23 ++ .../manager/ClusterSchemaManagerTest.java | 60 ++++ .../manager/ProcedureManagerTest.java | 33 ++ .../persistence/PartitionInfoTest.java | 194 +++++++++- .../impl/CreateRegionGroupsProcedureTest.java | 255 +++++++++++++ .../schema/DeleteDatabaseProcedureTest.java | 90 +++++ .../DatabaseLifecycleLockManagerTest.java | 141 ++++++++ 25 files changed, 1930 insertions(+), 226 deletions(-) create mode 100644 iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/write/region/BatchRemoveRegionCreateTasksPlan.java create mode 100644 iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/AbstractDatabaseProcedure.java create mode 100644 iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManager.java create mode 100644 iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManagerTest.java diff --git a/iotdb-core/confignode/src/main/i18n/en/org/apache/iotdb/confignode/i18n/ConfigNodeMessages.java b/iotdb-core/confignode/src/main/i18n/en/org/apache/iotdb/confignode/i18n/ConfigNodeMessages.java index 494201b1e9fec..0b7ff7eb0edb5 100644 --- a/iotdb-core/confignode/src/main/i18n/en/org/apache/iotdb/confignode/i18n/ConfigNodeMessages.java +++ b/iotdb-core/confignode/src/main/i18n/en/org/apache/iotdb/confignode/i18n/ConfigNodeMessages.java @@ -665,4 +665,16 @@ private ConfigNodeMessages() {} public static final String EXCEPTION_FAILED_TO_CREATE_OR_ALTER_TOPIC_MODE_CONSENSUS_DOES_NOT_SUPPORT_TOPIC_ATTRIBUTES_ARG_3C2D0BDA = "Failed to create or alter topic, mode=consensus does not support topic attributes %s"; + public static final String + LOG_REJECT_CREATEREGIONGROUPSPLAN_BECAUSE_DATABASE_ARG_DOES_NOT_EXIST_616E0CDE = + "Reject CreateRegionGroupsPlan because database {} does not exist"; + public static final String + LOG_REJECT_CREATEREGIONGROUPSPLAN_BECAUSE_DATABASE_ARG_IS_BEING_DELETED_C085AC01 = + "Reject CreateRegionGroupsPlan because database {} is being deleted"; + public static final String + MESSAGE_CREATE_REGIONGROUPS_FAILED_BECAUSE_DATABASE_ARG_DOES_NOT_EXIST_AF0F2440 = + "Create RegionGroups failed because database %s does not exist"; + public static final String + MESSAGE_CREATE_REGIONGROUPS_FAILED_BECAUSE_DATABASE_ARG_IS_BEING_DELETED_651DB780 = + "Create RegionGroups failed because database %s is being deleted"; } diff --git a/iotdb-core/confignode/src/main/i18n/en/org/apache/iotdb/confignode/i18n/ManagerMessages.java b/iotdb-core/confignode/src/main/i18n/en/org/apache/iotdb/confignode/i18n/ManagerMessages.java index 20316edc39713..45361a98d8053 100644 --- a/iotdb-core/confignode/src/main/i18n/en/org/apache/iotdb/confignode/i18n/ManagerMessages.java +++ b/iotdb-core/confignode/src/main/i18n/en/org/apache/iotdb/confignode/i18n/ManagerMessages.java @@ -587,6 +587,11 @@ private ManagerMessages() {} public static final String MESSAGE_SCHEMA_ENGINE_MODE_E37ED98C = "schema_engine_mode"; public static final String MESSAGE_TAG_ATTRIBUTE_TOTAL_SIZE_AF658CFE = "tag_attribute_total_size"; public static final String MESSAGE_DATABASE_LIMIT_THRESHOLD_45C23274 = "database_limit_threshold"; + public static final String + MESSAGE_DATABASE_ARG_STILL_HAS_UNFINISHED_LIFECYCLE_PROCEDURES_67573924 = + "Database %s still has unfinished lifecycle procedures"; + public static final String MESSAGE_SOME_OTHER_TASK_IS_DELETING_DATABASE_ARG_7BDB2C0F = + "Some other task is deleting database %s"; public static final String LOG_UNEXPECTED_ERROR_HAPPENED_SETTING_SPACE_QUOTA_DATABASE_ARG_F6ED7586 = "Unexpected error happened while setting space quota on database: %s "; public static final String LOG_UNEXPECTED_ERROR_HAPPENED_SETTING_THROTTLE_QUOTA_USER_ARG_C111BE81 = "Unexpected error happened while setting throttle quota on user: %s "; public static final String LOG_SCHEMA_TEMPLATE_NEED_TWO_FILES_1E57542A = "schema_template need two files"; diff --git a/iotdb-core/confignode/src/main/i18n/zh/org/apache/iotdb/confignode/i18n/ConfigNodeMessages.java b/iotdb-core/confignode/src/main/i18n/zh/org/apache/iotdb/confignode/i18n/ConfigNodeMessages.java index 0193e5a04854e..d5228705755b5 100644 --- a/iotdb-core/confignode/src/main/i18n/zh/org/apache/iotdb/confignode/i18n/ConfigNodeMessages.java +++ b/iotdb-core/confignode/src/main/i18n/zh/org/apache/iotdb/confignode/i18n/ConfigNodeMessages.java @@ -710,4 +710,16 @@ private ConfigNodeMessages() {} public static final String EXCEPTION_FAILED_TO_CREATE_OR_ALTER_TOPIC_MODE_CONSENSUS_DOES_NOT_SUPPORT_TOPIC_ATTRIBUTES_ARG_3C2D0BDA = "创建或修改 topic 失败，mode=consensus 不支持 topic 属性 %s"; + public static final String + LOG_REJECT_CREATEREGIONGROUPSPLAN_BECAUSE_DATABASE_ARG_DOES_NOT_EXIST_616E0CDE = + "拒绝 CreateRegionGroupsPlan，因为数据库 {} 不存在"; + public static final String + LOG_REJECT_CREATEREGIONGROUPSPLAN_BECAUSE_DATABASE_ARG_IS_BEING_DELETED_C085AC01 = + "拒绝 CreateRegionGroupsPlan，因为数据库 {} 正在删除"; + public static final String + MESSAGE_CREATE_REGIONGROUPS_FAILED_BECAUSE_DATABASE_ARG_DOES_NOT_EXIST_AF0F2440 = + "创建 RegionGroups 失败，因为数据库 %s 不存在"; + public static final String + MESSAGE_CREATE_REGIONGROUPS_FAILED_BECAUSE_DATABASE_ARG_IS_BEING_DELETED_651DB780 = + "创建 RegionGroups 失败，因为数据库 %s 正在删除"; } diff --git a/iotdb-core/confignode/src/main/i18n/zh/org/apache/iotdb/confignode/i18n/ManagerMessages.java b/iotdb-core/confignode/src/main/i18n/zh/org/apache/iotdb/confignode/i18n/ManagerMessages.java index 8dc9afd1e8bf0..5ed644c0f6dab 100644 --- a/iotdb-core/confignode/src/main/i18n/zh/org/apache/iotdb/confignode/i18n/ManagerMessages.java +++ b/iotdb-core/confignode/src/main/i18n/zh/org/apache/iotdb/confignode/i18n/ManagerMessages.java @@ -579,6 +579,11 @@ private ManagerMessages() {} public static final String MESSAGE_SCHEMA_ENGINE_MODE_E37ED98C = "schema_engine_mode"; public static final String MESSAGE_TAG_ATTRIBUTE_TOTAL_SIZE_AF658CFE = "tag_attribute_total_size"; public static final String MESSAGE_DATABASE_LIMIT_THRESHOLD_45C23274 = "database_limit_threshold"; + public static final String + MESSAGE_DATABASE_ARG_STILL_HAS_UNFINISHED_LIFECYCLE_PROCEDURES_67573924 = + "数据库 %s 仍有未完成的生命周期流程"; + public static final String MESSAGE_SOME_OTHER_TASK_IS_DELETING_DATABASE_ARG_7BDB2C0F = + "其他任务正在删除数据库 %s"; public static final String LOG_UNEXPECTED_ERROR_HAPPENED_SETTING_SPACE_QUOTA_DATABASE_ARG_F6ED7586 = "设置数据库 %s 的空间配额时发生意外错误 "; public static final String LOG_UNEXPECTED_ERROR_HAPPENED_SETTING_THROTTLE_QUOTA_USER_ARG_C111BE81 = "设置用户 %s 的限流配额时发生意外错误 "; public static final String LOG_SCHEMA_TEMPLATE_NEED_TWO_FILES_1E57542A = "schema_template 需要两个文件"; diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlan.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlan.java index 0d9ca912571a4..8b6546f662584 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlan.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlan.java @@ -85,6 +85,7 @@ import org.apache.iotdb.confignode.consensus.request.write.procedure.UpdateProcedurePlan; import org.apache.iotdb.confignode.consensus.request.write.quota.SetSpaceQuotaPlan; import org.apache.iotdb.confignode.consensus.request.write.quota.SetThrottleQuotaPlan; +import org.apache.iotdb.confignode.consensus.request.write.region.BatchRemoveRegionCreateTasksPlan; import org.apache.iotdb.confignode.consensus.request.write.region.CreateRegionGroupsPlan; import org.apache.iotdb.confignode.consensus.request.write.region.OfferRegionMaintainTasksPlan; import org.apache.iotdb.confignode.consensus.request.write.region.PollRegionMaintainTaskPlan; @@ -266,6 +267,9 @@ public static ConfigPhysicalPlan create(final ByteBuffer buffer) throws IOExcept case PollSpecificRegionMaintainTask: plan = new PollSpecificRegionMaintainTaskPlan(); break; + case BatchRemoveRegionCreateTasks: + plan = new BatchRemoveRegionCreateTasksPlan(); + break; case CreateSchemaPartition: plan = new CreateSchemaPartitionPlan(); break; diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanType.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanType.java index 1be9518141483..e6f9c9847d148 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanType.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanType.java @@ -72,6 +72,7 @@ public enum ConfigPhysicalPlanType { AddRegionLocation((short) 311), RemoveRegionLocation((short) 312), GetRegionGroupsByTime((short) 313), + BatchRemoveRegionCreateTasks((short) 314), /** Partition. */ GetSchemaPartition((short) 400), diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/write/region/BatchRemoveRegionCreateTasksPlan.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/write/region/BatchRemoveRegionCreateTasksPlan.java new file mode 100644 index 0000000000000..256b290eb6250 --- /dev/null +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/write/region/BatchRemoveRegionCreateTasksPlan.java @@ -0,0 +1,82 @@ +/* + * 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.iotdb.confignode.consensus.request.write.region; + +import org.apache.iotdb.confignode.consensus.request.ConfigPhysicalPlan; +import org.apache.iotdb.confignode.consensus.request.ConfigPhysicalPlanType; + +import org.apache.tsfile.utils.ReadWriteIOUtils; + +import java.io.DataOutputStream; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.Objects; + +/** + * Removes every queued RegionCreateTask that belongs to the specified pre-deleted database. + * + *

The state machine ignores this plan when the database is missing or active, so replaying a + * cancellation cannot affect a later database incarnation that reuses the same name. + */ +public class BatchRemoveRegionCreateTasksPlan extends ConfigPhysicalPlan { + + private String database; + + public BatchRemoveRegionCreateTasksPlan() { + super(ConfigPhysicalPlanType.BatchRemoveRegionCreateTasks); + } + + public BatchRemoveRegionCreateTasksPlan(final String database) { + super(ConfigPhysicalPlanType.BatchRemoveRegionCreateTasks); + this.database = database; + } + + public String getDatabase() { + return database; + } + + @Override + protected void serializeImpl(DataOutputStream stream) throws IOException { + stream.writeShort(getType().getPlanType()); + ReadWriteIOUtils.write(database, stream); + } + + @Override + protected void deserializeImpl(ByteBuffer buffer) throws IOException { + database = ReadWriteIOUtils.readString(buffer); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof BatchRemoveRegionCreateTasksPlan)) { + return false; + } + final BatchRemoveRegionCreateTasksPlan that = (BatchRemoveRegionCreateTasksPlan) o; + return Objects.equals(database, that.database); + } + + @Override + public int hashCode() { + return Objects.hash(database); + } +} diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java index 8f5b27b7e2fa4..c76615d1fc6d9 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java @@ -134,6 +134,7 @@ import org.apache.iotdb.confignode.procedure.impl.testonly.CreateManyDatabasesProcedure; import org.apache.iotdb.confignode.procedure.impl.trigger.CreateTriggerProcedure; import org.apache.iotdb.confignode.procedure.impl.trigger.DropTriggerProcedure; +import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLifecycleLockManager.DatabaseLock; import org.apache.iotdb.confignode.procedure.scheduler.ProcedureScheduler; import org.apache.iotdb.confignode.procedure.scheduler.SimpleProcedureScheduler; import org.apache.iotdb.confignode.procedure.store.ConfigProcedureStore; @@ -183,6 +184,7 @@ import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashSet; import java.util.List; @@ -305,34 +307,40 @@ public TSStatus deleteDatabases( for (final TDatabaseSchema databaseSchema : deleteSgSchemaList) { final String database = databaseSchema.getName(); boolean hasOverlappedTask = false; - synchronized (this) { - while (executor.isRunning() - && System.currentTimeMillis() - startCheckTimeForProcedures < PROCEDURE_WAIT_TIME_OUT) { - final Pair procedureIdDuplicatePair = - checkDuplicateTableTask( - database, null, null, null, null, ProcedureType.DELETE_DATABASE_PROCEDURE); - hasOverlappedTask = procedureIdDuplicatePair.getRight(); - - if (Boolean.FALSE.equals(procedureIdDuplicatePair.getRight())) { - DeleteDatabaseProcedure procedure = - new DeleteDatabaseProcedure(databaseSchema, isGeneratedByPipe); - this.executor.submitProcedure(procedure); - procedures.add(procedure); - break; + while (executor.isRunning() + && System.currentTimeMillis() - startCheckTimeForProcedures < PROCEDURE_WAIT_TIME_OUT) { + try (final DatabaseLock ignored = acquireDatabaseLifecycleLock(database)) { + synchronized (this) { + final Pair procedureIdDuplicatePair = + checkDuplicateTableTask( + database, null, null, null, null, ProcedureType.DELETE_DATABASE_PROCEDURE); + hasOverlappedTask = procedureIdDuplicatePair.getRight(); + + if (Boolean.FALSE.equals(procedureIdDuplicatePair.getRight())) { + final DeleteDatabaseProcedure procedure = + new DeleteDatabaseProcedure(databaseSchema, isGeneratedByPipe); + this.executor.submitProcedure(procedure); + procedures.add(procedure); + } } + } + if (!hasOverlappedTask) { + break; + } + synchronized (this) { try { wait(PROCEDURE_WAIT_RETRY_TIMEOUT); } catch (final InterruptedException e) { Thread.currentThread().interrupt(); } } - if (hasOverlappedTask) { - return RpcUtils.getStatus( - TSStatusCode.OVERLAP_WITH_EXISTING_TASK, - String.format( - "Some other task is operating table under the database %s, please retry after the procedure finishes.", - database)); - } + } + if (hasOverlappedTask) { + return RpcUtils.getStatus( + TSStatusCode.OVERLAP_WITH_EXISTING_TASK, + String.format( + "Some other task is operating table under the database %s, please retry after the procedure finishes.", + database)); } } List results = new ArrayList<>(procedures.size()); @@ -1501,18 +1509,28 @@ private static TDataNodeLocation buildFakeDataNodeLocation(int dataNodeId, Strin // endregion + public CreateRegionGroupsProcedure submitCreateRegionGroups( + final TConsensusGroupType consensusGroupType, + final CreateRegionGroupsPlan createRegionGroupsPlan) { + final CreateRegionGroupsProcedure procedure = + new CreateRegionGroupsProcedure(consensusGroupType, createRegionGroupsPlan); + // Reentrant for PartitionManager, which holds the same database locks while allocating the + // plan. Keeping this guard here also makes a future direct submission visible atomically to + // same-name database creation and deletion. + try (final DatabaseLock ignored = + acquireDatabaseLifecycleLocks(createRegionGroupsPlan.getRegionGroupMap().keySet())) { + executor.submitProcedure(procedure); + } + return procedure; + } + /** - * Generate {@link CreateRegionGroupsProcedure} and wait until it finished. + * Wait until a {@link CreateRegionGroupsProcedure} finishes. * * @return {@link TSStatusCode#SUCCESS_STATUS} if all RegionGroups have been created successfully, * {@link TSStatusCode#CREATE_REGION_ERROR} otherwise */ - public TSStatus createRegionGroups( - final TConsensusGroupType consensusGroupType, - final CreateRegionGroupsPlan createRegionGroupsPlan) { - final CreateRegionGroupsProcedure procedure = - new CreateRegionGroupsProcedure(consensusGroupType, createRegionGroupsPlan); - executor.submitProcedure(procedure); + public TSStatus waitCreateRegionGroups(final CreateRegionGroupsProcedure procedure) { final TSStatus status = waitingProcedureFinished(procedure); if (status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) { return status; @@ -1522,6 +1540,25 @@ public TSStatus createRegionGroups( } } + public DatabaseLock acquireDatabaseLifecycleLock(final String database) { + return acquireDatabaseLifecycleLocks(Collections.singleton(database)); + } + + public DatabaseLock acquireDatabaseLifecycleLocks(final Set databases) { + return env.getDatabaseLifecycleLockManager().acquireLocks(databases); + } + + public boolean hasUnfinishedDatabaseLifecycleProcedure(final String database) { + return executor.getProcedures().values().stream() + .filter(procedure -> !procedure.isFinished()) + .anyMatch( + procedure -> + (procedure instanceof DeleteDatabaseProcedure + && database.equals(((DeleteDatabaseProcedure) procedure).getDatabase())) + || (procedure instanceof CreateRegionGroupsProcedure + && ((CreateRegionGroupsProcedure) procedure).containsDatabase(database))); + } + /** * Generate {@link CreateTriggerProcedure} and wait until it finished. * diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/partition/PartitionManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/partition/PartitionManager.java index 7efdae278db3c..95325f268c983 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/partition/PartitionManager.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/partition/PartitionManager.java @@ -27,6 +27,7 @@ import org.apache.iotdb.common.rpc.thrift.TSeriesPartitionSlot; import org.apache.iotdb.common.rpc.thrift.TTimePartitionSlot; import org.apache.iotdb.commons.cluster.RegionRoleType; +import org.apache.iotdb.commons.cluster.RegionStatus; import org.apache.iotdb.commons.concurrent.IoTDBThreadPoolFactory; import org.apache.iotdb.commons.concurrent.ThreadName; import org.apache.iotdb.commons.concurrent.threadpool.ScheduledExecutorUtil; @@ -59,6 +60,7 @@ import org.apache.iotdb.confignode.consensus.request.write.partition.CreateDataPartitionPlan; import org.apache.iotdb.confignode.consensus.request.write.partition.CreateSchemaPartitionPlan; import org.apache.iotdb.confignode.consensus.request.write.partition.RemoveRegionLocationPlan; +import org.apache.iotdb.confignode.consensus.request.write.region.BatchRemoveRegionCreateTasksPlan; import org.apache.iotdb.confignode.consensus.request.write.region.CreateRegionGroupsPlan; import org.apache.iotdb.confignode.consensus.request.write.region.PollSpecificRegionMaintainTaskPlan; import org.apache.iotdb.confignode.consensus.response.partition.CountTimeSlotListResp; @@ -86,6 +88,8 @@ import org.apache.iotdb.confignode.persistence.partition.maintainer.RegionCreateTask; import org.apache.iotdb.confignode.persistence.partition.maintainer.RegionMaintainTask; import org.apache.iotdb.confignode.procedure.impl.partition.DataPartitionTableIntegrityCheckProcedure; +import org.apache.iotdb.confignode.procedure.impl.region.CreateRegionGroupsProcedure; +import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLifecycleLockManager.DatabaseLock; import org.apache.iotdb.confignode.rpc.thrift.TCountTimeSlotListReq; import org.apache.iotdb.confignode.rpc.thrift.TGetRegionGroupsByTimeReq; import org.apache.iotdb.confignode.rpc.thrift.TGetRegionIdReq; @@ -123,6 +127,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; @@ -154,8 +159,16 @@ public class PartitionManager { // Try to delete Regions in every 10s private static final int REGION_MAINTAINER_WORK_INTERVAL = 10; + private static final int SCHEMA_REGION_CREATE_BATCH_SIZE_PER_DATA_NODE = 32; + private static final int DATA_REGION_CREATE_BATCH_SIZE_PER_DATA_NODE = 64; + private static final long REGION_CREATE_BACKOFF_BASE_NANOS = + TimeUnit.SECONDS.toNanos(REGION_MAINTAINER_WORK_INTERVAL); + private static final long REGION_CREATE_BACKOFF_MAX_NANOS = TimeUnit.MINUTES.toNanos(5); + private final ScheduledExecutorService regionMaintainer; private Future currentRegionMaintainerFuture; + private final Map> regionCreateBackoffMap = + new EnumMap<>(TConsensusGroupType.class); private final AtomicBoolean dataPartitionTableIntegrityCheckProcedureRunning = new AtomicBoolean(false); @@ -741,11 +754,22 @@ private TSStatus generateAndAllocateRegionGroups( final Map allotmentMap, final TConsensusGroupType consensusGroupType) throws NotEnoughDataNodeException, DatabaseNotExistsException { if (!allotmentMap.isEmpty()) { - final CreateRegionGroupsPlan createRegionGroupsPlan = - getLoadManager().allocateRegionGroups(allotmentMap, consensusGroupType); - LOGGER.info(ManagerMessages.CREATEREGIONGROUPS_STARTING_TO_CREATE_THE_FOLLOWING_REGIONGROUPS); - createRegionGroupsPlan.planLog(LOGGER); - return getProcedureManager().createRegionGroups(consensusGroupType, createRegionGroupsPlan); + final CreateRegionGroupsProcedure procedure; + // Cover both ID allocation and submission with the same per-database locks used by database + // creation and deletion. A delayed plan therefore cannot become invisible between deleting + // and recreating a database with the same name. + try (final DatabaseLock ignored = + getProcedureManager().acquireDatabaseLifecycleLocks(allotmentMap.keySet())) { + final CreateRegionGroupsPlan createRegionGroupsPlan = + getLoadManager().allocateRegionGroups(allotmentMap, consensusGroupType); + LOGGER.info( + ManagerMessages.CREATEREGIONGROUPS_STARTING_TO_CREATE_THE_FOLLOWING_REGIONGROUPS); + createRegionGroupsPlan.planLog(LOGGER); + procedure = + getProcedureManager() + .submitCreateRegionGroups(consensusGroupType, createRegionGroupsPlan); + } + return getProcedureManager().waitCreateRegionGroups(procedure); } else { return RpcUtils.SUCCESS_STATUS; } @@ -1117,14 +1141,29 @@ public SchemaNodeManagementResp getNodePathsPartition(GetNodePathsPartitionPlan } } - public void preDeleteDatabase( + public TSStatus preDeleteDatabase( final String database, final PreDeleteDatabasePlan.PreDeleteType preDeleteType) { final PreDeleteDatabasePlan preDeleteDatabasePlan = new PreDeleteDatabasePlan(database, preDeleteType); try { - getConsensusManager().write(preDeleteDatabasePlan); + return getConsensusManager().write(preDeleteDatabasePlan); + } catch (final ConsensusException e) { + LOGGER.warn(CONSENSUS_WRITE_ERROR, e); + final TSStatus status = new TSStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR.getStatusCode()); + status.setMessage(e.getMessage()); + return status; + } + } + + /** Durably removes all queued RegionCreateTasks of the specified database. */ + public TSStatus batchRemoveRegionCreateTasks(final String database) { + try { + return getConsensusManager().write(new BatchRemoveRegionCreateTasksPlan(database)); } catch (final ConsensusException e) { LOGGER.warn(CONSENSUS_WRITE_ERROR, e); + final TSStatus status = new TSStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR.getStatusCode()); + status.setMessage(e.getMessage()); + return status; } } @@ -1132,6 +1171,10 @@ public boolean isDatabasePreDeleted(final String database) { return partitionInfo.isDatabasePreDeleted(database); } + public TSStatus validateCreateRegionGroups(final CreateRegionGroupsPlan plan) { + return partitionInfo.validateCreateRegionGroups(plan); + } + /** * Get TSeriesPartitionSlot. * @@ -1365,11 +1408,60 @@ public void maintainRegionReplicas() { return; } + final List databases = + partitionInfo.getRegionMaintainEntryList().stream() + .filter(RegionCreateTask.class::isInstance) + .map(RegionCreateTask.class::cast) + .map(RegionCreateTask::getStorageGroup) + .distinct() + .sorted() + .collect(Collectors.toList()); + for (final String database : databases) { + try (final DatabaseLock ignored = + getProcedureManager().acquireDatabaseLifecycleLock(database)) { + // Leadership may have changed while this invocation was waiting for an in-flight database + // procedure. The next leader will repair and retry the persisted queue. + if (!getConsensusManager().isLeader()) { + return; + } + maintainRegionReplicasUnderLock(database); + } + } + } + + private void maintainRegionReplicasUnderLock(final String database) { + final List persistedTasks = + partitionInfo.getRegionMaintainEntryList().stream() + .filter(RegionCreateTask.class::isInstance) + .map(RegionCreateTask.class::cast) + .filter(task -> database.equals(task.getStorageGroup())) + .collect(Collectors.toList()); + final Map invalidTaskCountByRegion = new HashMap<>(); + for (RegionMaintainTask task : persistedTasks) { + if (!(task instanceof RegionCreateTask) + || !isRegionCreateTaskRegionValid((RegionCreateTask) task)) { + invalidTaskCountByRegion.merge(task.getRegionId(), 1, Integer::sum); + } + } + if (!removeInvalidRegionCreateTasks(invalidTaskCountByRegion)) { + return; + } + final Set invalidRegionIds = invalidTaskCountByRegion.keySet(); + + // Do not infer that an Unknown cache entry means a missing replica until the new leader has + // collected enough heartbeats. Orphan/pre-deleted tasks above are still cleaned immediately. + if (!getLoadManager().isLoadReady()) { + return; + } + // Group the queued tasks into one FIFO sub-queue per region. The queue only ever holds // RegionCreateTasks now (delete tasks are filtered out at the PartitionInfo ingestion points), // and a region may carry several of them when more than one of its replicas failed to create. - final Map> tasksByRegion = new HashMap<>(); - for (RegionMaintainTask task : partitionInfo.getRegionMaintainEntryList()) { + final Map> tasksByRegion = new LinkedHashMap<>(); + for (RegionMaintainTask task : persistedTasks) { + if (invalidRegionIds.contains(task.getRegionId())) { + continue; + } if (!(task instanceof RegionCreateTask)) { // Unreachable: the queue only holds create tasks now (legacy delete tasks are dropped at // the @@ -1383,51 +1475,105 @@ public void maintainRegionReplicas() { .add((RegionCreateTask) task); } - // Drain the sub-queues head-by-head. Each round takes the head of every region, batches those - // heads by region type into a single create RPC per type, then durably polls the tasks that - // succeeded. Tasks of the same region are advanced one at a time to preserve their offer order. - while (!tasksByRegion.isEmpty()) { - final Map> headsByType = - new EnumMap<>(TConsensusGroupType.class); - for (Queue queue : tasksByRegion.values()) { - final RegionCreateTask head = queue.peek(); - headsByType.computeIfAbsent(head.getRegionId().getType(), k -> new ArrayList<>()).add(head); + final Set invalidHeadRegionIds = new HashSet<>(); + final Map> headsByType = + new EnumMap<>(TConsensusGroupType.class); + final Map> selectedCountByTypeAndDataNode = + new EnumMap<>(TConsensusGroupType.class); + for (Queue queue : tasksByRegion.values()) { + final RegionCreateTask head = queue.peek(); + if (!isRegionCreateTaskTargetValid(head)) { + invalidHeadRegionIds.add(head.getRegionId()); + continue; } - - final Set successfulRegions = new HashSet<>(); - int selectedCount = 0; - for (Map.Entry> entry : headsByType.entrySet()) { - selectedCount += entry.getValue().size(); - successfulRegions.addAll(submitRegionCreateTasks(entry.getKey(), entry.getValue())); + if (isRegionCreateTargetInBackoff(head)) { + continue; } - - if (successfulRegions.isEmpty()) { - break; + final TConsensusGroupType type = head.getRegionId().getType(); + final int dataNodeId = head.getTargetDataNode().getDataNodeId(); + final Map selectedCountByDataNode = + selectedCountByTypeAndDataNode.computeIfAbsent(type, ignored -> new HashMap<>()); + final int selectedCount = selectedCountByDataNode.getOrDefault(dataNodeId, 0); + if (selectedCount >= getRegionCreateBatchSize(type)) { + continue; } + selectedCountByDataNode.put(dataNodeId, selectedCount + 1); + headsByType.computeIfAbsent(type, ignored -> new ArrayList<>()).add(head); + } - // Advance the in-memory sub-queues so the next round picks the following task of each region. - for (TConsensusGroupId regionId : successfulRegions) { - tasksByRegion.computeIfPresent( - regionId, - (k, queue) -> { - queue.poll(); - return queue.isEmpty() ? null : queue; - }); - } + // A target-specific stale task only removes the head of its Region queue. A following task of + // the same Region may still point to another replica that is genuinely missing. + if (!invalidHeadRegionIds.isEmpty() + && !writeRegionCreateTaskPlan( + new PollSpecificRegionMaintainTaskPlan(invalidHeadRegionIds))) { + return; + } - // Durably remove the head of every successfully created region from the persisted queue. - try { - getConsensusManager().write(new PollSpecificRegionMaintainTaskPlan(successfulRegions)); - } catch (ConsensusException e) { - LOGGER.warn(CONSENSUS_WRITE_ERROR, e); - } + final Set successfulRegions = new HashSet<>(); + for (Map.Entry> entry : headsByType.entrySet()) { + successfulRegions.addAll(submitRegionCreateTasks(entry.getKey(), entry.getValue())); + } + if (!successfulRegions.isEmpty()) { + writeRegionCreateTaskPlan(new PollSpecificRegionMaintainTaskPlan(successfulRegions)); + } + } - if (successfulRegions.size() < selectedCount) { - // Some tasks failed this round; stop and retry on the next schedule so that the tasks of - // each region keep being executed in the order they were offered. - break; + private boolean writeRegionCreateTaskPlan(ConfigPhysicalPlan plan) { + try { + return getConsensusManager().write(plan).getCode() + == TSStatusCode.SUCCESS_STATUS.getStatusCode(); + } catch (ConsensusException e) { + LOGGER.warn(CONSENSUS_WRITE_ERROR, e); + return false; + } + } + + private boolean removeInvalidRegionCreateTasks( + final Map invalidTaskCountByRegion) { + if (invalidTaskCountByRegion.isEmpty()) { + return true; + } + final int maxTaskCount = + invalidTaskCountByRegion.values().stream().mapToInt(Integer::intValue).max().orElse(0); + for (int taskIndex = 0; taskIndex < maxTaskCount; taskIndex++) { + final int currentTaskIndex = taskIndex; + final Set regionIds = + invalidTaskCountByRegion.entrySet().stream() + .filter(entry -> entry.getValue() > currentTaskIndex) + .map(Map.Entry::getKey) + .collect(Collectors.toSet()); + if (!writeRegionCreateTaskPlan(new PollSpecificRegionMaintainTaskPlan(regionIds))) { + return false; } } + return true; + } + + private boolean isRegionCreateTaskRegionValid(RegionCreateTask task) { + return partitionInfo.isDatabaseExisted(task.getStorageGroup()) + && Objects.equals( + task.getStorageGroup(), partitionInfo.getRegionDatabase(task.getRegionId())); + } + + private boolean isRegionCreateTaskTargetValid(RegionCreateTask task) { + final List currentReplicaSets = + partitionInfo.getReplicaSets( + task.getStorageGroup(), Collections.singletonList(task.getRegionId())); + if (currentReplicaSets.size() != 1 + || currentReplicaSets.get(0).getDataNodeLocations().stream() + .noneMatch( + location -> location.getDataNodeId() == task.getTargetDataNode().getDataNodeId())) { + return false; + } + return RegionStatus.Unknown.equals( + getLoadManager() + .getRegionStatus(task.getRegionId(), task.getTargetDataNode().getDataNodeId())); + } + + private int getRegionCreateBatchSize(TConsensusGroupType regionType) { + return TConsensusGroupType.SchemaRegion.equals(regionType) + ? SCHEMA_REGION_CREATE_BATCH_SIZE_PER_DATA_NODE + : DATA_REGION_CREATE_BATCH_SIZE_PER_DATA_NODE; } /** @@ -1451,10 +1597,8 @@ private Set submitRegionCreateTasks( new TCreateSchemaRegionReq(task.getRegionReplicaSet(), task.getStorageGroup())); schemaHandler.putNodeLocation(task.getRegionId().getId(), task.getTargetDataNode()); } - CnToDnInternalServiceAsyncRequestManager.getInstance() - .sendAsyncRequestWithRetry(schemaHandler); - collectSuccessfulRegions( - schemaHandler.getResponseMap(), TConsensusGroupType.SchemaRegion, successfulRegions); + CnToDnInternalServiceAsyncRequestManager.getInstance().sendAsyncRequest(schemaHandler); + collectSuccessfulRegions(schemaHandler.getResponseMap(), createTasks, successfulRegions); break; case DataRegion: final DataNodeAsyncRequestContext dataHandler = @@ -1469,10 +1613,8 @@ private Set submitRegionCreateTasks( new TCreateDataRegionReq(task.getRegionReplicaSet(), task.getStorageGroup())); dataHandler.putNodeLocation(task.getRegionId().getId(), task.getTargetDataNode()); } - CnToDnInternalServiceAsyncRequestManager.getInstance() - .sendAsyncRequestWithRetry(dataHandler); - collectSuccessfulRegions( - dataHandler.getResponseMap(), TConsensusGroupType.DataRegion, successfulRegions); + CnToDnInternalServiceAsyncRequestManager.getInstance().sendAsyncRequest(dataHandler); + collectSuccessfulRegions(dataHandler.getResponseMap(), createTasks, successfulRegions); break; default: break; @@ -1482,13 +1624,92 @@ private Set submitRegionCreateTasks( private void collectSuccessfulRegions( Map responseMap, - TConsensusGroupType regionType, + List createTasks, Set successfulRegions) { - for (Map.Entry entry : responseMap.entrySet()) { - if (entry.getValue().getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) { - successfulRegions.add(new TConsensusGroupId(regionType, entry.getKey())); + final Map> tasksByDataNode = new HashMap<>(); + for (RegionCreateTask task : createTasks) { + tasksByDataNode + .computeIfAbsent(task.getTargetDataNode().getDataNodeId(), ignored -> new ArrayList<>()) + .add(task); + final TSStatus status = responseMap.get(task.getRegionId().getId()); + if (status != null && status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) { + successfulRegions.add(task.getRegionId()); + } + } + for (List dataNodeTasks : tasksByDataNode.values()) { + RegionCreateTask failedTask = null; + TSStatus failedStatus = null; + for (RegionCreateTask task : dataNodeTasks) { + final TSStatus status = responseMap.get(task.getRegionId().getId()); + if (status == null || status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) { + if (failedTask == null || isDirectMemoryFailure(status)) { + failedTask = task; + failedStatus = status; + } + } } + if (failedTask != null) { + recordRegionCreateFailure(failedTask, failedStatus); + } else { + clearRegionCreateBackoff(dataNodeTasks.get(0)); + } + } + } + + private boolean isRegionCreateTargetInBackoff(RegionCreateTask task) { + return Optional.ofNullable(regionCreateBackoffMap.get(task.getRegionId().getType())) + .map(backoffByDataNode -> backoffByDataNode.get(task.getTargetDataNode().getDataNodeId())) + .map(backoff -> backoff.nextAttemptNanos > System.nanoTime()) + .orElse(false); + } + + private void clearRegionCreateBackoff(RegionCreateTask task) { + Optional.ofNullable(regionCreateBackoffMap.get(task.getRegionId().getType())) + .ifPresent( + backoffByDataNode -> + backoffByDataNode.remove(task.getTargetDataNode().getDataNodeId())); + } + + private void recordRegionCreateFailure(RegionCreateTask task, TSStatus status) { + final RegionCreateBackoff backoff = + regionCreateBackoffMap + .computeIfAbsent(task.getRegionId().getType(), ignored -> new HashMap<>()) + .computeIfAbsent( + task.getTargetDataNode().getDataNodeId(), ignored -> new RegionCreateBackoff()); + backoff.failureCount++; + long delayNanos = + Math.min( + REGION_CREATE_BACKOFF_MAX_NANOS, + REGION_CREATE_BACKOFF_BASE_NANOS << Math.min(backoff.failureCount - 1, 5)); + if (isDirectMemoryFailure(status)) { + delayNanos = REGION_CREATE_BACKOFF_MAX_NANOS; + } else { + final long jitterBound = Math.max(1, delayNanos / 4); + delayNanos = + Math.min( + REGION_CREATE_BACKOFF_MAX_NANOS, + delayNanos + ThreadLocalRandom.current().nextLong(jitterBound)); + } + backoff.nextAttemptNanos = System.nanoTime() + delayNanos; + } + + private boolean isDirectMemoryFailure(TSStatus status) { + if (status == null || status.getMessage() == null) { + return false; } + final String normalizedMessage = status.getMessage().toLowerCase(java.util.Locale.ROOT); + return normalizedMessage.contains("direct memory") + || normalizedMessage.contains("direct buffer") + || normalizedMessage.contains("directbuffer") + || normalizedMessage.contains("outofmemory") + || normalizedMessage.contains("out of memory") + || normalizedMessage.contains("oom") + || normalizedMessage.contains("内存"); + } + + private static class RegionCreateBackoff { + private int failureCount; + private long nextAttemptNanos; } public void startRegionCleaner() { diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/schema/ClusterSchemaManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/schema/ClusterSchemaManager.java index a0284445ea20a..d850c6d4f3482 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/schema/ClusterSchemaManager.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/schema/ClusterSchemaManager.java @@ -92,6 +92,7 @@ import org.apache.iotdb.confignode.i18n.ManagerMessages; import org.apache.iotdb.confignode.i18n.ProcedureMessages; import org.apache.iotdb.confignode.manager.IManager; +import org.apache.iotdb.confignode.manager.ProcedureManager; import org.apache.iotdb.confignode.manager.consensus.ConsensusManager; import org.apache.iotdb.confignode.manager.node.NodeManager; import org.apache.iotdb.confignode.manager.partition.PartitionManager; @@ -99,6 +100,7 @@ import org.apache.iotdb.confignode.manager.partition.RegionGroupExtensionPolicy; import org.apache.iotdb.confignode.persistence.schema.ClusterSchemaInfo; import org.apache.iotdb.confignode.persistence.schema.ConfigSchemaStatistics; +import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLifecycleLockManager.DatabaseLock; import org.apache.iotdb.confignode.rpc.thrift.TDatabaseInfo; import org.apache.iotdb.confignode.rpc.thrift.TDatabaseSchema; import org.apache.iotdb.confignode.rpc.thrift.TDescTable4InformationSchemaResp; @@ -172,66 +174,90 @@ public ClusterSchemaManager( /** Set Database */ public TSStatus setDatabase( final DatabaseSchemaPlan databaseSchemaPlan, final boolean isGeneratedByPipe) { - TSStatus result; - final TDatabaseSchema schema = databaseSchemaPlan.getSchema(); - if (getPartitionManager().isDatabasePreDeleted(schema.getName())) { - return RpcUtils.getStatus( - TSStatusCode.METADATA_ERROR, - String.format("Some other task is deleting database %s", schema.getName())); - } - - createDatabaseLock.lock(); - try { - clusterSchemaInfo.isDatabaseNameValid( - schema.getName(), schema.isSetIsTableModel() && schema.isIsTableModel()); - if (!schema.getName().equals(SchemaConstant.SYSTEM_DATABASE) - && !schema.getName().equals(SchemaConstant.AUDIT_DATABASE) - && !schema.getName().equals(Audit.TABLE_MODEL_AUDIT_DATABASE)) { - clusterSchemaInfo.checkDatabaseLimit(); + final ProcedureManager procedureManager = configManager.getProcedureManager(); + try (final DatabaseLock ignored = + procedureManager.acquireDatabaseLifecycleLock(schema.getName())) { + if (procedureManager.hasUnfinishedDatabaseLifecycleProcedure(schema.getName())) { + return RpcUtils.getStatus( + TSStatusCode.METADATA_ERROR, + String.format( + ManagerMessages + .MESSAGE_DATABASE_ARG_STILL_HAS_UNFINISHED_LIFECYCLE_PROCEDURES_67573924, + schema.getName())); } - // Cache DatabaseSchema - result = - getConsensusManager() - .write( - isGeneratedByPipe - ? new PipeEnrichedPlan(databaseSchemaPlan) - : databaseSchemaPlan); - // set ttl - if (schema.isSetTTL()) { - result = configManager.getTTLManager().setTTL(databaseSchemaPlan, isGeneratedByPipe); + if (getPartitionManager().isDatabasePreDeleted(schema.getName())) { + return RpcUtils.getStatus( + TSStatusCode.METADATA_ERROR, + String.format( + ManagerMessages.MESSAGE_SOME_OTHER_TASK_IS_DELETING_DATABASE_ARG_7BDB2C0F, + schema.getName())); } - // Bind Database metrics - PartitionMetrics.bindDatabaseRelatedMetricsWhenUpdate( - MetricService.getInstance(), - configManager, - schema.getName(), - schema.getDataReplicationFactor(), - schema.getSchemaReplicationFactor()); - PartitionMetrics.bindDatabaseTableMetrics( - MetricService.getInstance(), - clusterSchemaInfo.getConfigSchemaStatistics(), - schema.getName()); - // Adjust the maximum RegionGroup number of each Database - adjustMaxRegionGroupNum(); - } catch (final ConsensusException e) { - LOGGER.warn(CONSENSUS_WRITE_ERROR, e); - result = new TSStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR.getStatusCode()); - result.setMessage(e.getMessage()); - } catch (final MetadataException metadataException) { - // Reject if Database already set - result = new TSStatus(metadataException.getErrorCode()); - result.setMessage(metadataException.getMessage()); - } finally { - createDatabaseLock.unlock(); - } - return result; + TSStatus result; + createDatabaseLock.lock(); + try { + clusterSchemaInfo.isDatabaseNameValid( + schema.getName(), schema.isSetIsTableModel() && schema.isIsTableModel()); + if (!schema.getName().equals(SchemaConstant.SYSTEM_DATABASE) + && !schema.getName().equals(SchemaConstant.AUDIT_DATABASE) + && !schema.getName().equals(Audit.TABLE_MODEL_AUDIT_DATABASE)) { + clusterSchemaInfo.checkDatabaseLimit(); + } + // Cache DatabaseSchema + result = + getConsensusManager() + .write( + isGeneratedByPipe + ? new PipeEnrichedPlan(databaseSchemaPlan) + : databaseSchemaPlan); + // set ttl + if (schema.isSetTTL()) { + result = configManager.getTTLManager().setTTL(databaseSchemaPlan, isGeneratedByPipe); + } + // Bind Database metrics + PartitionMetrics.bindDatabaseRelatedMetricsWhenUpdate( + MetricService.getInstance(), + configManager, + schema.getName(), + schema.getDataReplicationFactor(), + schema.getSchemaReplicationFactor()); + PartitionMetrics.bindDatabaseTableMetrics( + MetricService.getInstance(), + clusterSchemaInfo.getConfigSchemaStatistics(), + schema.getName()); + // Adjust the maximum RegionGroup number of each Database + adjustMaxRegionGroupNum(); + } catch (final ConsensusException e) { + LOGGER.warn(CONSENSUS_WRITE_ERROR, e); + result = new TSStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR.getStatusCode()); + result.setMessage(e.getMessage()); + } catch (final MetadataException metadataException) { + // Reject if Database already set + result = new TSStatus(metadataException.getErrorCode()); + result.setMessage(metadataException.getMessage()); + } finally { + createDatabaseLock.unlock(); + } + + return result; + } } /** Alter Database */ public TSStatus alterDatabase( final DatabaseSchemaPlan databaseSchemaPlan, final boolean isGeneratedByPipe) { + final TDatabaseSchema databaseSchema = databaseSchemaPlan.getSchema(); + try (final DatabaseLock ignored = + configManager + .getProcedureManager() + .acquireDatabaseLifecycleLock(databaseSchema.getName())) { + return alterDatabaseUnderLock(databaseSchemaPlan, isGeneratedByPipe); + } + } + + private TSStatus alterDatabaseUnderLock( + final DatabaseSchemaPlan databaseSchemaPlan, final boolean isGeneratedByPipe) { TSStatus result; final TDatabaseSchema databaseSchema = databaseSchemaPlan.getSchema(); diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/executor/ConfigPlanExecutor.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/executor/ConfigPlanExecutor.java index 772f46baa3162..7fe8923855ac5 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/executor/ConfigPlanExecutor.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/executor/ConfigPlanExecutor.java @@ -111,6 +111,7 @@ import org.apache.iotdb.confignode.consensus.request.write.procedure.UpdateProcedurePlan; import org.apache.iotdb.confignode.consensus.request.write.quota.SetSpaceQuotaPlan; import org.apache.iotdb.confignode.consensus.request.write.quota.SetThrottleQuotaPlan; +import org.apache.iotdb.confignode.consensus.request.write.region.BatchRemoveRegionCreateTasksPlan; import org.apache.iotdb.confignode.consensus.request.write.region.CreateRegionGroupsPlan; import org.apache.iotdb.confignode.consensus.request.write.region.OfferRegionMaintainTasksPlan; import org.apache.iotdb.confignode.consensus.request.write.region.PollSpecificRegionMaintainTaskPlan; @@ -454,6 +455,9 @@ public TSStatus executeNonQueryPlan(ConfigPhysicalPlan physicalPlan) case PollSpecificRegionMaintainTask: return partitionInfo.pollSpecificRegionMaintainTask( (PollSpecificRegionMaintainTaskPlan) physicalPlan); + case BatchRemoveRegionCreateTasks: + return partitionInfo.batchRemoveRegionCreateTasks( + (BatchRemoveRegionCreateTasksPlan) physicalPlan); case CreateSchemaPartition: return partitionInfo.createSchemaPartition((CreateSchemaPartitionPlan) physicalPlan); case CreateDataPartition: diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java index e71b61d7b338c..47685a60fcfe5 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java @@ -49,6 +49,7 @@ import org.apache.iotdb.confignode.consensus.request.write.partition.CreateSchemaPartitionPlan; import org.apache.iotdb.confignode.consensus.request.write.partition.RemoveRegionLocationPlan; import org.apache.iotdb.confignode.consensus.request.write.partition.UpdateRegionLocationPlan; +import org.apache.iotdb.confignode.consensus.request.write.region.BatchRemoveRegionCreateTasksPlan; import org.apache.iotdb.confignode.consensus.request.write.region.CreateRegionGroupsPlan; import org.apache.iotdb.confignode.consensus.request.write.region.OfferRegionMaintainTasksPlan; import org.apache.iotdb.confignode.consensus.request.write.region.PollSpecificRegionMaintainTaskPlan; @@ -63,6 +64,7 @@ import org.apache.iotdb.confignode.consensus.response.partition.SchemaPartitionResp; import org.apache.iotdb.confignode.exception.DatabaseNotExistsException; import org.apache.iotdb.confignode.i18n.ConfigNodeMessages; +import org.apache.iotdb.confignode.persistence.partition.maintainer.RegionCreateTask; import org.apache.iotdb.confignode.persistence.partition.maintainer.RegionMaintainTask; import org.apache.iotdb.confignode.persistence.partition.maintainer.RegionMaintainType; import org.apache.iotdb.confignode.rpc.thrift.TRegionInfo; @@ -192,37 +194,71 @@ public TSStatus createDatabase(final DatabaseSchemaPlan plan) { * @return {@link TSStatusCode#SUCCESS_STATUS} */ public TSStatus createRegionGroups(CreateRegionGroupsPlan plan) { - TSStatus result; - AtomicInteger maxRegionId = new AtomicInteger(Integer.MIN_VALUE); + updateNextRegionGroupId(plan); + + final TSStatus validationStatus = validateCreateRegionGroups(plan); + if (validationStatus.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) { + return validationStatus; + } plan.getRegionGroupMap() .forEach( (database, regionReplicaSets) -> { - if (isDatabasePreDeleted(database)) { - LOGGER.warn( - ConfigNodeMessages - .CREATEREGIONGROUPS_DATABASE_HAS_BEEN_DELETED_CORRESPONDING_REGIONGROUPS, - database); - return; - } databasePartitionTables.get(database).createRegionGroups(regionReplicaSets); - regionReplicaSets.forEach( - regionReplicaSet -> - maxRegionId.set( - Math.max(maxRegionId.get(), regionReplicaSet.getRegionId().getId()))); }); + return new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode()); + } + + /** Validates all databases before any RegionGroup in a potentially batched plan is persisted. */ + public TSStatus validateCreateRegionGroups(final CreateRegionGroupsPlan plan) { + for (final String database : plan.getRegionGroupMap().keySet()) { + final DatabasePartitionTable databasePartitionTable = databasePartitionTables.get(database); + if (databasePartitionTable == null) { + LOGGER.warn( + ConfigNodeMessages + .LOG_REJECT_CREATEREGIONGROUPSPLAN_BECAUSE_DATABASE_ARG_DOES_NOT_EXIST_616E0CDE, + database); + return new TSStatus(TSStatusCode.DATABASE_NOT_EXIST.getStatusCode()) + .setMessage( + String.format( + ConfigNodeMessages + .MESSAGE_CREATE_REGIONGROUPS_FAILED_BECAUSE_DATABASE_ARG_DOES_NOT_EXIST_AF0F2440, + database)); + } + if (!databasePartitionTable.isNotPreDeleted()) { + LOGGER.warn( + ConfigNodeMessages + .LOG_REJECT_CREATEREGIONGROUPSPLAN_BECAUSE_DATABASE_ARG_IS_BEING_DELETED_C085AC01, + database); + return new TSStatus(TSStatusCode.DATABASE_NOT_EXIST.getStatusCode()) + .setMessage( + String.format( + ConfigNodeMessages + .MESSAGE_CREATE_REGIONGROUPS_FAILED_BECAUSE_DATABASE_ARG_IS_BEING_DELETED_651DB780, + database)); + } + } + + return new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode()); + } + + private void updateNextRegionGroupId(final CreateRegionGroupsPlan plan) { + final int maxRegionId = + plan.getRegionGroupMap().values().stream() + .flatMap(List::stream) + .mapToInt(regionReplicaSet -> regionReplicaSet.getRegionId().getId()) + .max() + .orElse(Integer.MIN_VALUE); + // To ensure that the nextRegionGroupId is updated correctly when // the ConfigNode-followers concurrently processes CreateRegionsPlan, // we need to add a synchronization lock here synchronized (nextRegionGroupId) { - if (nextRegionGroupId.get() < maxRegionId.get()) { - nextRegionGroupId.set(maxRegionId.get()); + if (nextRegionGroupId.get() < maxRegionId) { + nextRegionGroupId.set(maxRegionId); } } - - result = new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode()); - return result; } /** @@ -233,6 +269,7 @@ public TSStatus createRegionGroups(CreateRegionGroupsPlan plan) { public TSStatus offerRegionMaintainTasks( OfferRegionMaintainTasksPlan offerRegionMaintainTasksPlan) { synchronized (regionMaintainTaskList) { + final Set existingTasks = new HashSet<>(regionMaintainTaskList); // The RegionMaintainer queue only recreates failed region replicas now; region deletion is // owned by RemoveRegionGroupProcedure. Drop any legacy DELETE task that an upgraded node may // replay from an old consensus log, so it cannot get stuck in the queue and block the @@ -245,7 +282,13 @@ public TSStatus offerRegionMaintainTasks( task.getRegionId()); continue; } - regionMaintainTaskList.add(task); + final RegionCreateTask createTask = (RegionCreateTask) task; + if (!isRegionCreateTaskOwnedByCurrentPartitionTable(createTask)) { + continue; + } + if (existingTasks.add(task)) { + regionMaintainTaskList.add(task); + } } return RpcUtils.SUCCESS_STATUS; } @@ -290,6 +333,37 @@ public TSStatus pollSpecificRegionMaintainTask(PollSpecificRegionMaintainTaskPla } } + /** Idempotently remove all RegionCreateTasks that belong to the specified database. */ + public TSStatus batchRemoveRegionCreateTasks(BatchRemoveRegionCreateTasksPlan plan) { + synchronized (regionMaintainTaskList) { + final DatabasePartitionTable databasePartitionTable = + databasePartitionTables.get(plan.getDatabase()); + if (databasePartitionTable == null || databasePartitionTable.isNotPreDeleted()) { + return RpcUtils.SUCCESS_STATUS; + } + regionMaintainTaskList.removeIf( + task -> + task instanceof RegionCreateTask + && Objects.equals( + plan.getDatabase(), ((RegionCreateTask) task).getStorageGroup())); + return RpcUtils.SUCCESS_STATUS; + } + } + + private boolean isRegionCreateTaskOwnedByCurrentPartitionTable(RegionCreateTask task) { + if (!isDatabaseExisted(task.getStorageGroup())) { + return false; + } + return getReplicaSets(task.getStorageGroup(), Collections.singletonList(task.getRegionId())) + .stream() + .anyMatch( + replicaSet -> + replicaSet.getDataNodeLocations().stream() + .anyMatch( + location -> + location.getDataNodeId() == task.getTargetDataNode().getDataNodeId())); + } + /** * Get a deep copy of RegionCleanList for RegionCleaner to maintain cluster RegionReplicas. * @@ -1024,10 +1098,13 @@ public boolean processTakeSnapshot(File snapshotDir) throws TException, IOExcept databasePartitionTableEntry.getValue().serialize(bufferedOutputStream, protocol); } - // serialize regionCleanList - ReadWriteIOUtils.write(regionMaintainTaskList.size(), bufferedOutputStream); - for (RegionMaintainTask task : regionMaintainTaskList) { - task.serialize(bufferedOutputStream, protocol); + // Serialize the queue under the same monitor used by every consensus mutation so the count + // and entries belong to one atomic snapshot. + synchronized (regionMaintainTaskList) { + ReadWriteIOUtils.write(regionMaintainTaskList.size(), bufferedOutputStream); + for (RegionMaintainTask task : regionMaintainTaskList) { + task.serialize(bufferedOutputStream, protocol); + } } // write to file diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/ConfigNodeProcedureEnv.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/ConfigNodeProcedureEnv.java index 76bf575681c87..2124761521c75 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/ConfigNodeProcedureEnv.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/ConfigNodeProcedureEnv.java @@ -53,8 +53,10 @@ import org.apache.iotdb.confignode.manager.schema.ClusterSchemaManager; import org.apache.iotdb.confignode.persistence.partition.PartitionInfo; import org.apache.iotdb.confignode.persistence.schema.ClusterSchemaInfo; +import org.apache.iotdb.confignode.procedure.Procedure; import org.apache.iotdb.confignode.procedure.exception.ProcedureException; import org.apache.iotdb.confignode.procedure.impl.schema.SchemaUtils; +import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLifecycleLockManager; import org.apache.iotdb.confignode.procedure.scheduler.LockQueue; import org.apache.iotdb.confignode.procedure.scheduler.ProcedureScheduler; import org.apache.iotdb.confignode.rpc.thrift.TAddConsensusGroupReq; @@ -115,6 +117,8 @@ public class ConfigNodeProcedureEnv { /** Add or remove node lock. */ private final LockQueue nodeLock = new LockQueue(); + private final DatabaseLifecycleLockManager databaseLifecycleLockManager; + private final ReentrantLock schedulerLock = new ReentrantLock(true); private final ReentrantLock submitRegionMigrateLock = new ReentrantLock(true); @@ -132,6 +136,7 @@ public class ConfigNodeProcedureEnv { public ConfigNodeProcedureEnv(ConfigManager configManager, ProcedureScheduler scheduler) { this.configManager = configManager; this.scheduler = scheduler; + this.databaseLifecycleLockManager = new DatabaseLifecycleLockManager(scheduler); this.regionMaintainHandler = new RegionMaintainHandler(configManager); this.removeDataNodeHandler = new RemoveDataNodeHandler(configManager); this.removeConfigNodeLock = new ReentrantLock(); @@ -159,9 +164,13 @@ public TSStatus deleteDatabaseConfig(final String name, final boolean isGenerate * @param preDeleteType execute/rollback * @param deleteSgName database name */ - public void preDeleteDatabase( + public TSStatus preDeleteDatabase( final PreDeleteDatabasePlan.PreDeleteType preDeleteType, final String deleteSgName) { - getPartitionManager().preDeleteDatabase(deleteSgName, preDeleteType); + return getPartitionManager().preDeleteDatabase(deleteSgName, preDeleteType); + } + + public TSStatus batchRemoveRegionCreateTasks(final String database) { + return getPartitionManager().batchRemoveRegionCreateTasks(database); } public boolean invalidateCache(final String databaseName) throws IOException, TException { @@ -490,6 +499,10 @@ public TSStatus persistRegionGroup(CreateRegionGroupsPlan createRegionGroupsPlan } } + public TSStatus validateCreateRegionGroups(final CreateRegionGroupsPlan createRegionGroupsPlan) { + return getPartitionManager().validateCreateRegionGroups(createRegionGroupsPlan); + } + /** * Force activating RegionGroup by setting status to Running, therefore the ConfigNode-leader can * select leader for it and use it to allocate new Partitions @@ -1135,6 +1148,27 @@ public LockQueue getNodeLock() { return nodeLock; } + /** + * Atomically tries to lock all databases in lexical order. + * + * @return the first database whose lock is unavailable, or null when all locks are acquired + */ + public String tryLockDatabases(final Procedure procedure, final Set databaseNames) { + return databaseLifecycleLockManager.tryLock(procedure, databaseNames); + } + + public void waitDatabaseLock(final Procedure procedure, final String databaseName) { + databaseLifecycleLockManager.waitProcedure(procedure, databaseName); + } + + public void releaseDatabaseLocks(final Procedure procedure, final Set databaseNames) { + databaseLifecycleLockManager.releaseLocks(procedure, databaseNames); + } + + public DatabaseLifecycleLockManager getDatabaseLifecycleLockManager() { + return databaseLifecycleLockManager; + } + public ProcedureScheduler getScheduler() { return scheduler; } diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/AbstractDatabaseProcedure.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/AbstractDatabaseProcedure.java new file mode 100644 index 0000000000000..fc41b9afbc5a3 --- /dev/null +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/AbstractDatabaseProcedure.java @@ -0,0 +1,67 @@ +/* + * 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.iotdb.confignode.procedure.impl; + +import org.apache.iotdb.confignode.procedure.env.ConfigNodeProcedureEnv; +import org.apache.iotdb.confignode.procedure.state.ProcedureLockState; + +import java.util.Set; + +/** A procedure that holds exclusive lifecycle locks for its databases until it finishes. */ +public abstract class AbstractDatabaseProcedure + extends StateMachineProcedure { + + private transient String waitingDatabase; + + protected AbstractDatabaseProcedure() { + super(); + } + + protected AbstractDatabaseProcedure(final boolean isGeneratedByPipe) { + super(isGeneratedByPipe); + } + + protected abstract Set getDatabaseNames(); + + @Override + protected ProcedureLockState acquireLock(final ConfigNodeProcedureEnv env) { + waitingDatabase = env.tryLockDatabases(this, getDatabaseNames()); + return waitingDatabase == null + ? ProcedureLockState.LOCK_ACQUIRED + : ProcedureLockState.LOCK_EVENT_WAIT; + } + + @Override + protected void waitForLock(final ConfigNodeProcedureEnv env) { + if (waitingDatabase != null) { + env.waitDatabaseLock(this, waitingDatabase); + } + } + + @Override + protected void releaseLock(final ConfigNodeProcedureEnv env) { + env.releaseDatabaseLocks(this, getDatabaseNames()); + } + + @Override + protected boolean holdLock(final ConfigNodeProcedureEnv env) { + return true; + } +} diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/region/CreateRegionGroupsProcedure.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/region/CreateRegionGroupsProcedure.java index 276cdf432d9e6..8672e8e793d80 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/region/CreateRegionGroupsProcedure.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/region/CreateRegionGroupsProcedure.java @@ -38,7 +38,7 @@ import org.apache.iotdb.confignode.persistence.partition.maintainer.RegionCreateTask; import org.apache.iotdb.confignode.procedure.env.ConfigNodeProcedureEnv; import org.apache.iotdb.confignode.procedure.exception.ProcedureException; -import org.apache.iotdb.confignode.procedure.impl.StateMachineProcedure; +import org.apache.iotdb.confignode.procedure.impl.AbstractDatabaseProcedure; import org.apache.iotdb.confignode.procedure.state.CreateRegionGroupsState; import org.apache.iotdb.confignode.procedure.store.ProcedureType; import org.apache.iotdb.consensus.exception.ConsensusException; @@ -61,7 +61,7 @@ import java.util.stream.Collectors; public class CreateRegionGroupsProcedure - extends StateMachineProcedure { + extends AbstractDatabaseProcedure { private static final Logger LOGGER = LoggerFactory.getLogger(CreateRegionGroupsProcedure.class); @@ -100,6 +100,23 @@ public CreateRegionGroupsProcedure( @Override protected Flow executeFromState( final ConfigNodeProcedureEnv env, final CreateRegionGroupsState state) { + final TSStatus validationStatus = env.validateCreateRegionGroups(createRegionGroupsPlan); + if (validationStatus.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) { + // Only SHUNT_REGION_REPLICAS can have sent create RPCs without transferring ownership of the + // planned RegionGroups to PartitionInfo. Delete every planned replica idempotently; all later + // states are already owned and cleaned by DeleteDatabaseProcedure. + if (state == CreateRegionGroupsState.SHUNT_REGION_REPLICAS) { + addPlannedRegionReplicaCleanup(); + // The children run before this existing terminal state. Validation is repeated when the + // parent resumes there, so the procedure reports the original fence failure only after all + // possible pre-persistence replicas have been deleted. + setNextState(CreateRegionGroupsState.CREATE_REGION_GROUPS_FINISH); + return Flow.HAS_MORE_STATE; + } + setFailure(new ProcedureException(new IoTDBException(validationStatus))); + return Flow.NO_MORE_STATE; + } + switch (state) { case CREATE_REGION_GROUPS: failedRegionReplicaSets = env.doRegionCreation(consensusGroupType, createRegionGroupsPlan); @@ -109,10 +126,7 @@ protected Flow executeFromState( persistPlan = new CreateRegionGroupsPlan(); final OfferRegionMaintainTasksPlan offerPlan = new OfferRegionMaintainTasksPlan(); // RegionGroups that failed to reach a serving quorum have their redundant (already-created) - // replicas removed via an independent root RemoveRegionGroupProcedure. Submitting them as - // root procedures (instead of children) keeps this procedure from waiting for or being - // failed by the cleanup: each one retries forever until those replicas are deleted, while - // this procedure proceeds to activate the region groups that did form a quorum. + // replicas removed by child procedures while this procedure retains its database locks. final List removeRegionGroupProcedures = new ArrayList<>(); // Filter those RegionGroups that created successfully createRegionGroupsPlan @@ -191,35 +205,32 @@ protected Flow executeFromState( final TSStatus persistStatus = env.persistRegionGroup(persistPlan); if (persistStatus.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) { - setFailure(new ProcedureException(new IoTDBException(persistStatus))); - return Flow.NO_MORE_STATE; + // Keep the database locks and retry the idempotent Consensus write. Releasing the lock or + // compensating here would create a gap in which the DataNodes may contain replicas but + // neither PartitionInfo nor a running lifecycle procedure owns them. + setNextState(CreateRegionGroupsState.SHUNT_REGION_REPLICAS); + return Flow.HAS_MORE_STATE; } + final TSStatus offerStatus; try { - env.getConfigManager().getConsensusManager().write(offerPlan); + offerStatus = env.getConfigManager().getConsensusManager().write(offerPlan); } catch (final ConsensusException e) { LOGGER.warn( ConfigNodeMessages.FAILED_IN_THE_WRITE_API_EXECUTING_THE_CONSENSUS_LAYER_DUE, e); + setNextState(CreateRegionGroupsState.SHUNT_REGION_REPLICAS); + return Flow.HAS_MORE_STATE; + } + if (offerStatus.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) { + setNextState(CreateRegionGroupsState.SHUNT_REGION_REPLICAS); + return Flow.HAS_MORE_STATE; } - // Submit the redundant-replica cleanups as independent root procedures. This is - // intentionally NOT guarded by isStateDeserialized(): the executor persists a procedure at - // a state BEFORE that state's body has run (it advances the state on the previous cycle, - // then may stop at the inter-state boundary on a leader switch — see - // ProcedureExecutor#executeProcedure), so a recovery that lands on SHUNT_REGION_REPLICAS - // means the submissions have NOT happened yet. Skipping them would leave the - // already-created - // replicas of sub-quorum region groups on disk with no cleanup and no partition-table - // record - // (the else branch above never persisted them). Re-submitting on recovery is safe instead: - // the cleanups are recomputed from the serialized failedRegionReplicaSets, each gets a - // fresh - // procId and performs an idempotent delete, so a duplicate is harmless whereas a skip - // leaks. - removeRegionGroupProcedures.forEach( - removeRegionGroupProcedure -> - env.getConfigManager() - .getProcedureManager() - .getExecutor() - .submitProcedure(removeRegionGroupProcedure)); + // Run redundant-replica cleanups as child procedures. The parent keeps the database locks + // until they finish, so database deletion cannot overtake an RPC whose RegionGroup was + // never persisted. This is intentionally NOT guarded by isStateDeserialized(): the + // executor persists a procedure at a state before that state's body has run, so recovery at + // SHUNT_REGION_REPLICAS must add these children. They are recomputed from the serialized + // failure map and delete idempotently. + removeRegionGroupProcedures.forEach(this::addChildProcedure); setNextState(CreateRegionGroupsState.REBALANCE_DATA_PARTITION_POLICY); break; case REBALANCE_DATA_PARTITION_POLICY: @@ -323,6 +334,22 @@ protected CreateRegionGroupsState getInitialState() { return CreateRegionGroupsState.CREATE_REGION_GROUPS; } + @Override + protected Set getDatabaseNames() { + return createRegionGroupsPlan.getRegionGroupMap().keySet(); + } + + public boolean containsDatabase(final String database) { + return createRegionGroupsPlan.getRegionGroupMap().containsKey(database); + } + + private void addPlannedRegionReplicaCleanup() { + createRegionGroupsPlan.getRegionGroupMap().values().stream() + .flatMap(List::stream) + .map(RemoveRegionGroupProcedure::new) + .forEach(this::addChildProcedure); + } + @Override public void serialize(final DataOutputStream stream) throws IOException { // Must serialize CREATE_REGION_GROUPS.getTypeCode() firstly diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/region/RemoveRegionGroupProcedure.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/region/RemoveRegionGroupProcedure.java index b2b49a688d204..c9c460d855a51 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/region/RemoveRegionGroupProcedure.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/region/RemoveRegionGroupProcedure.java @@ -60,16 +60,14 @@ * never finished forming. The DataNode runs the deletion asynchronously and this procedure polls * for the result, so a slow deletion is never wrongly reported as finished. * - *

This procedure is submitted as an independent root procedure (not a child) by its callers, - * which only enqueue the deletion and return immediately. It therefore owns the deletion end to - * end: on any failure it retries the current replica forever (backing off between attempts) instead - * of giving up, because there is no parent left to fall back to and the region's peer/data must not - * be left on disk. Each genuine re-attempt uses a FRESH DataNode-side taskId (the DataNode dedups - * by taskId and caches a terminal result forever, so reusing one taskId would make every retry a - * no-op that never re-runs the delete); the in-flight taskId is persisted so a leader change - * re-polls the same task rather than double-submitting. It carries its own {@link - * TRegionReplicaSet} copy, so it can finish even after the caller has dropped the partition table, - * and it survives ConfigNode leader change / restart. + *

Database lifecycle procedures run this procedure as a child while retaining the database lock. + * On any failure it retries the current replica forever (backing off between attempts) instead of + * giving up, because the region's peer/data must not be left on disk. Each genuine re-attempt uses + * a FRESH DataNode-side taskId (the DataNode dedups by taskId and caches a terminal result forever, + * so reusing one taskId would make every retry a no-op that never re-runs the delete); the + * in-flight taskId is persisted so a leader change re-polls the same task rather than + * double-submitting. It carries its own {@link TRegionReplicaSet} copy and survives ConfigNode + * leader change / restart. */ public class RemoveRegionGroupProcedure extends RegionOperationProcedure { private static final Logger LOGGER = LoggerFactory.getLogger(RemoveRegionGroupProcedure.class); diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteDatabaseProcedure.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteDatabaseProcedure.java index 2af7bd7ad699c..c9240878c9dae 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteDatabaseProcedure.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteDatabaseProcedure.java @@ -29,7 +29,7 @@ import org.apache.iotdb.confignode.manager.partition.PartitionMetrics; import org.apache.iotdb.confignode.procedure.env.ConfigNodeProcedureEnv; import org.apache.iotdb.confignode.procedure.exception.ProcedureException; -import org.apache.iotdb.confignode.procedure.impl.StateMachineProcedure; +import org.apache.iotdb.confignode.procedure.impl.AbstractDatabaseProcedure; import org.apache.iotdb.confignode.procedure.impl.region.RemoveRegionGroupProcedure; import org.apache.iotdb.confignode.procedure.state.schema.DeleteDatabaseState; import org.apache.iotdb.confignode.procedure.store.ProcedureType; @@ -43,11 +43,12 @@ import java.io.DataOutputStream; import java.io.IOException; import java.nio.ByteBuffer; +import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.Set; -public class DeleteDatabaseProcedure - extends StateMachineProcedure { +public class DeleteDatabaseProcedure extends AbstractDatabaseProcedure { private static final Logger LOG = LoggerFactory.getLogger(DeleteDatabaseProcedure.class); private static final int RETRY_THRESHOLD = 5; @@ -83,16 +84,35 @@ protected Flow executeFromState(final ConfigNodeProcedureEnv env, final DeleteDa LOG.info( ProcedureMessages.LOG_DELETEDATABASEPROCEDURE_PRE_DELETE_DATABASE_ARG_6A1FEACC, deleteDatabaseSchema.getName()); - env.preDeleteDatabase( - PreDeleteDatabasePlan.PreDeleteType.EXECUTE, deleteDatabaseSchema.getName()); - setNextState(DeleteDatabaseState.INVALIDATE_CACHE); + final TSStatus preDeleteStatus = + env.preDeleteDatabase( + PreDeleteDatabasePlan.PreDeleteType.EXECUTE, deleteDatabaseSchema.getName()); + if (preDeleteStatus.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) { + setNextState(DeleteDatabaseState.INVALIDATE_CACHE); + } else if (getCycles() > RETRY_THRESHOLD) { + setFailure( + new ProcedureException( + ProcedureMessages.DELETEDATABASEPROCEDURE_DELETE_DATABASESCHEMA_FAILED)); + } else { + setNextState(DeleteDatabaseState.PRE_DELETE_DATABASE); + } break; case INVALIDATE_CACHE: LOG.info( ProcedureMessages.LOG_DELETEDATABASEPROCEDURE_INVALIDATE_CACHE_DATABASE_ARG_299FC9BC, deleteDatabaseSchema.getName()); if (env.invalidateCache(deleteDatabaseSchema.getName())) { - setNextState(DeleteDatabaseState.DELETE_DATABASE_SCHEMA); + final TSStatus removeTasksStatus = + env.batchRemoveRegionCreateTasks(deleteDatabaseSchema.getName()); + if (removeTasksStatus.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) { + setNextState(DeleteDatabaseState.DELETE_DATABASE_SCHEMA); + } else if (getCycles() > RETRY_THRESHOLD) { + setFailure( + new ProcedureException( + ProcedureMessages.DELETEDATABASEPROCEDURE_DELETE_DATABASESCHEMA_FAILED)); + } else { + setNextState(DeleteDatabaseState.INVALIDATE_CACHE); + } } else { setFailure( new ProcedureException( @@ -104,13 +124,10 @@ protected Flow executeFromState(final ConfigNodeProcedureEnv env, final DeleteDa ProcedureMessages.LOG_DELETEDATABASEPROCEDURE_DELETE_DATABASESCHEMA_ARG_A49A47AC, deleteDatabaseSchema.getName()); - // Enqueue deletion of every region group (both schema and data regions) of this database. - // Each is submitted as an INDEPENDENT root RemoveRegionGroupProcedure rather than a - // child: - // this procedure only submits the deletions and then returns, so it can neither wait for - // nor be failed/rolled-back by a slow or failing region deletion. Each carries its own - // copy of the replica set, so the deletion still completes (and survives leader change / - // restart) even after the next state drops the partition table. + // Delete every RegionGroup as a child procedure. This procedure keeps the database lock + // while the children run, so neither same-name recreation nor a delayed Region creation + // can overtake cleanup. Each child carries its own replica-set copy and survives leader + // change or restart. // // Submission is intentionally NOT guarded by isStateDeserialized(): the executor persists // a procedure at a state BEFORE that state's body has run (it advances the state on the @@ -118,10 +135,8 @@ protected Flow executeFromState(final ConfigNodeProcedureEnv env, final DeleteDa // ProcedureExecutor#executeProcedure). So a recovery that lands on this state means the // submission has NOT happened yet; skipping it would drop every region group's cleanup // while the next state still drops the partition table, orphaning the region peers/data - // on - // disk with no record of where they live. Re-submitting on recovery is safe instead: - // every RemoveRegionGroupProcedure gets a fresh procId and performs an idempotent delete, - // so a duplicate is harmless whereas a skip leaks data. + // on disk with no record of where they live. Adding the children again on recovery is + // safe because RegionGroup deletion is idempotent. final List regionReplicaSets = env.getAllReplicaSets(deleteDatabaseSchema.getName()); regionReplicaSets.forEach( @@ -130,10 +145,7 @@ protected Flow executeFromState(final ConfigNodeProcedureEnv env, final DeleteDa env.getConfigManager() .getLoadManager() .removeRegionGroupRelatedCache(regionReplicaSet.getRegionId()); - env.getConfigManager() - .getProcedureManager() - .getExecutor() - .submitProcedure(new RemoveRegionGroupProcedure(regionReplicaSet)); + addChildProcedure(new RemoveRegionGroupProcedure(regionReplicaSet)); }); setNextState(DeleteDatabaseState.DELETE_DATABASE_CONFIG); break; @@ -166,6 +178,8 @@ protected Flow executeFromState(final ConfigNodeProcedureEnv env, final DeleteDa setFailure( new ProcedureException( ProcedureMessages.DELETEDATABASEPROCEDURE_DELETE_DATABASESCHEMA_FAILED)); + } else { + setNextState(DeleteDatabaseState.DELETE_DATABASE_CONFIG); } } } catch (final TException | IOException e) { @@ -187,6 +201,8 @@ protected Flow executeFromState(final ConfigNodeProcedureEnv env, final DeleteDa setFailure( new ProcedureException( ProcedureMessages.DELETEDATABASEPROCEDURE_STATE_STUCK_AT + state)); + } else { + setNextState(state); } } } @@ -236,6 +252,13 @@ protected DeleteDatabaseState getInitialState() { return DeleteDatabaseState.PRE_DELETE_DATABASE; } + @Override + protected Set getDatabaseNames() { + return deleteDatabaseSchema == null + ? Collections.emptySet() + : Collections.singleton(deleteDatabaseSchema.getName()); + } + public String getDatabase() { return deleteDatabaseSchema.getName(); } diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManager.java new file mode 100644 index 0000000000000..31f75950eb43d --- /dev/null +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManager.java @@ -0,0 +1,279 @@ +/* + * 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.iotdb.confignode.procedure.scheduler; + +import org.apache.iotdb.confignode.procedure.Procedure; + +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.ReentrantLock; + +/** + * The single source of database lifecycle locks in a ConfigNode. + * + *

A lock is keyed by the exact database name. Both short-lived manager requests and procedures + * use the same ownership table, so database creation, Region creation, maintenance retries, and + * database deletion cannot bypass one another. Procedure ownership is identified by procedure id + * rather than worker thread because a procedure can resume on a different executor thread. + */ +public class DatabaseLifecycleLockManager { + + private final ProcedureScheduler scheduler; + private final ReentrantLock stateLock = new ReentrantLock(true); + private final Condition lockReleased = stateLock.newCondition(); + private final Map lockStateMap = new HashMap<>(); + + public DatabaseLifecycleLockManager(final ProcedureScheduler scheduler) { + this.scheduler = scheduler; + } + + /** Acquire database locks for a synchronous manager request. */ + public DatabaseLock acquireLocks(final Set databaseNames) { + final List orderedDatabases = orderedDatabases(databaseNames); + final Thread owner = Thread.currentThread(); + stateLock.lock(); + try { + while (!canAcquireRequestLocks(owner, orderedDatabases)) { + lockReleased.awaitUninterruptibly(); + } + orderedDatabases.forEach( + database -> + lockStateMap + .computeIfAbsent(database, ignored -> new DatabaseLockState()) + .acquireRequestLock(owner)); + return new DatabaseLock(this, orderedDatabases, owner); + } finally { + stateLock.unlock(); + } + } + + /** + * Atomically tries to lock all databases for a procedure. + * + * @return the first database whose lock is unavailable, or null when all locks are acquired + */ + public String tryLock(final Procedure procedure, final Set databaseNames) { + stateLock.lock(); + try { + final List acquiredDatabases = new ArrayList<>(); + for (final String database : orderedDatabases(databaseNames)) { + final DatabaseLockState lockState = + lockStateMap.computeIfAbsent(database, ignored -> new DatabaseLockState()); + if (!lockState.canAcquireProcedureLock(procedure)) { + acquiredDatabases.forEach( + acquiredDatabase -> releaseProcedureLock(procedure, acquiredDatabase)); + return database; + } + if (lockState.acquireProcedureLock(procedure)) { + acquiredDatabases.add(database); + } + } + return null; + } finally { + stateLock.unlock(); + } + } + + public void waitProcedure(final Procedure procedure, final String databaseName) { + stateLock.lock(); + try { + final DatabaseLockState lockState = + lockStateMap.computeIfAbsent(databaseName, ignored -> new DatabaseLockState()); + if (lockState.isUnlocked()) { + scheduler.addFront(procedure); + removeIfIdle(databaseName, lockState); + } else { + lockState.waitProcedure(procedure); + } + } finally { + stateLock.unlock(); + } + } + + public void releaseLocks(final Procedure procedure, final Set databaseNames) { + stateLock.lock(); + try { + orderedDatabases(databaseNames) + .forEach(database -> releaseProcedureLock(procedure, database)); + } finally { + stateLock.unlock(); + } + } + + private boolean canAcquireRequestLocks(final Thread owner, final List orderedDatabases) { + for (final String database : orderedDatabases) { + final DatabaseLockState lockState = lockStateMap.get(database); + if (lockState != null && !lockState.canAcquireRequestLock(owner)) { + return false; + } + } + return true; + } + + private void releaseRequestLocks(final List orderedDatabases, final Thread requestOwner) { + stateLock.lock(); + try { + for (final String database : orderedDatabases) { + final DatabaseLockState lockState = lockStateMap.get(database); + if (lockState != null && lockState.releaseRequestLock(requestOwner)) { + wakeWaiters(lockState); + removeIfIdle(database, lockState); + } + } + } finally { + stateLock.unlock(); + } + } + + private void releaseProcedureLock(final Procedure procedure, final String database) { + final DatabaseLockState lockState = lockStateMap.get(database); + if (lockState != null && lockState.releaseProcedureLock(procedure)) { + wakeWaiters(lockState); + removeIfIdle(database, lockState); + } + } + + private void wakeWaiters(final DatabaseLockState lockState) { + lockState.wakeWaitingProcedures(scheduler); + lockReleased.signalAll(); + } + + private void removeIfIdle(final String database, final DatabaseLockState lockState) { + if (lockState.isIdle()) { + lockStateMap.remove(database, lockState); + } + } + + private static List orderedDatabases(final Set databaseNames) { + return new ArrayList<>(new TreeSet<>(databaseNames)); + } + + public static final class DatabaseLock implements AutoCloseable { + private final DatabaseLifecycleLockManager lockManager; + private final List databaseNames; + private final Thread owner; + private boolean closed; + + private DatabaseLock( + final DatabaseLifecycleLockManager lockManager, + final List databaseNames, + final Thread owner) { + this.lockManager = lockManager; + this.databaseNames = databaseNames; + this.owner = owner; + } + + @Override + public void close() { + if (!closed) { + closed = true; + lockManager.releaseRequestLocks(databaseNames, owner); + } + } + } + + private static final class DatabaseLockState { + private final ArrayDeque> waitingProcedures = new ArrayDeque<>(); + private Procedure procedureOwner; + private Thread requestOwner; + private int requestHoldCount; + + private boolean canAcquireRequestLock(final Thread owner) { + if (requestOwner == owner) { + return true; + } + return requestOwner == null && procedureOwner == null && waitingProcedures.isEmpty(); + } + + private void acquireRequestLock(final Thread owner) { + requestOwner = owner; + requestHoldCount++; + } + + /** + * @return true when the lock became fully released + */ + private boolean releaseRequestLock(final Thread owner) { + if (requestOwner != owner) { + return false; + } + requestHoldCount--; + if (requestHoldCount == 0) { + requestOwner = null; + return true; + } + return false; + } + + private boolean canAcquireProcedureLock(final Procedure procedure) { + return requestOwner == null + && (procedureOwner == null || procedureOwner.getProcId() == procedure.getProcId()); + } + + /** + * @return true when this invocation newly acquired the lock + */ + private boolean acquireProcedureLock(final Procedure procedure) { + if (procedureOwner == null) { + procedureOwner = procedure; + return true; + } + return false; + } + + /** + * @return true when the lock was released + */ + private boolean releaseProcedureLock(final Procedure procedure) { + if (procedureOwner == null || procedureOwner.getProcId() != procedure.getProcId()) { + return false; + } + procedureOwner = null; + return true; + } + + private void waitProcedure(final Procedure procedure) { + if (waitingProcedures.stream() + .noneMatch(waitingProcedure -> waitingProcedure.getProcId() == procedure.getProcId())) { + waitingProcedures.addLast(procedure); + } + } + + private void wakeWaitingProcedures(final ProcedureScheduler procedureScheduler) { + while (!waitingProcedures.isEmpty()) { + procedureScheduler.addFront(waitingProcedures.pollFirst()); + } + } + + private boolean isUnlocked() { + return procedureOwner == null && requestOwner == null; + } + + private boolean isIdle() { + return isUnlocked() && waitingProcedures.isEmpty(); + } + } +} diff --git a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanSerDeTest.java b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanSerDeTest.java index e844dcf6910e9..4fe56aa3f7d43 100644 --- a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanSerDeTest.java +++ b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanSerDeTest.java @@ -118,6 +118,7 @@ import org.apache.iotdb.confignode.consensus.request.write.procedure.UpdateProcedurePlan; import org.apache.iotdb.confignode.consensus.request.write.quota.SetSpaceQuotaPlan; import org.apache.iotdb.confignode.consensus.request.write.quota.SetThrottleQuotaPlan; +import org.apache.iotdb.confignode.consensus.request.write.region.BatchRemoveRegionCreateTasksPlan; import org.apache.iotdb.confignode.consensus.request.write.region.CreateRegionGroupsPlan; import org.apache.iotdb.confignode.consensus.request.write.region.OfferRegionMaintainTasksPlan; import org.apache.iotdb.confignode.consensus.request.write.region.PollRegionMaintainTaskPlan; @@ -273,6 +274,19 @@ public void CreateDatabasePlanTest() throws IOException { Assert.assertEquals(req0, req1); } + @Test + public void CreateRegionGroupsPlanTest() throws IOException { + final CreateRegionGroupsPlan plan = new CreateRegionGroupsPlan(); + plan.addRegionGroup( + "root.sg", + new TRegionReplicaSet( + new TConsensusGroupId(TConsensusGroupType.DataRegion, 1), Collections.emptyList())); + + final CreateRegionGroupsPlan deserializedPlan = + (CreateRegionGroupsPlan) ConfigPhysicalPlan.Factory.create(plan.serializeToByteBuffer()); + Assert.assertEquals(plan, deserializedPlan); + } + @Test public void AlterDatabasePlanTest() throws IOException { DatabaseSchemaPlan req0 = @@ -423,6 +437,15 @@ public void PollRegionMaintainTaskPlan() throws IOException { Assert.assertEquals(plan0, plan1); } + @Test + public void BatchRemoveRegionCreateTasksPlanTest() throws IOException { + final BatchRemoveRegionCreateTasksPlan plan0 = new BatchRemoveRegionCreateTasksPlan("root.sg"); + final BatchRemoveRegionCreateTasksPlan plan1 = + (BatchRemoveRegionCreateTasksPlan) + ConfigPhysicalPlan.Factory.create(plan0.serializeToByteBuffer()); + Assert.assertEquals(plan0, plan1); + } + @Test public void CreateSchemaPartitionPlanTest() throws IOException { TDataNodeLocation dataNodeLocation = new TDataNodeLocation(); diff --git a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/ClusterSchemaManagerTest.java b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/ClusterSchemaManagerTest.java index 261f60ed65a1d..db40647f2377b 100644 --- a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/ClusterSchemaManagerTest.java +++ b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/ClusterSchemaManagerTest.java @@ -18,10 +18,29 @@ */ package org.apache.iotdb.confignode.manager; +import org.apache.iotdb.common.rpc.thrift.TSStatus; +import org.apache.iotdb.confignode.consensus.request.ConfigPhysicalPlanType; +import org.apache.iotdb.confignode.consensus.request.write.database.DatabaseSchemaPlan; import org.apache.iotdb.confignode.manager.schema.ClusterSchemaManager; +import org.apache.iotdb.confignode.manager.schema.ClusterSchemaQuotaStatistics; +import org.apache.iotdb.confignode.persistence.schema.ClusterSchemaInfo; +import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLifecycleLockManager; +import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLifecycleLockManager.DatabaseLock; +import org.apache.iotdb.confignode.procedure.scheduler.SimpleProcedureScheduler; +import org.apache.iotdb.confignode.rpc.thrift.TDatabaseSchema; +import org.apache.iotdb.rpc.TSStatusCode; import org.junit.Assert; import org.junit.Test; +import org.mockito.Mockito; + +import java.util.Collections; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; public class ClusterSchemaManagerTest { @@ -37,4 +56,45 @@ public void testCalcMaxRegionGroupNum() { // (resourceWeight * resource) / (createdStorageGroupNum * replicationFactor) Assert.assertEquals(20, ClusterSchemaManager.calcMaxRegionGroupNum(3, 1.0, 120, 2, 3, 5)); } + + @Test + public void testSetDatabaseWaitsForLifecycleAdmissionBeforeCheckingProcedures() throws Exception { + final String database = "root.sg"; + final IManager configManager = Mockito.mock(IManager.class); + final ProcedureManager procedureManager = Mockito.mock(ProcedureManager.class); + final DatabaseLifecycleLockManager lockManager = + new DatabaseLifecycleLockManager(new SimpleProcedureScheduler()); + final CountDownLatch admissionAttempted = new CountDownLatch(1); + final AtomicBoolean unfinishedProcedure = new AtomicBoolean(false); + Mockito.when(configManager.getProcedureManager()).thenReturn(procedureManager); + Mockito.when(procedureManager.acquireDatabaseLifecycleLock(database)) + .thenAnswer( + ignored -> { + admissionAttempted.countDown(); + return lockManager.acquireLocks(Collections.singleton(database)); + }); + Mockito.when(procedureManager.hasUnfinishedDatabaseLifecycleProcedure(database)) + .thenAnswer(ignored -> unfinishedProcedure.get()); + + final ClusterSchemaManager schemaManager = + new ClusterSchemaManager( + configManager, + Mockito.mock(ClusterSchemaInfo.class), + Mockito.mock(ClusterSchemaQuotaStatistics.class)); + final DatabaseSchemaPlan plan = + new DatabaseSchemaPlan( + ConfigPhysicalPlanType.CreateDatabase, new TDatabaseSchema(database)); + final ExecutorService executor = Executors.newSingleThreadExecutor(); + + final Future statusFuture; + try (final DatabaseLock ignored = lockManager.acquireLocks(Collections.singleton(database))) { + statusFuture = executor.submit(() -> schemaManager.setDatabase(plan, false)); + Assert.assertTrue(admissionAttempted.await(10, TimeUnit.SECONDS)); + unfinishedProcedure.set(true); + } + final TSStatus status = statusFuture.get(10, TimeUnit.SECONDS); + Assert.assertEquals(TSStatusCode.METADATA_ERROR.getStatusCode(), status.getCode()); + Mockito.verify(procedureManager).hasUnfinishedDatabaseLifecycleProcedure(database); + executor.shutdownNow(); + } } diff --git a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/ProcedureManagerTest.java b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/ProcedureManagerTest.java index 4de883b9c158f..193abc74cfa5e 100644 --- a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/ProcedureManagerTest.java +++ b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/ProcedureManagerTest.java @@ -23,19 +23,24 @@ import org.apache.iotdb.common.rpc.thrift.TConsensusGroupType; import org.apache.iotdb.common.rpc.thrift.TDataNodeLocation; import org.apache.iotdb.common.rpc.thrift.TEndPoint; +import org.apache.iotdb.common.rpc.thrift.TRegionReplicaSet; import org.apache.iotdb.common.rpc.thrift.TSStatus; import org.apache.iotdb.commons.cluster.NodeStatus; import org.apache.iotdb.commons.pipe.config.constant.PipeSourceConstant; import org.apache.iotdb.commons.schema.table.TreeViewSchema; import org.apache.iotdb.commons.schema.table.TsTable; +import org.apache.iotdb.confignode.consensus.request.write.region.CreateRegionGroupsPlan; import org.apache.iotdb.confignode.manager.load.LoadManager; import org.apache.iotdb.confignode.procedure.Procedure; import org.apache.iotdb.confignode.procedure.ProcedureExecutor; import org.apache.iotdb.confignode.procedure.env.ConfigNodeProcedureEnv; import org.apache.iotdb.confignode.procedure.env.RemoveDataNodeHandler; import org.apache.iotdb.confignode.procedure.impl.node.RemoveDataNodesProcedure; +import org.apache.iotdb.confignode.procedure.impl.region.CreateRegionGroupsProcedure; import org.apache.iotdb.confignode.procedure.impl.region.RegionMigrateProcedure; import org.apache.iotdb.confignode.procedure.impl.region.RegionMigrationPlan; +import org.apache.iotdb.confignode.procedure.impl.schema.DeleteDatabaseProcedure; +import org.apache.iotdb.confignode.rpc.thrift.TDatabaseSchema; import org.junit.Assert; import org.junit.BeforeClass; @@ -128,6 +133,7 @@ public static void setUp() throws IOException { when(PROCEDURE_MANAGER.getExecutor()).thenReturn(PROCEDURE_EXECUTOR); when(PROCEDURE_EXECUTOR.getProcedures()).thenReturn(procedureMap); + PROCEDURE_MANAGER.setExecutor(PROCEDURE_EXECUTOR); when(PROCEDURE_MANAGER.getEnv()).thenReturn(ENV); when(ENV.getRemoveDataNodeHandler()).thenReturn(REMOVE_DATA_NODE_HANDLER); } @@ -233,4 +239,31 @@ public void testInjectTreeViewSourceAttributesPreservesUserTreePattern() { Assert.assertFalse( topicAttributes.containsKey(PipeSourceConstant.SOURCE_PATTERN_INCLUSION_KEY)); } + + @Test + public void testDetectUnfinishedDatabaseLifecycleProcedures() { + final CreateRegionGroupsPlan createPlan = new CreateRegionGroupsPlan(); + createPlan.addRegionGroup( + "root.create", + new TRegionReplicaSet( + new TConsensusGroupId(TConsensusGroupType.DataRegion, 10), List.of())); + final CreateRegionGroupsProcedure createProcedure = + new CreateRegionGroupsProcedure(TConsensusGroupType.DataRegion, createPlan); + createProcedure.setProcId(100); + final DeleteDatabaseProcedure deleteProcedure = + new DeleteDatabaseProcedure(new TDatabaseSchema("root.delete"), false); + deleteProcedure.setProcId(101); + + procedureMap.clear(); + try { + procedureMap.put(createProcedure.getProcId(), createProcedure); + procedureMap.put(deleteProcedure.getProcId(), deleteProcedure); + + Assert.assertTrue(PROCEDURE_MANAGER.hasUnfinishedDatabaseLifecycleProcedure("root.create")); + Assert.assertTrue(PROCEDURE_MANAGER.hasUnfinishedDatabaseLifecycleProcedure("root.delete")); + Assert.assertFalse(PROCEDURE_MANAGER.hasUnfinishedDatabaseLifecycleProcedure("root.other")); + } finally { + procedureMap.clear(); + } + } } diff --git a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/persistence/PartitionInfoTest.java b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/persistence/PartitionInfoTest.java index 8c32378cfcb1a..8a13c24384be0 100644 --- a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/persistence/PartitionInfoTest.java +++ b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/persistence/PartitionInfoTest.java @@ -24,6 +24,7 @@ import org.apache.iotdb.common.rpc.thrift.TDataNodeLocation; import org.apache.iotdb.common.rpc.thrift.TEndPoint; import org.apache.iotdb.common.rpc.thrift.TRegionReplicaSet; +import org.apache.iotdb.common.rpc.thrift.TSStatus; import org.apache.iotdb.common.rpc.thrift.TSeriesPartitionSlot; import org.apache.iotdb.common.rpc.thrift.TTimePartitionSlot; import org.apache.iotdb.commons.partition.DataPartitionTable; @@ -32,11 +33,15 @@ import org.apache.iotdb.confignode.consensus.request.ConfigPhysicalPlanType; import org.apache.iotdb.confignode.consensus.request.read.region.GetRegionInfoListPlan; import org.apache.iotdb.confignode.consensus.request.write.database.DatabaseSchemaPlan; +import org.apache.iotdb.confignode.consensus.request.write.database.DeleteDatabasePlan; +import org.apache.iotdb.confignode.consensus.request.write.database.PreDeleteDatabasePlan; import org.apache.iotdb.confignode.consensus.request.write.partition.CreateDataPartitionPlan; import org.apache.iotdb.confignode.consensus.request.write.partition.CreateSchemaPartitionPlan; +import org.apache.iotdb.confignode.consensus.request.write.region.BatchRemoveRegionCreateTasksPlan; import org.apache.iotdb.confignode.consensus.request.write.region.CreateRegionGroupsPlan; import org.apache.iotdb.confignode.consensus.request.write.region.OfferRegionMaintainTasksPlan; import org.apache.iotdb.confignode.consensus.response.partition.RegionInfoListResp; +import org.apache.iotdb.confignode.exception.DatabaseNotExistsException; import org.apache.iotdb.confignode.persistence.partition.PartitionInfo; import org.apache.iotdb.confignode.persistence.partition.maintainer.RegionCreateTask; import org.apache.iotdb.confignode.persistence.partition.maintainer.RegionDeleteTask; @@ -44,6 +49,7 @@ import org.apache.iotdb.confignode.persistence.partition.maintainer.RegionMaintainType; import org.apache.iotdb.confignode.rpc.thrift.TDatabaseSchema; import org.apache.iotdb.confignode.rpc.thrift.TShowRegionReq; +import org.apache.iotdb.rpc.TSStatusCode; import org.apache.thrift.TException; import org.apache.tsfile.external.commons.io.FileUtils; @@ -52,8 +58,10 @@ import org.junit.Before; import org.junit.Test; +import java.io.DataInputStream; import java.io.File; import java.io.IOException; +import java.nio.file.Files; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -148,6 +156,17 @@ public void testSnapshot() throws TException, IOException { partitionInfo.offerRegionMaintainTasks(generateOfferRegionMaintainTasksPlan()); Assert.assertTrue(partitionInfo.processTakeSnapshot(snapshotDir)); + try (final DataInputStream inputStream = + new DataInputStream( + Files.newInputStream(new File(snapshotDir, "partition_info.bin").toPath()))) { + // Keep the historical snapshot framing: next RegionGroupId followed by database count. + Assert.assertEquals( + Math.max( + schemaRegionReplicaSet.getRegionId().getId(), + dataRegionReplicaSet.getRegionId().getId()), + inputStream.readInt()); + Assert.assertEquals(1, inputStream.readInt()); + } PartitionInfo partitionInfo1 = new PartitionInfo(); partitionInfo1.processLoadSnapshot(snapshotDir); @@ -162,7 +181,16 @@ public void testLegacyRegionDeleteTasksAreFiltered() throws TException, IOExcept // it cannot block the recreation of that region's other replicas. // The offer plan mixes two RegionCreateTasks with one legacy RegionDeleteTask. - partitionInfo.offerRegionMaintainTasks(generateOfferRegionMaintainTasksPlan()); + final OfferRegionMaintainTasksPlan offerPlan = generateOfferRegionMaintainTasksPlan(); + final RegionCreateTask createTask = + (RegionCreateTask) offerPlan.getRegionMaintainTaskList().get(0); + partitionInfo.createDatabase( + new DatabaseSchemaPlan( + ConfigPhysicalPlanType.CreateDatabase, new TDatabaseSchema("root.sg"))); + final CreateRegionGroupsPlan createRegionGroupsPlan = new CreateRegionGroupsPlan(); + createRegionGroupsPlan.addRegionGroup("root.sg", createTask.getRegionReplicaSet()); + partitionInfo.createRegionGroups(createRegionGroupsPlan); + partitionInfo.offerRegionMaintainTasks(offerPlan); // The DELETE task is filtered out at offer time; only the two CREATE tasks remain queued. List queuedTasks = partitionInfo.getRegionMaintainEntryList(); @@ -179,6 +207,165 @@ public void testLegacyRegionDeleteTasksAreFiltered() throws TException, IOExcept Assert.assertEquals(2, loaded.getRegionMaintainEntryList().size()); } + @Test + public void testBatchRemoveAllRegionCreateTasksAndSnapshot() throws TException, IOException { + final String database = "root.sg"; + final String otherDatabase = "root.other"; + partitionInfo.createDatabase( + new DatabaseSchemaPlan( + ConfigPhysicalPlanType.CreateDatabase, new TDatabaseSchema(database))); + partitionInfo.createDatabase( + new DatabaseSchemaPlan( + ConfigPhysicalPlanType.CreateDatabase, new TDatabaseSchema(otherDatabase))); + + final TRegionReplicaSet region0 = + generateTRegionReplicaSet(0, new TConsensusGroupId(TConsensusGroupType.DataRegion, 0)); + final TRegionReplicaSet region1 = + generateTRegionReplicaSet(10, new TConsensusGroupId(TConsensusGroupType.DataRegion, 1)); + final TRegionReplicaSet otherRegion = + generateTRegionReplicaSet(20, new TConsensusGroupId(TConsensusGroupType.DataRegion, 2)); + final CreateRegionGroupsPlan createRegionGroupsPlan = new CreateRegionGroupsPlan(); + createRegionGroupsPlan.addRegionGroup(database, region0); + createRegionGroupsPlan.addRegionGroup(database, region1); + createRegionGroupsPlan.addRegionGroup(otherDatabase, otherRegion); + partitionInfo.createRegionGroups(createRegionGroupsPlan); + + final OfferRegionMaintainTasksPlan offerPlan = new OfferRegionMaintainTasksPlan(); + offerPlan.appendRegionMaintainTask( + new RegionCreateTask(region0.getDataNodeLocations().get(0), database, region0)); + offerPlan.appendRegionMaintainTask( + new RegionCreateTask(region0.getDataNodeLocations().get(1), database, region0)); + offerPlan.appendRegionMaintainTask( + new RegionCreateTask(region1.getDataNodeLocations().get(0), database, region1)); + offerPlan.appendRegionMaintainTask( + new RegionCreateTask( + otherRegion.getDataNodeLocations().get(0), otherDatabase, otherRegion)); + partitionInfo.offerRegionMaintainTasks(offerPlan); + Assert.assertEquals(4, partitionInfo.getRegionMaintainEntryList().size()); + // Retrying an uncertain Consensus write must not duplicate durable create tasks. + partitionInfo.offerRegionMaintainTasks(offerPlan); + Assert.assertEquals(4, partitionInfo.getRegionMaintainEntryList().size()); + + partitionInfo.preDeleteDatabase( + new PreDeleteDatabasePlan(database, PreDeleteDatabasePlan.PreDeleteType.EXECUTE)); + partitionInfo.batchRemoveRegionCreateTasks(new BatchRemoveRegionCreateTasksPlan(database)); + Assert.assertEquals(1, partitionInfo.getRegionMaintainEntryList().size()); + Assert.assertEquals( + otherRegion.getRegionId(), partitionInfo.getRegionMaintainEntryList().get(0).getRegionId()); + + // Replaying the same consensus plan is idempotent, and a snapshot cannot revive removed tasks. + partitionInfo.batchRemoveRegionCreateTasks(new BatchRemoveRegionCreateTasksPlan(database)); + Assert.assertTrue(partitionInfo.processTakeSnapshot(snapshotDir)); + final PartitionInfo loaded = new PartitionInfo(); + loaded.processLoadSnapshot(snapshotDir); + Assert.assertEquals(1, loaded.getRegionMaintainEntryList().size()); + Assert.assertEquals( + otherRegion.getRegionId(), loaded.getRegionMaintainEntryList().get(0).getRegionId()); + } + + @Test + public void testCancelledTasksCannotAffectRecreatedDatabase() { + final String database = "root.sg"; + partitionInfo.createDatabase( + new DatabaseSchemaPlan( + ConfigPhysicalPlanType.CreateDatabase, new TDatabaseSchema(database))); + final TRegionReplicaSet oldRegion = + generateTRegionReplicaSet(0, new TConsensusGroupId(TConsensusGroupType.DataRegion, 0)); + final CreateRegionGroupsPlan oldCreatePlan = new CreateRegionGroupsPlan(); + oldCreatePlan.addRegionGroup(database, oldRegion); + partitionInfo.createRegionGroups(oldCreatePlan); + + final OfferRegionMaintainTasksPlan oldOfferPlan = new OfferRegionMaintainTasksPlan(); + oldOfferPlan.appendRegionMaintainTask( + new RegionCreateTask(oldRegion.getDataNodeLocations().get(0), database, oldRegion)); + partitionInfo.offerRegionMaintainTasks(oldOfferPlan); + Assert.assertEquals(1, partitionInfo.getRegionMaintainEntryList().size()); + + partitionInfo.preDeleteDatabase( + new PreDeleteDatabasePlan(database, PreDeleteDatabasePlan.PreDeleteType.EXECUTE)); + final BatchRemoveRegionCreateTasksPlan oldCancellation = + new BatchRemoveRegionCreateTasksPlan(database); + partitionInfo.batchRemoveRegionCreateTasks(oldCancellation); + Assert.assertTrue(partitionInfo.getRegionMaintainEntryList().isEmpty()); + + // A late offer from the old create procedure is rejected after PRE_DELETE. + partitionInfo.offerRegionMaintainTasks(oldOfferPlan); + Assert.assertTrue(partitionInfo.getRegionMaintainEntryList().isEmpty()); + + partitionInfo.deleteDatabase(new DeleteDatabasePlan(database)); + partitionInfo.createDatabase( + new DatabaseSchemaPlan( + ConfigPhysicalPlanType.CreateDatabase, new TDatabaseSchema(database))); + final TRegionReplicaSet newRegion = + generateTRegionReplicaSet(10, new TConsensusGroupId(TConsensusGroupType.DataRegion, 1)); + final CreateRegionGroupsPlan newCreatePlan = new CreateRegionGroupsPlan(); + newCreatePlan.addRegionGroup(database, newRegion); + partitionInfo.createRegionGroups(newCreatePlan); + final OfferRegionMaintainTasksPlan newOfferPlan = new OfferRegionMaintainTasksPlan(); + newOfferPlan.appendRegionMaintainTask( + new RegionCreateTask(newRegion.getDataNodeLocations().get(0), database, newRegion)); + partitionInfo.offerRegionMaintainTasks(newOfferPlan); + + // Replaying the old cancellation is a no-op because the same-name database is not pre-deleted. + // A late task offer from the old RegionGroup is rejected by Region ownership validation. + partitionInfo.batchRemoveRegionCreateTasks(oldCancellation); + partitionInfo.offerRegionMaintainTasks(oldOfferPlan); + Assert.assertEquals(1, partitionInfo.getRegionMaintainEntryList().size()); + Assert.assertEquals( + newRegion.getRegionId(), partitionInfo.getRegionMaintainEntryList().get(0).getRegionId()); + } + + @Test + public void testCreateRegionGroupsRejectsPreDeletedAndMissingDatabase() + throws DatabaseNotExistsException { + final String database = "root.lifecycle"; + partitionInfo.createDatabase( + new DatabaseSchemaPlan( + ConfigPhysicalPlanType.CreateDatabase, new TDatabaseSchema(database))); + + final CreateRegionGroupsPlan preDeletedPlan = new CreateRegionGroupsPlan(); + preDeletedPlan.addRegionGroup( + database, + generateTRegionReplicaSet(0, new TConsensusGroupId(TConsensusGroupType.DataRegion, 1))); + partitionInfo.preDeleteDatabase( + new PreDeleteDatabasePlan(database, PreDeleteDatabasePlan.PreDeleteType.EXECUTE)); + + TSStatus status = partitionInfo.createRegionGroups(preDeletedPlan); + Assert.assertEquals(TSStatusCode.DATABASE_NOT_EXIST.getStatusCode(), status.getCode()); + Assert.assertTrue( + partitionInfo.getAllReplicaSets(database, TConsensusGroupType.DataRegion).isEmpty()); + + final CreateRegionGroupsPlan missingPlan = new CreateRegionGroupsPlan(); + missingPlan.addRegionGroup( + "root.missing", + generateTRegionReplicaSet(10, new TConsensusGroupId(TConsensusGroupType.SchemaRegion, 2))); + status = partitionInfo.createRegionGroups(missingPlan); + Assert.assertEquals(TSStatusCode.DATABASE_NOT_EXIST.getStatusCode(), status.getCode()); + } + + @Test + public void testBatchedCreateRegionGroupsPlanIsValidatedAtomically() + throws DatabaseNotExistsException { + final String existingDatabase = "root.existing"; + partitionInfo.createDatabase( + new DatabaseSchemaPlan( + ConfigPhysicalPlanType.CreateDatabase, new TDatabaseSchema(existingDatabase))); + + final CreateRegionGroupsPlan batchedPlan = new CreateRegionGroupsPlan(); + batchedPlan.addRegionGroup( + existingDatabase, + generateTRegionReplicaSet(0, new TConsensusGroupId(TConsensusGroupType.DataRegion, 40))); + batchedPlan.addRegionGroup( + "root.missing", + generateTRegionReplicaSet(10, new TConsensusGroupId(TConsensusGroupType.DataRegion, 41))); + + final TSStatus status = partitionInfo.createRegionGroups(batchedPlan); + Assert.assertEquals(TSStatusCode.DATABASE_NOT_EXIST.getStatusCode(), status.getCode()); + Assert.assertEquals( + 0, partitionInfo.getRegionGroupCount(existingDatabase, TConsensusGroupType.DataRegion)); + Assert.assertEquals(42, partitionInfo.generateNextRegionGroupId()); + } + @Test public void testGetRegionType() { @@ -322,14 +509,15 @@ private OfferRegionMaintainTasksPlan generateOfferRegionMaintainTasksPlan() { dataNodeLocation.setMPPDataExchangeEndPoint(new TEndPoint("0.0.0.0", 10740)); dataNodeLocation.setDataRegionConsensusEndPoint(new TEndPoint("0.0.0.0", 10760)); dataNodeLocation.setSchemaRegionConsensusEndPoint(new TEndPoint("0.0.0.0", 10750)); + final TDataNodeLocation secondDataNodeLocation = dataNodeLocation.deepCopy().setDataNodeId(1); TRegionReplicaSet regionReplicaSet = new TRegionReplicaSet(); regionReplicaSet.setRegionId(new TConsensusGroupId(TConsensusGroupType.DataRegion, 0)); - regionReplicaSet.setDataNodeLocations(Collections.singletonList(dataNodeLocation)); + regionReplicaSet.setDataNodeLocations(List.of(dataNodeLocation, secondDataNodeLocation)); OfferRegionMaintainTasksPlan offerPlan = new OfferRegionMaintainTasksPlan(); offerPlan.appendRegionMaintainTask( - new RegionCreateTask(dataNodeLocation, "root.sg", regionReplicaSet)); + new RegionCreateTask(secondDataNodeLocation, "root.sg", regionReplicaSet)); offerPlan.appendRegionMaintainTask( new RegionCreateTask(dataNodeLocation, "root.sg", regionReplicaSet)); offerPlan.appendRegionMaintainTask( diff --git a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/impl/CreateRegionGroupsProcedureTest.java b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/impl/CreateRegionGroupsProcedureTest.java index 6269953736100..85358fbfe8d5f 100644 --- a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/impl/CreateRegionGroupsProcedureTest.java +++ b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/impl/CreateRegionGroupsProcedureTest.java @@ -24,20 +24,35 @@ import org.apache.iotdb.common.rpc.thrift.TDataNodeLocation; import org.apache.iotdb.common.rpc.thrift.TEndPoint; import org.apache.iotdb.common.rpc.thrift.TRegionReplicaSet; +import org.apache.iotdb.common.rpc.thrift.TSStatus; import org.apache.iotdb.confignode.consensus.request.write.region.CreateRegionGroupsPlan; +import org.apache.iotdb.confignode.manager.ConfigManager; +import org.apache.iotdb.confignode.manager.consensus.ConsensusManager; +import org.apache.iotdb.confignode.procedure.Procedure; +import org.apache.iotdb.confignode.procedure.env.ConfigNodeProcedureEnv; import org.apache.iotdb.confignode.procedure.impl.region.CreateRegionGroupsProcedure; +import org.apache.iotdb.confignode.procedure.impl.region.RemoveRegionGroupProcedure; +import org.apache.iotdb.confignode.procedure.impl.schema.DeleteDatabaseProcedure; +import org.apache.iotdb.confignode.procedure.scheduler.ProcedureScheduler; +import org.apache.iotdb.confignode.procedure.state.CreateRegionGroupsState; +import org.apache.iotdb.confignode.procedure.state.ProcedureLockState; import org.apache.iotdb.confignode.procedure.store.ProcedureFactory; import org.apache.iotdb.confignode.procedure.store.ProcedureType; +import org.apache.iotdb.confignode.rpc.thrift.TDatabaseSchema; +import org.apache.iotdb.rpc.TSStatusCode; import org.apache.tsfile.utils.PublicBAOS; import org.junit.Assert; import org.junit.Test; +import org.mockito.Mockito; import java.io.DataOutputStream; import java.io.IOException; import java.nio.ByteBuffer; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import static org.apache.iotdb.common.rpc.thrift.TConsensusGroupType.DataRegion; @@ -47,6 +62,72 @@ public class CreateRegionGroupsProcedureTest { + private static class TestCreateRegionGroupsProcedure extends CreateRegionGroupsProcedure { + + private final List> childProcedures = new ArrayList<>(); + + private TestCreateRegionGroupsProcedure() { + super(); + } + + private TestCreateRegionGroupsProcedure( + final TConsensusGroupType consensusGroupType, + final CreateRegionGroupsPlan createRegionGroupsPlan, + final CreateRegionGroupsPlan persistPlan, + final Map failedRegionReplicaSets) { + super(consensusGroupType, createRegionGroupsPlan, persistPlan, failedRegionReplicaSets); + } + + private void executeShunt(final ConfigNodeProcedureEnv env) { + executeFromState(env, CreateRegionGroupsState.SHUNT_REGION_REPLICAS); + } + + private void executeCreate(final ConfigNodeProcedureEnv env) { + executeFromState(env, CreateRegionGroupsState.CREATE_REGION_GROUPS); + } + + private void executePostPersist(final ConfigNodeProcedureEnv env) { + executeFromState(env, CreateRegionGroupsState.REBALANCE_DATA_PARTITION_POLICY); + } + + private void executeFinish(final ConfigNodeProcedureEnv env) { + executeFromState(env, CreateRegionGroupsState.CREATE_REGION_GROUPS_FINISH); + } + + private ProcedureLockState acquireDatabaseLock(final ConfigNodeProcedureEnv env) { + return acquireLock(env); + } + + private void releaseDatabaseLock(final ConfigNodeProcedureEnv env) { + releaseLock(env); + } + + @Override + protected void addChildProcedure(final Procedure childProcedure) { + super.addChildProcedure(childProcedure); + childProcedures.add(childProcedure); + } + + private List> getChildProcedures() { + return childProcedures; + } + } + + private static class TestDeleteDatabaseProcedure extends DeleteDatabaseProcedure { + + private TestDeleteDatabaseProcedure(final TDatabaseSchema databaseSchema) { + super(databaseSchema, false); + } + + private ProcedureLockState acquireDatabaseLock(final ConfigNodeProcedureEnv env) { + return acquireLock(env); + } + + private void releaseDatabaseLock(final ConfigNodeProcedureEnv env) { + releaseLock(env); + } + } + @Test public void serializeDeserializeTest() { TDataNodeLocation dataNodeLocation0 = new TDataNodeLocation(); @@ -111,6 +192,7 @@ public void serializeDeserializeTest() { ByteBuffer.wrap(byteArrayOutputStream.getBuf(), 0, byteArrayOutputStream.size()); Assert.assertEquals(ProcedureType.CREATE_REGION_GROUPS.getTypeCode(), buffer.getShort()); procedure1.deserialize(buffer); + Assert.assertFalse(buffer.hasRemaining()); assertEquals(procedure0, procedure1); assertEquals(procedure0.hashCode(), procedure1.hashCode()); @@ -124,4 +206,177 @@ public void serializeDeserializeTest() { fail(); } } + + @Test + public void testPersistFailureRetriesWithoutReleasingOwnership() { + final TDataNodeLocation createdDataNode = + new TDataNodeLocation().setDataNodeId(1).setInternalEndPoint(new TEndPoint("0.0.0.1", 1)); + final TDataNodeLocation failedDataNode = + new TDataNodeLocation().setDataNodeId(2).setInternalEndPoint(new TEndPoint("0.0.0.2", 2)); + final TDataNodeLocation otherFailedDataNode = + new TDataNodeLocation().setDataNodeId(3).setInternalEndPoint(new TEndPoint("0.0.0.3", 3)); + final TConsensusGroupId regionId = new TConsensusGroupId(DataRegion, 10); + final TConsensusGroupId otherRegionId = new TConsensusGroupId(DataRegion, 11); + final TRegionReplicaSet allocatedReplicaSet = + new TRegionReplicaSet(regionId, List.of(createdDataNode, failedDataNode)); + final TRegionReplicaSet failedReplicaSet = + new TRegionReplicaSet(regionId, Collections.singletonList(failedDataNode)); + final TRegionReplicaSet otherAllocatedReplicaSet = + new TRegionReplicaSet(otherRegionId, Collections.singletonList(otherFailedDataNode)); + + final CreateRegionGroupsPlan createPlan = new CreateRegionGroupsPlan(); + createPlan.addRegionGroup("root.sg", allocatedReplicaSet); + createPlan.addRegionGroup("root.sg", otherAllocatedReplicaSet); + final Map failedReplicaSets = new HashMap<>(); + failedReplicaSets.put(regionId, failedReplicaSet); + failedReplicaSets.put(otherRegionId, otherAllocatedReplicaSet); + final TestCreateRegionGroupsProcedure procedure = + new TestCreateRegionGroupsProcedure( + DataRegion, createPlan, new CreateRegionGroupsPlan(), failedReplicaSets); + + final ConfigNodeProcedureEnv env = Mockito.mock(ConfigNodeProcedureEnv.class); + Mockito.when(env.validateCreateRegionGroups(createPlan)) + .thenReturn(new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode())); + Mockito.when(env.persistRegionGroup(Mockito.any())) + .thenReturn(new TSStatus(TSStatusCode.DATABASE_NOT_EXIST.getStatusCode())); + + procedure.executeShunt(env); + + Assert.assertTrue(procedure.getChildProcedures().isEmpty()); + Assert.assertFalse(procedure.isFailed()); + Mockito.verify(env).persistRegionGroup(Mockito.any()); + } + + @Test + public void testRegionCreateTaskPersistenceRetriesIdempotently() throws Exception { + final TDataNodeLocation dataNode0 = new TDataNodeLocation().setDataNodeId(1); + final TDataNodeLocation dataNode1 = new TDataNodeLocation().setDataNodeId(2); + final TDataNodeLocation failedDataNode = new TDataNodeLocation().setDataNodeId(3); + final TConsensusGroupId regionId = new TConsensusGroupId(DataRegion, 10); + final TRegionReplicaSet allocatedReplicaSet = + new TRegionReplicaSet(regionId, List.of(dataNode0, dataNode1, failedDataNode)); + final TRegionReplicaSet failedReplicaSet = + new TRegionReplicaSet(regionId, Collections.singletonList(failedDataNode)); + final CreateRegionGroupsPlan createPlan = new CreateRegionGroupsPlan(); + createPlan.addRegionGroup("root.sg", allocatedReplicaSet); + final Map failedReplicaSets = new HashMap<>(); + failedReplicaSets.put(regionId, failedReplicaSet); + final TestCreateRegionGroupsProcedure procedure = + new TestCreateRegionGroupsProcedure( + DataRegion, createPlan, new CreateRegionGroupsPlan(), failedReplicaSets); + + final ConfigNodeProcedureEnv env = Mockito.mock(ConfigNodeProcedureEnv.class); + final ConfigManager configManager = Mockito.mock(ConfigManager.class); + final ConsensusManager consensusManager = Mockito.mock(ConsensusManager.class); + final TSStatus success = new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode()); + final TSStatus failure = new TSStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR.getStatusCode()); + Mockito.when(env.validateCreateRegionGroups(createPlan)).thenReturn(success); + Mockito.when(env.persistRegionGroup(Mockito.any())).thenReturn(success); + Mockito.when(env.getConfigManager()).thenReturn(configManager); + Mockito.when(configManager.getConsensusManager()).thenReturn(consensusManager); + Mockito.when(consensusManager.write(Mockito.any())).thenReturn(failure, success); + + procedure.executeShunt(env); + procedure.executeShunt(env); + + Assert.assertFalse(procedure.isFailed()); + Mockito.verify(env, Mockito.times(2)).persistRegionGroup(Mockito.any()); + Mockito.verify(consensusManager, Mockito.times(2)).write(Mockito.any()); + } + + @Test + public void testFencedBeforeCreateRpcDoesNotSubmitCleanup() { + final String database = "root.sg"; + final CreateRegionGroupsPlan createPlan = new CreateRegionGroupsPlan(); + createPlan.addRegionGroup( + database, + new TRegionReplicaSet(new TConsensusGroupId(DataRegion, 10), Collections.emptyList())); + final TestCreateRegionGroupsProcedure procedure = + new TestCreateRegionGroupsProcedure( + DataRegion, createPlan, new CreateRegionGroupsPlan(), Collections.emptyMap()); + + final ConfigNodeProcedureEnv env = Mockito.mock(ConfigNodeProcedureEnv.class); + Mockito.when(env.validateCreateRegionGroups(createPlan)) + .thenReturn(new TSStatus(TSStatusCode.DATABASE_CONFIG_ERROR.getStatusCode())); + procedure.executeCreate(env); + + Mockito.verify(env, Mockito.never()) + .doRegionCreation(Mockito.any(), Mockito.any(CreateRegionGroupsPlan.class)); + Mockito.verify(env, Mockito.never()).getConfigManager(); + Assert.assertTrue(procedure.isFailed()); + } + + @Test + public void testFencedAfterCreateRpcCleansEveryPlannedRegionReplica() { + final TRegionReplicaSet replicaSet = + new TRegionReplicaSet( + new TConsensusGroupId(DataRegion, 10), + Collections.singletonList(new TDataNodeLocation().setDataNodeId(1))); + final CreateRegionGroupsPlan createPlan = new CreateRegionGroupsPlan(); + createPlan.addRegionGroup("root.sg", replicaSet); + final TestCreateRegionGroupsProcedure procedure = + new TestCreateRegionGroupsProcedure( + DataRegion, createPlan, new CreateRegionGroupsPlan(), Collections.emptyMap()); + + final ConfigNodeProcedureEnv env = Mockito.mock(ConfigNodeProcedureEnv.class); + Mockito.when(env.validateCreateRegionGroups(createPlan)) + .thenReturn(new TSStatus(TSStatusCode.DATABASE_NOT_EXIST.getStatusCode())); + + procedure.executeShunt(env); + + Assert.assertEquals( + Collections.singletonList(new RemoveRegionGroupProcedure(replicaSet)), + procedure.getChildProcedures()); + Mockito.verify(env, Mockito.never()).persistRegionGroup(Mockito.any()); + Assert.assertFalse(procedure.isFailed()); + procedure.executeFinish(env); + Assert.assertTrue(procedure.isFailed()); + } + + @Test + public void testFencedAfterPersistenceDoesNotSubmitCleanup() { + final CreateRegionGroupsPlan createPlan = new CreateRegionGroupsPlan(); + createPlan.addRegionGroup( + "root.sg", + new TRegionReplicaSet(new TConsensusGroupId(DataRegion, 10), Collections.emptyList())); + final TestCreateRegionGroupsProcedure procedure = + new TestCreateRegionGroupsProcedure( + DataRegion, createPlan, createPlan, Collections.emptyMap()); + + final ConfigNodeProcedureEnv env = Mockito.mock(ConfigNodeProcedureEnv.class); + Mockito.when(env.validateCreateRegionGroups(createPlan)) + .thenReturn(new TSStatus(TSStatusCode.DATABASE_NOT_EXIST.getStatusCode())); + + procedure.executePostPersist(env); + + Mockito.verify(env, Mockito.never()).getConfigManager(); + Assert.assertTrue(procedure.isFailed()); + } + + @Test + public void testCreateAndDeleteDatabaseLifecycleAreMutuallyExclusive() { + final String database = "root.sg"; + final CreateRegionGroupsPlan createPlan = new CreateRegionGroupsPlan(); + createPlan.addRegionGroup( + database, + new TRegionReplicaSet(new TConsensusGroupId(DataRegion, 1), Collections.emptyList())); + final TestCreateRegionGroupsProcedure createProcedure = + new TestCreateRegionGroupsProcedure( + DataRegion, createPlan, new CreateRegionGroupsPlan(), Collections.emptyMap()); + createProcedure.setProcId(1); + final TestDeleteDatabaseProcedure deleteProcedure = + new TestDeleteDatabaseProcedure(new TDatabaseSchema(database)); + deleteProcedure.setProcId(2); + + final ConfigNodeProcedureEnv env = + new ConfigNodeProcedureEnv( + Mockito.mock(ConfigManager.class), Mockito.mock(ProcedureScheduler.class)); + Assert.assertEquals(ProcedureLockState.LOCK_ACQUIRED, createProcedure.acquireDatabaseLock(env)); + Assert.assertEquals( + ProcedureLockState.LOCK_EVENT_WAIT, deleteProcedure.acquireDatabaseLock(env)); + + createProcedure.releaseDatabaseLock(env); + Assert.assertEquals(ProcedureLockState.LOCK_ACQUIRED, deleteProcedure.acquireDatabaseLock(env)); + deleteProcedure.releaseDatabaseLock(env); + } } diff --git a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteDatabaseProcedureTest.java b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteDatabaseProcedureTest.java index b12f49d9bd7da..2050605df61e2 100644 --- a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteDatabaseProcedureTest.java +++ b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteDatabaseProcedureTest.java @@ -19,20 +19,65 @@ package org.apache.iotdb.confignode.procedure.impl.schema; +import org.apache.iotdb.common.rpc.thrift.TConsensusGroupId; +import org.apache.iotdb.common.rpc.thrift.TConsensusGroupType; +import org.apache.iotdb.common.rpc.thrift.TDataNodeLocation; +import org.apache.iotdb.common.rpc.thrift.TRegionReplicaSet; +import org.apache.iotdb.common.rpc.thrift.TSStatus; +import org.apache.iotdb.confignode.consensus.request.write.database.PreDeleteDatabasePlan; +import org.apache.iotdb.confignode.manager.ConfigManager; +import org.apache.iotdb.confignode.manager.load.LoadManager; +import org.apache.iotdb.confignode.procedure.Procedure; +import org.apache.iotdb.confignode.procedure.env.ConfigNodeProcedureEnv; +import org.apache.iotdb.confignode.procedure.impl.region.RemoveRegionGroupProcedure; +import org.apache.iotdb.confignode.procedure.state.schema.DeleteDatabaseState; import org.apache.iotdb.confignode.procedure.store.ProcedureFactory; import org.apache.iotdb.confignode.rpc.thrift.TDatabaseSchema; +import org.apache.iotdb.rpc.TSStatusCode; import org.apache.tsfile.utils.PublicBAOS; import org.junit.Test; +import org.mockito.Mockito; import java.io.DataOutputStream; import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.fail; public class DeleteDatabaseProcedureTest { + private static class TestDeleteDatabaseProcedure extends DeleteDatabaseProcedure { + private final List> childProcedures = new ArrayList<>(); + + private TestDeleteDatabaseProcedure(final TDatabaseSchema databaseSchema) { + super(databaseSchema, false); + } + + @Override + protected void addChildProcedure(final Procedure childProcedure) { + super.addChildProcedure(childProcedure); + childProcedures.add(childProcedure); + } + + private void executeRegionGroupDeletion(final ConfigNodeProcedureEnv env) + throws InterruptedException { + executeFromState(env, DeleteDatabaseState.DELETE_DATABASE_SCHEMA); + } + + private void executeOneStep(final ConfigNodeProcedureEnv env) throws InterruptedException { + execute(env); + } + + private DeleteDatabaseState currentState() { + return getCurrentState(); + } + } + @Test public void serializeDeserializeTest() { @@ -47,10 +92,55 @@ public void serializeDeserializeTest() { DeleteDatabaseProcedure p2 = (DeleteDatabaseProcedure) ProcedureFactory.getInstance().create(buffer); + assertFalse(buffer.hasRemaining()); assertEquals(p1, p2); } catch (Exception e) { fail(); } } + + @Test + public void testRegionGroupsAreDeletedByChildProcedures() throws InterruptedException { + final TRegionReplicaSet regionReplicaSet = + new TRegionReplicaSet( + new TConsensusGroupId(TConsensusGroupType.DataRegion, 1), + Collections.singletonList(new TDataNodeLocation().setDataNodeId(1))); + final TestDeleteDatabaseProcedure procedure = + new TestDeleteDatabaseProcedure(new TDatabaseSchema("root.sg")); + final ConfigNodeProcedureEnv env = Mockito.mock(ConfigNodeProcedureEnv.class); + final ConfigManager configManager = Mockito.mock(ConfigManager.class); + Mockito.when(env.getConfigManager()).thenReturn(configManager); + Mockito.when(configManager.getLoadManager()).thenReturn(Mockito.mock(LoadManager.class)); + Mockito.when(env.getAllReplicaSets("root.sg")) + .thenReturn(Collections.singletonList(regionReplicaSet)); + + procedure.executeRegionGroupDeletion(env); + + assertEquals( + Collections.singletonList(new RemoveRegionGroupProcedure(regionReplicaSet)), + procedure.childProcedures); + } + + @Test + public void testTransientConsensusFailuresRetryTheSameState() throws Exception { + final TestDeleteDatabaseProcedure procedure = + new TestDeleteDatabaseProcedure(new TDatabaseSchema("root.sg")); + final ConfigNodeProcedureEnv env = Mockito.mock(ConfigNodeProcedureEnv.class); + final TSStatus success = new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode()); + final TSStatus failure = new TSStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR.getStatusCode()); + Mockito.when(env.preDeleteDatabase(PreDeleteDatabasePlan.PreDeleteType.EXECUTE, "root.sg")) + .thenReturn(failure, success); + Mockito.when(env.invalidateCache("root.sg")).thenReturn(true); + Mockito.when(env.batchRemoveRegionCreateTasks("root.sg")).thenReturn(failure, success); + + procedure.executeOneStep(env); + assertEquals(DeleteDatabaseState.PRE_DELETE_DATABASE, procedure.currentState()); + procedure.executeOneStep(env); + assertEquals(DeleteDatabaseState.INVALIDATE_CACHE, procedure.currentState()); + procedure.executeOneStep(env); + assertEquals(DeleteDatabaseState.INVALIDATE_CACHE, procedure.currentState()); + procedure.executeOneStep(env); + assertEquals(DeleteDatabaseState.DELETE_DATABASE_SCHEMA, procedure.currentState()); + } } diff --git a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManagerTest.java b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManagerTest.java new file mode 100644 index 0000000000000..366c58553ee7a --- /dev/null +++ b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManagerTest.java @@ -0,0 +1,141 @@ +/* + * 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.iotdb.confignode.procedure.scheduler; + +import org.apache.iotdb.confignode.procedure.Procedure; +import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLifecycleLockManager.DatabaseLock; + +import org.junit.Assert; +import org.junit.Test; +import org.mockito.Mockito; + +import java.util.Collections; +import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +public class DatabaseLifecycleLockManagerTest { + + @Test + public void testRequestLocksAreScopedByDatabaseName() throws Exception { + final DatabaseLifecycleLockManager lockManager = + new DatabaseLifecycleLockManager(Mockito.mock(ProcedureScheduler.class)); + final ExecutorService executor = Executors.newFixedThreadPool(2); + final CountDownLatch sameDatabaseAcquired = new CountDownLatch(1); + try (final DatabaseLock ignored = lockManager.acquireLocks(Collections.singleton("root.sg"))) { + final Future sameDatabaseFuture = + executor.submit( + () -> { + try (final DatabaseLock sameDatabaseLock = + lockManager.acquireLocks(Collections.singleton("root.sg"))) { + sameDatabaseAcquired.countDown(); + } + }); + final Future otherDatabaseFuture = + executor.submit( + () -> { + try (final DatabaseLock otherDatabaseLock = + lockManager.acquireLocks(Collections.singleton("root.other"))) { + // Acquiring a different database proves the lock is not cluster-global. + } + }); + + otherDatabaseFuture.get(10, TimeUnit.SECONDS); + Assert.assertFalse(sameDatabaseAcquired.await(200, TimeUnit.MILLISECONDS)); + Assert.assertFalse(sameDatabaseFuture.isDone()); + } finally { + executor.shutdownNow(); + } + Assert.assertTrue(sameDatabaseAcquired.await(10, TimeUnit.SECONDS)); + } + + @Test + public void testProcedureAndRequestShareTheSameLock() throws Exception { + final ProcedureScheduler scheduler = Mockito.mock(ProcedureScheduler.class); + final DatabaseLifecycleLockManager lockManager = new DatabaseLifecycleLockManager(scheduler); + final Procedure owner = procedure(1); + final Procedure waiter = procedure(2); + final Set databases = Collections.singleton("root.sg"); + + Assert.assertNull(lockManager.tryLock(owner, databases)); + Assert.assertEquals("root.sg", lockManager.tryLock(waiter, databases)); + lockManager.waitProcedure(waiter, "root.sg"); + + final ExecutorService executor = Executors.newSingleThreadExecutor(); + final CountDownLatch requestAcquired = new CountDownLatch(1); + try { + executor.submit( + () -> { + try (final DatabaseLock ignored = lockManager.acquireLocks(databases)) { + requestAcquired.countDown(); + } + }); + Assert.assertFalse(requestAcquired.await(200, TimeUnit.MILLISECONDS)); + + lockManager.releaseLocks(owner, databases); + Mockito.verify(scheduler).addFront(waiter); + Assert.assertTrue(requestAcquired.await(10, TimeUnit.SECONDS)); + } finally { + executor.shutdownNow(); + } + } + + @Test + public void testFailedMultiDatabaseAcquisitionDoesNotLeakPartialLocks() { + final DatabaseLifecycleLockManager lockManager = + new DatabaseLifecycleLockManager(Mockito.mock(ProcedureScheduler.class)); + final Procedure firstProcedure = procedure(1); + final Procedure secondProcedure = procedure(2); + + Assert.assertNull(lockManager.tryLock(firstProcedure, Collections.singleton("root.b"))); + Assert.assertEquals("root.b", lockManager.tryLock(secondProcedure, Set.of("root.a", "root.b"))); + + try (final DatabaseLock ignored = lockManager.acquireLocks(Collections.singleton("root.a"))) { + // The partial root.a acquisition of secondProcedure must have been released. + } + lockManager.releaseLocks(firstProcedure, Collections.singleton("root.b")); + } + + @Test + public void testRequestLocksAreReentrantOnTheOwningThread() { + final DatabaseLifecycleLockManager lockManager = + new DatabaseLifecycleLockManager(Mockito.mock(ProcedureScheduler.class)); + final Procedure procedure = procedure(1); + final Set databases = Collections.singleton("root.sg"); + + try (final DatabaseLock outerLock = lockManager.acquireLocks(databases)) { + try (final DatabaseLock innerLock = lockManager.acquireLocks(databases)) { + Assert.assertEquals("root.sg", lockManager.tryLock(procedure, databases)); + } + Assert.assertEquals("root.sg", lockManager.tryLock(procedure, databases)); + } + Assert.assertNull(lockManager.tryLock(procedure, databases)); + lockManager.releaseLocks(procedure, databases); + } + + private static Procedure procedure(final long procedureId) { + final Procedure procedure = Mockito.mock(Procedure.class); + Mockito.when(procedure.getProcId()).thenReturn(procedureId); + return procedure; + } +} From 78a28c533488244abf51dc434487db5b17205350 Mon Sep 17 00:00:00 2001 From: Yongzao <532741407@qq.com> Date: Wed, 22 Jul 2026 22:44:26 +0800 Subject: [PATCH 2/5] [ConfigNode] Address lifecycle lock review feedback --- .../confignode/i18n/ManagerMessages.java | 2 - .../confignode/i18n/ManagerMessages.java | 2 - .../confignode/manager/ProcedureManager.java | 131 ++++++++---- .../manager/partition/PartitionManager.java | 118 ++++++----- .../persistence/partition/PartitionInfo.java | 23 +- .../impl/AbstractDatabaseProcedure.java | 6 +- .../impl/schema/DeleteDatabaseProcedure.java | 200 ++++++++++-------- .../DatabaseLifecycleLockManager.java | 99 +++++++-- .../DatabaseLifecycleLockManagerTest.java | 36 +++- 9 files changed, 379 insertions(+), 238 deletions(-) diff --git a/iotdb-core/confignode/src/main/i18n/en/org/apache/iotdb/confignode/i18n/ManagerMessages.java b/iotdb-core/confignode/src/main/i18n/en/org/apache/iotdb/confignode/i18n/ManagerMessages.java index 45361a98d8053..8eb35035b1754 100644 --- a/iotdb-core/confignode/src/main/i18n/en/org/apache/iotdb/confignode/i18n/ManagerMessages.java +++ b/iotdb-core/confignode/src/main/i18n/en/org/apache/iotdb/confignode/i18n/ManagerMessages.java @@ -522,8 +522,6 @@ public final class ManagerMessages { "Unexpected interruption during waiting for configNode leader ready."; public static final String UNEXPECTED_INTERRUPTION_DURING_WAITING_FOR_GET_CLUSTER_ID = "Unexpected interruption during waiting for get cluster id."; - public static final String UNEXPECTED_NON_CREATE_REGION_MAINTAIN_TASK_SKIPPED = - "Unexpected non-create task in the RegionMaintainer queue; skipping it (the queue only recreates region replicas now, and region deletion is handled by RemoveRegionGroupProcedure)."; public static final String UNEXPECTED_NULL_PROCEDURE_PARAMETERS_FOR_WAITINGPROCEDUREFINISHED = "Unexpected null procedure parameters for waitingProcedureFinished"; public static final String UNKNOWN_DATAPARTITION_ALLOCATION_STRATEGY_USING_INHERIT_STRATEGY_BY_DEFAULT = diff --git a/iotdb-core/confignode/src/main/i18n/zh/org/apache/iotdb/confignode/i18n/ManagerMessages.java b/iotdb-core/confignode/src/main/i18n/zh/org/apache/iotdb/confignode/i18n/ManagerMessages.java index 5ed644c0f6dab..44009361febe4 100644 --- a/iotdb-core/confignode/src/main/i18n/zh/org/apache/iotdb/confignode/i18n/ManagerMessages.java +++ b/iotdb-core/confignode/src/main/i18n/zh/org/apache/iotdb/confignode/i18n/ManagerMessages.java @@ -517,8 +517,6 @@ public final class ManagerMessages { "等待 configNode leader 就绪过程中发生意外中断。"; public static final String UNEXPECTED_INTERRUPTION_DURING_WAITING_FOR_GET_CLUSTER_ID = "等待获取 cluster id 过程中发生意外中断。"; - public static final String UNEXPECTED_NON_CREATE_REGION_MAINTAIN_TASK_SKIPPED = - "RegionMaintainer 队列中出现意外的非 create 任务；跳过处理（该队列目前仅用于重建 region 副本，region 删除由 RemoveRegionGroupProcedure 处理）。"; public static final String UNEXPECTED_NULL_PROCEDURE_PARAMETERS_FOR_WAITINGPROCEDUREFINISHED = "waitingProcedureFinished 的 procedure 参数为空"; public static final String UNKNOWN_DATAPARTITION_ALLOCATION_STRATEGY_USING_INHERIT_STRATEGY_BY_DEFAULT = diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java index c76615d1fc6d9..de0d311102e7a 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java @@ -193,6 +193,7 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; import java.util.function.BiFunction; import java.util.stream.Collectors; @@ -303,54 +304,96 @@ public TSStatus testSubProcedure() { public TSStatus deleteDatabases( final List deleteSgSchemaList, final boolean isGeneratedByPipe) { final List procedures = new ArrayList<>(); - final long startCheckTimeForProcedures = System.currentTimeMillis(); + final long deadlineNanos = + System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(PROCEDURE_WAIT_TIME_OUT); for (final TDatabaseSchema databaseSchema : deleteSgSchemaList) { - final String database = databaseSchema.getName(); - boolean hasOverlappedTask = false; - while (executor.isRunning() - && System.currentTimeMillis() - startCheckTimeForProcedures < PROCEDURE_WAIT_TIME_OUT) { - try (final DatabaseLock ignored = acquireDatabaseLifecycleLock(database)) { - synchronized (this) { - final Pair procedureIdDuplicatePair = - checkDuplicateTableTask( - database, null, null, null, null, ProcedureType.DELETE_DATABASE_PROCEDURE); - hasOverlappedTask = procedureIdDuplicatePair.getRight(); - - if (Boolean.FALSE.equals(procedureIdDuplicatePair.getRight())) { - final DeleteDatabaseProcedure procedure = - new DeleteDatabaseProcedure(databaseSchema, isGeneratedByPipe); - this.executor.submitProcedure(procedure); - procedures.add(procedure); - } - } - } - if (!hasOverlappedTask) { - break; - } - synchronized (this) { - try { - wait(PROCEDURE_WAIT_RETRY_TIMEOUT); - } catch (final InterruptedException e) { - Thread.currentThread().interrupt(); - } - } - } - if (hasOverlappedTask) { - return RpcUtils.getStatus( - TSStatusCode.OVERLAP_WITH_EXISTING_TASK, - String.format( - "Some other task is operating table under the database %s, please retry after the procedure finishes.", - database)); + if (!submitDeleteDatabaseProcedure( + databaseSchema, isGeneratedByPipe, procedures, deadlineNanos)) { + return getDeleteDatabaseOverlapStatus(databaseSchema.getName()); } } - List results = new ArrayList<>(procedures.size()); + + final List results = new ArrayList<>(procedures.size()); procedures.forEach(procedure -> results.add(waitingProcedureFinished(procedure))); if (results.stream() .allMatch(result -> result.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode())) { return StatusUtils.OK; - } else { - return RpcUtils.getStatus(results); } + return RpcUtils.getStatus(results); + } + + private boolean submitDeleteDatabaseProcedure( + final TDatabaseSchema databaseSchema, + final boolean isGeneratedByPipe, + final List procedures, + final long deadlineNanos) { + while (executor.isRunning()) { + final long remainingNanos = deadlineNanos - System.nanoTime(); + if (remainingNanos <= 0) { + return false; + } + + final DatabaseLock databaseLock; + try { + databaseLock = + env.getDatabaseLifecycleLockManager() + .tryAcquireLocks( + Collections.singleton(databaseSchema.getName()), + remainingNanos, + TimeUnit.NANOSECONDS); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + return false; + } + if (databaseLock == null) { + return false; + } + + try (databaseLock) { + synchronized (this) { + final Pair procedureIdDuplicatePair = + checkDuplicateTableTask( + databaseSchema.getName(), + null, + null, + null, + null, + ProcedureType.DELETE_DATABASE_PROCEDURE); + if (Boolean.FALSE.equals(procedureIdDuplicatePair.getRight())) { + final DeleteDatabaseProcedure procedure = + new DeleteDatabaseProcedure(databaseSchema, isGeneratedByPipe); + executor.submitProcedure(procedure); + procedures.add(procedure); + return true; + } + } + } + + if (!waitForDeleteDatabaseRetry()) { + return false; + } + } + return false; + } + + private boolean waitForDeleteDatabaseRetry() { + synchronized (this) { + try { + wait(PROCEDURE_WAIT_RETRY_TIMEOUT); + return true; + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + return false; + } + } + } + + private TSStatus getDeleteDatabaseOverlapStatus(final String database) { + return RpcUtils.getStatus( + TSStatusCode.OVERLAP_WITH_EXISTING_TASK, + String.format( + "Some other task is operating table under the database %s, please retry after the procedure finishes.", + database)); } public TSStatus alterEncodingCompressor( @@ -1553,10 +1596,10 @@ public boolean hasUnfinishedDatabaseLifecycleProcedure(final String database) { .filter(procedure -> !procedure.isFinished()) .anyMatch( procedure -> - (procedure instanceof DeleteDatabaseProcedure - && database.equals(((DeleteDatabaseProcedure) procedure).getDatabase())) - || (procedure instanceof CreateRegionGroupsProcedure - && ((CreateRegionGroupsProcedure) procedure).containsDatabase(database))); + (procedure instanceof DeleteDatabaseProcedure deleteProcedure + && database.equals(deleteProcedure.getDatabase())) + || (procedure instanceof CreateRegionGroupsProcedure createProcedure + && createProcedure.containsDatabase(database))); } /** diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/partition/PartitionManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/partition/PartitionManager.java index 95325f268c983..d7ab72cbf6afe 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/partition/PartitionManager.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/partition/PartitionManager.java @@ -86,7 +86,6 @@ import org.apache.iotdb.confignode.manager.schema.ClusterSchemaManager; import org.apache.iotdb.confignode.persistence.partition.PartitionInfo; import org.apache.iotdb.confignode.persistence.partition.maintainer.RegionCreateTask; -import org.apache.iotdb.confignode.persistence.partition.maintainer.RegionMaintainTask; import org.apache.iotdb.confignode.procedure.impl.partition.DataPartitionTableIntegrityCheckProcedure; import org.apache.iotdb.confignode.procedure.impl.region.CreateRegionGroupsProcedure; import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLifecycleLockManager.DatabaseLock; @@ -1415,7 +1414,7 @@ public void maintainRegionReplicas() { .map(RegionCreateTask::getStorageGroup) .distinct() .sorted() - .collect(Collectors.toList()); + .toList(); for (final String database : databases) { try (final DatabaseLock ignored = getProcedureManager().acquireDatabaseLifecycleLock(database)) { @@ -1430,16 +1429,15 @@ public void maintainRegionReplicas() { } private void maintainRegionReplicasUnderLock(final String database) { - final List persistedTasks = + final List persistedTasks = partitionInfo.getRegionMaintainEntryList().stream() .filter(RegionCreateTask.class::isInstance) .map(RegionCreateTask.class::cast) .filter(task -> database.equals(task.getStorageGroup())) - .collect(Collectors.toList()); + .toList(); final Map invalidTaskCountByRegion = new HashMap<>(); - for (RegionMaintainTask task : persistedTasks) { - if (!(task instanceof RegionCreateTask) - || !isRegionCreateTaskRegionValid((RegionCreateTask) task)) { + for (final RegionCreateTask task : persistedTasks) { + if (!isRegionCreateTaskRegionValid(task)) { invalidTaskCountByRegion.merge(task.getRegionId(), 1, Integer::sum); } } @@ -1458,21 +1456,10 @@ private void maintainRegionReplicasUnderLock(final String database) { // RegionCreateTasks now (delete tasks are filtered out at the PartitionInfo ingestion points), // and a region may carry several of them when more than one of its replicas failed to create. final Map> tasksByRegion = new LinkedHashMap<>(); - for (RegionMaintainTask task : persistedTasks) { - if (invalidRegionIds.contains(task.getRegionId())) { - continue; + for (final RegionCreateTask task : persistedTasks) { + if (!invalidRegionIds.contains(task.getRegionId())) { + tasksByRegion.computeIfAbsent(task.getRegionId(), ignored -> new LinkedList<>()).add(task); } - if (!(task instanceof RegionCreateTask)) { - // Unreachable: the queue only holds create tasks now (legacy delete tasks are dropped at - // the - // PartitionInfo ingestion points). Guard against a regression so an unexpected task type - // cannot silently stall the loop. - LOGGER.warn(ManagerMessages.UNEXPECTED_NON_CREATE_REGION_MAINTAIN_TASK_SKIPPED); - continue; - } - tasksByRegion - .computeIfAbsent(task.getRegionId(), k -> new LinkedList<>()) - .add((RegionCreateTask) task); } final Set invalidHeadRegionIds = new HashSet<>(); @@ -1480,25 +1467,9 @@ private void maintainRegionReplicasUnderLock(final String database) { new EnumMap<>(TConsensusGroupType.class); final Map> selectedCountByTypeAndDataNode = new EnumMap<>(TConsensusGroupType.class); - for (Queue queue : tasksByRegion.values()) { - final RegionCreateTask head = queue.peek(); - if (!isRegionCreateTaskTargetValid(head)) { - invalidHeadRegionIds.add(head.getRegionId()); - continue; - } - if (isRegionCreateTargetInBackoff(head)) { - continue; - } - final TConsensusGroupType type = head.getRegionId().getType(); - final int dataNodeId = head.getTargetDataNode().getDataNodeId(); - final Map selectedCountByDataNode = - selectedCountByTypeAndDataNode.computeIfAbsent(type, ignored -> new HashMap<>()); - final int selectedCount = selectedCountByDataNode.getOrDefault(dataNodeId, 0); - if (selectedCount >= getRegionCreateBatchSize(type)) { - continue; - } - selectedCountByDataNode.put(dataNodeId, selectedCount + 1); - headsByType.computeIfAbsent(type, ignored -> new ArrayList<>()).add(head); + for (final Queue queue : tasksByRegion.values()) { + selectRegionCreateTask( + queue.peek(), invalidHeadRegionIds, headsByType, selectedCountByTypeAndDataNode); } // A target-specific stale task only removes the head of its Region queue. A following task of @@ -1518,6 +1489,30 @@ private void maintainRegionReplicasUnderLock(final String database) { } } + private void selectRegionCreateTask( + final RegionCreateTask task, + final Set invalidRegionIds, + final Map> tasksByType, + final Map> selectedCountByTypeAndDataNode) { + if (!isRegionCreateTaskTargetValid(task)) { + invalidRegionIds.add(task.getRegionId()); + return; + } + if (isRegionCreateTargetInBackoff(task)) { + return; + } + + final TConsensusGroupType type = task.getRegionId().getType(); + final int dataNodeId = task.getTargetDataNode().getDataNodeId(); + final Map selectedCountByDataNode = + selectedCountByTypeAndDataNode.computeIfAbsent(type, ignored -> new HashMap<>()); + final int selectedCount = selectedCountByDataNode.getOrDefault(dataNodeId, 0); + if (selectedCount < getRegionCreateBatchSize(type)) { + selectedCountByDataNode.put(dataNodeId, selectedCount + 1); + tasksByType.computeIfAbsent(type, ignored -> new ArrayList<>()).add(task); + } + } + private boolean writeRegionCreateTaskPlan(ConfigPhysicalPlan plan) { try { return getConsensusManager().write(plan).getCode() @@ -1631,29 +1626,36 @@ private void collectSuccessfulRegions( tasksByDataNode .computeIfAbsent(task.getTargetDataNode().getDataNodeId(), ignored -> new ArrayList<>()) .add(task); - final TSStatus status = responseMap.get(task.getRegionId().getId()); - if (status != null && status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) { + if (isRegionCreateSuccessful(responseMap.get(task.getRegionId().getId()))) { successfulRegions.add(task.getRegionId()); } } - for (List dataNodeTasks : tasksByDataNode.values()) { - RegionCreateTask failedTask = null; - TSStatus failedStatus = null; - for (RegionCreateTask task : dataNodeTasks) { - final TSStatus status = responseMap.get(task.getRegionId().getId()); - if (status == null || status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) { - if (failedTask == null || isDirectMemoryFailure(status)) { - failedTask = task; - failedStatus = status; - } - } - } - if (failedTask != null) { - recordRegionCreateFailure(failedTask, failedStatus); - } else { - clearRegionCreateBackoff(dataNodeTasks.get(0)); + for (final List dataNodeTasks : tasksByDataNode.values()) { + updateRegionCreateBackoff(responseMap, dataNodeTasks); + } + } + + private void updateRegionCreateBackoff( + final Map responseMap, final List dataNodeTasks) { + RegionCreateTask failedTask = null; + TSStatus failedStatus = null; + for (final RegionCreateTask task : dataNodeTasks) { + final TSStatus status = responseMap.get(task.getRegionId().getId()); + if (!isRegionCreateSuccessful(status) + && (failedTask == null || isDirectMemoryFailure(status))) { + failedTask = task; + failedStatus = status; } } + if (failedTask == null) { + clearRegionCreateBackoff(dataNodeTasks.get(0)); + } else { + recordRegionCreateFailure(failedTask, failedStatus); + } + } + + private boolean isRegionCreateSuccessful(final TSStatus status) { + return status != null && status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode(); } private boolean isRegionCreateTargetInBackoff(RegionCreateTask task) { diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java index 47685a60fcfe5..54a0bb497ca15 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java @@ -203,9 +203,8 @@ public TSStatus createRegionGroups(CreateRegionGroupsPlan plan) { plan.getRegionGroupMap() .forEach( - (database, regionReplicaSets) -> { - databasePartitionTables.get(database).createRegionGroups(regionReplicaSets); - }); + (database, regionReplicaSets) -> + databasePartitionTables.get(database).createRegionGroups(regionReplicaSets)); return new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode()); } @@ -274,19 +273,16 @@ public TSStatus offerRegionMaintainTasks( // owned by RemoveRegionGroupProcedure. Drop any legacy DELETE task that an upgraded node may // replay from an old consensus log, so it cannot get stuck in the queue and block the // recreation of that region's other replicas. - for (RegionMaintainTask task : offerRegionMaintainTasksPlan.getRegionMaintainTaskList()) { + for (final RegionMaintainTask task : + offerRegionMaintainTasksPlan.getRegionMaintainTaskList()) { if (RegionMaintainType.DELETE.equals(task.getType())) { LOGGER.info( ConfigNodeMessages .MESSAGE_DROPPING_LEGACY_REGION_DELETE_TASK_FOR_ARG_WHILE_REPLAYING_OFFER_PLAN_REGION_DELETION_IS_NOW_HANDLED_BY_REMOVEREGIONGROUPPROCEDURE_2A81A649, task.getRegionId()); - continue; - } - final RegionCreateTask createTask = (RegionCreateTask) task; - if (!isRegionCreateTaskOwnedByCurrentPartitionTable(createTask)) { - continue; - } - if (existingTasks.add(task)) { + } else if (task instanceof RegionCreateTask createTask + && isRegionCreateTaskOwnedByCurrentPartitionTable(createTask) + && existingTasks.add(task)) { regionMaintainTaskList.add(task); } } @@ -343,9 +339,8 @@ public TSStatus batchRemoveRegionCreateTasks(BatchRemoveRegionCreateTasksPlan pl } regionMaintainTaskList.removeIf( task -> - task instanceof RegionCreateTask - && Objects.equals( - plan.getDatabase(), ((RegionCreateTask) task).getStorageGroup())); + task instanceof RegionCreateTask createTask + && Objects.equals(plan.getDatabase(), createTask.getStorageGroup())); return RpcUtils.SUCCESS_STATUS; } } diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/AbstractDatabaseProcedure.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/AbstractDatabaseProcedure.java index fc41b9afbc5a3..9d7bbaf91ff6d 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/AbstractDatabaseProcedure.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/AbstractDatabaseProcedure.java @@ -25,10 +25,10 @@ import java.util.Set; /** A procedure that holds exclusive lifecycle locks for its databases until it finishes. */ -public abstract class AbstractDatabaseProcedure - extends StateMachineProcedure { +public abstract class AbstractDatabaseProcedure + extends StateMachineProcedure { - private transient String waitingDatabase; + private String waitingDatabase; protected AbstractDatabaseProcedure() { super(); diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteDatabaseProcedure.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteDatabaseProcedure.java index c9240878c9dae..0a17bc61b2f5d 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteDatabaseProcedure.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/schema/DeleteDatabaseProcedure.java @@ -81,106 +81,19 @@ protected Flow executeFromState(final ConfigNodeProcedureEnv env, final DeleteDa try { switch (state) { case PRE_DELETE_DATABASE: - LOG.info( - ProcedureMessages.LOG_DELETEDATABASEPROCEDURE_PRE_DELETE_DATABASE_ARG_6A1FEACC, - deleteDatabaseSchema.getName()); - final TSStatus preDeleteStatus = - env.preDeleteDatabase( - PreDeleteDatabasePlan.PreDeleteType.EXECUTE, deleteDatabaseSchema.getName()); - if (preDeleteStatus.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) { - setNextState(DeleteDatabaseState.INVALIDATE_CACHE); - } else if (getCycles() > RETRY_THRESHOLD) { - setFailure( - new ProcedureException( - ProcedureMessages.DELETEDATABASEPROCEDURE_DELETE_DATABASESCHEMA_FAILED)); - } else { - setNextState(DeleteDatabaseState.PRE_DELETE_DATABASE); - } + executePreDeleteDatabase(env); break; case INVALIDATE_CACHE: - LOG.info( - ProcedureMessages.LOG_DELETEDATABASEPROCEDURE_INVALIDATE_CACHE_DATABASE_ARG_299FC9BC, - deleteDatabaseSchema.getName()); - if (env.invalidateCache(deleteDatabaseSchema.getName())) { - final TSStatus removeTasksStatus = - env.batchRemoveRegionCreateTasks(deleteDatabaseSchema.getName()); - if (removeTasksStatus.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) { - setNextState(DeleteDatabaseState.DELETE_DATABASE_SCHEMA); - } else if (getCycles() > RETRY_THRESHOLD) { - setFailure( - new ProcedureException( - ProcedureMessages.DELETEDATABASEPROCEDURE_DELETE_DATABASESCHEMA_FAILED)); - } else { - setNextState(DeleteDatabaseState.INVALIDATE_CACHE); - } - } else { - setFailure( - new ProcedureException( - ProcedureMessages.DELETEDATABASEPROCEDURE_INVALIDATE_CACHE_FAILED)); - } + executeInvalidateCache(env); break; case DELETE_DATABASE_SCHEMA: - LOG.info( - ProcedureMessages.LOG_DELETEDATABASEPROCEDURE_DELETE_DATABASESCHEMA_ARG_A49A47AC, - deleteDatabaseSchema.getName()); - - // Delete every RegionGroup as a child procedure. This procedure keeps the database lock - // while the children run, so neither same-name recreation nor a delayed Region creation - // can overtake cleanup. Each child carries its own replica-set copy and survives leader - // change or restart. - // - // Submission is intentionally NOT guarded by isStateDeserialized(): the executor persists - // a procedure at a state BEFORE that state's body has run (it advances the state on the - // previous cycle, then may stop at the inter-state boundary on a leader switch — see - // ProcedureExecutor#executeProcedure). So a recovery that lands on this state means the - // submission has NOT happened yet; skipping it would drop every region group's cleanup - // while the next state still drops the partition table, orphaning the region peers/data - // on disk with no record of where they live. Adding the children again on recovery is - // safe because RegionGroup deletion is idempotent. - final List regionReplicaSets = - env.getAllReplicaSets(deleteDatabaseSchema.getName()); - regionReplicaSets.forEach( - regionReplicaSet -> { - // Clear heartbeat cache along the way - env.getConfigManager() - .getLoadManager() - .removeRegionGroupRelatedCache(regionReplicaSet.getRegionId()); - addChildProcedure(new RemoveRegionGroupProcedure(regionReplicaSet)); - }); - setNextState(DeleteDatabaseState.DELETE_DATABASE_CONFIG); + executeDeleteDatabaseSchema(env); break; case DELETE_DATABASE_CONFIG: - env.getConfigManager() - .getLoadManager() - .clearDataPartitionPolicyTable(deleteDatabaseSchema.getName()); - LOG.info( - ProcedureMessages - .LOG_DELETEDATABASEPROCEDURE_DATA_PARTITION_POLICY_TABLE_DATABASE_ARG_CLEARED_7A32E28A, - deleteDatabaseSchema.getName()); - - // Delete Database metrics - PartitionMetrics.unbindDatabaseRelatedMetricsWhenUpdate( - MetricService.getInstance(), deleteDatabaseSchema.getName()); - PartitionMetrics.unbindDatabaseTableMetrics( - MetricService.getInstance(), deleteDatabaseSchema.getName()); - - // Delete DatabasePartitionTable - final TSStatus deleteConfigResult = - env.deleteDatabaseConfig(deleteDatabaseSchema.getName(), isGeneratedByPipe); - - if (deleteConfigResult.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) { - LOG.info( - ProcedureMessages - .LOG_DELETEDATABASEPROCEDURE_DATABASE_ARG_DELETED_SUCCESSFULLY_3A4E9202, - deleteDatabaseSchema.getName()); + if (executeDeleteDatabaseConfig(env)) { return Flow.NO_MORE_STATE; - } else if (getCycles() > RETRY_THRESHOLD) { - setFailure( - new ProcedureException( - ProcedureMessages.DELETEDATABASEPROCEDURE_DELETE_DATABASESCHEMA_FAILED)); - } else { - setNextState(DeleteDatabaseState.DELETE_DATABASE_CONFIG); } + break; } } catch (final TException | IOException e) { if (isRollbackSupported(state)) { @@ -209,6 +122,109 @@ protected Flow executeFromState(final ConfigNodeProcedureEnv env, final DeleteDa return Flow.HAS_MORE_STATE; } + private void executePreDeleteDatabase(final ConfigNodeProcedureEnv env) { + LOG.info( + ProcedureMessages.LOG_DELETEDATABASEPROCEDURE_PRE_DELETE_DATABASE_ARG_6A1FEACC, + deleteDatabaseSchema.getName()); + final TSStatus preDeleteStatus = + env.preDeleteDatabase( + PreDeleteDatabasePlan.PreDeleteType.EXECUTE, deleteDatabaseSchema.getName()); + if (preDeleteStatus.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) { + setNextState(DeleteDatabaseState.INVALIDATE_CACHE); + } else { + retryOrFail(DeleteDatabaseState.PRE_DELETE_DATABASE); + } + } + + private void executeInvalidateCache(final ConfigNodeProcedureEnv env) + throws IOException, TException { + LOG.info( + ProcedureMessages.LOG_DELETEDATABASEPROCEDURE_INVALIDATE_CACHE_DATABASE_ARG_299FC9BC, + deleteDatabaseSchema.getName()); + if (!env.invalidateCache(deleteDatabaseSchema.getName())) { + setFailure( + new ProcedureException( + ProcedureMessages.DELETEDATABASEPROCEDURE_INVALIDATE_CACHE_FAILED)); + return; + } + + final TSStatus removeTasksStatus = + env.batchRemoveRegionCreateTasks(deleteDatabaseSchema.getName()); + if (removeTasksStatus.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) { + setNextState(DeleteDatabaseState.DELETE_DATABASE_SCHEMA); + } else { + retryOrFail(DeleteDatabaseState.INVALIDATE_CACHE); + } + } + + private void executeDeleteDatabaseSchema(final ConfigNodeProcedureEnv env) { + LOG.info( + ProcedureMessages.LOG_DELETEDATABASEPROCEDURE_DELETE_DATABASESCHEMA_ARG_A49A47AC, + deleteDatabaseSchema.getName()); + + // Delete every RegionGroup as a child procedure. This procedure keeps the database lock while + // the children run, so neither same-name recreation nor a delayed Region creation can overtake + // cleanup. Each child carries its own replica-set copy and survives leader change or restart. + // + // Submission is intentionally NOT guarded by isStateDeserialized(): the executor persists a + // procedure at a state BEFORE that state's body has run (it advances the state on the previous + // cycle, then may stop at the inter-state boundary on a leader switch — see + // ProcedureExecutor#executeProcedure). So a recovery that lands on this state means the + // submission has NOT happened yet; skipping it would drop every region group's cleanup while + // the next state still drops the partition table, orphaning the region peers/data on disk with + // no record of where they live. Adding the children again on recovery is safe because + // RegionGroup deletion is idempotent. + final List regionReplicaSets = + env.getAllReplicaSets(deleteDatabaseSchema.getName()); + regionReplicaSets.forEach( + regionReplicaSet -> { + // Clear heartbeat cache along the way + env.getConfigManager() + .getLoadManager() + .removeRegionGroupRelatedCache(regionReplicaSet.getRegionId()); + addChildProcedure(new RemoveRegionGroupProcedure(regionReplicaSet)); + }); + setNextState(DeleteDatabaseState.DELETE_DATABASE_CONFIG); + } + + private boolean executeDeleteDatabaseConfig(final ConfigNodeProcedureEnv env) { + env.getConfigManager() + .getLoadManager() + .clearDataPartitionPolicyTable(deleteDatabaseSchema.getName()); + LOG.info( + ProcedureMessages + .LOG_DELETEDATABASEPROCEDURE_DATA_PARTITION_POLICY_TABLE_DATABASE_ARG_CLEARED_7A32E28A, + deleteDatabaseSchema.getName()); + + // Delete Database metrics + PartitionMetrics.unbindDatabaseRelatedMetricsWhenUpdate( + MetricService.getInstance(), deleteDatabaseSchema.getName()); + PartitionMetrics.unbindDatabaseTableMetrics( + MetricService.getInstance(), deleteDatabaseSchema.getName()); + + // Delete DatabasePartitionTable + final TSStatus deleteConfigResult = + env.deleteDatabaseConfig(deleteDatabaseSchema.getName(), isGeneratedByPipe); + if (deleteConfigResult.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) { + LOG.info( + ProcedureMessages.LOG_DELETEDATABASEPROCEDURE_DATABASE_ARG_DELETED_SUCCESSFULLY_3A4E9202, + deleteDatabaseSchema.getName()); + return true; + } + retryOrFail(DeleteDatabaseState.DELETE_DATABASE_CONFIG); + return false; + } + + private void retryOrFail(final DeleteDatabaseState state) { + if (getCycles() > RETRY_THRESHOLD) { + setFailure( + new ProcedureException( + ProcedureMessages.DELETEDATABASEPROCEDURE_DELETE_DATABASESCHEMA_FAILED)); + } else { + setNextState(state); + } + } + @Override protected void rollbackState(final ConfigNodeProcedureEnv env, final DeleteDatabaseState state) throws IOException, InterruptedException { diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManager.java index 31f75950eb43d..54e5786872f87 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManager.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManager.java @@ -28,6 +28,7 @@ import java.util.Map; import java.util.Set; import java.util.TreeSet; +import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.ReentrantLock; @@ -59,12 +60,34 @@ public DatabaseLock acquireLocks(final Set databaseNames) { while (!canAcquireRequestLocks(owner, orderedDatabases)) { lockReleased.awaitUninterruptibly(); } - orderedDatabases.forEach( - database -> - lockStateMap - .computeIfAbsent(database, ignored -> new DatabaseLockState()) - .acquireRequestLock(owner)); - return new DatabaseLock(this, orderedDatabases, owner); + return acquireRequestLocks(orderedDatabases, owner); + } finally { + stateLock.unlock(); + } + } + + /** + * Tries to acquire database locks for a synchronous manager request within the given timeout. + * + * @return the acquired lock, or null if the timeout elapsed before every lock became available + */ + public DatabaseLock tryAcquireLocks( + final Set databaseNames, final long timeout, final TimeUnit timeUnit) + throws InterruptedException { + final List orderedDatabases = orderedDatabases(databaseNames); + final Thread owner = Thread.currentThread(); + long remainingNanos = timeUnit.toNanos(timeout); + if (!stateLock.tryLock(remainingNanos, TimeUnit.NANOSECONDS)) { + return null; + } + try { + while (!canAcquireRequestLocks(owner, orderedDatabases)) { + if (remainingNanos <= 0) { + return null; + } + remainingNanos = lockReleased.awaitNanos(remainingNanos); + } + return acquireRequestLocks(orderedDatabases, owner); } finally { stateLock.unlock(); } @@ -78,11 +101,13 @@ public DatabaseLock acquireLocks(final Set databaseNames) { public String tryLock(final Procedure procedure, final Set databaseNames) { stateLock.lock(); try { + final List orderedDatabases = orderedDatabases(databaseNames); final List acquiredDatabases = new ArrayList<>(); - for (final String database : orderedDatabases(databaseNames)) { + for (final String database : orderedDatabases) { final DatabaseLockState lockState = lockStateMap.computeIfAbsent(database, ignored -> new DatabaseLockState()); - if (!lockState.canAcquireProcedureLock(procedure)) { + final boolean hasWaiterPriority = lockState.isHeadWaiter(procedure); + if (!lockState.canAcquireProcedureLock(procedure, hasWaiterPriority)) { acquiredDatabases.forEach( acquiredDatabase -> releaseProcedureLock(procedure, acquiredDatabase)); return database; @@ -90,6 +115,9 @@ public String tryLock(final Procedure procedure, final Set databaseNa if (lockState.acquireProcedureLock(procedure)) { acquiredDatabases.add(database); } + if (hasWaiterPriority) { + lockState.removeHeadWaiter(procedure); + } } return null; } finally { @@ -102,7 +130,7 @@ public void waitProcedure(final Procedure procedure, final String databaseNam try { final DatabaseLockState lockState = lockStateMap.computeIfAbsent(databaseName, ignored -> new DatabaseLockState()); - if (lockState.isUnlocked()) { + if (lockState.isUnlocked() && !lockState.hasWaitingProcedures()) { scheduler.addFront(procedure); removeIfIdle(databaseName, lockState); } else { @@ -133,6 +161,16 @@ private boolean canAcquireRequestLocks(final Thread owner, final List or return true; } + private DatabaseLock acquireRequestLocks( + final List orderedDatabases, final Thread owner) { + orderedDatabases.forEach( + database -> + lockStateMap + .computeIfAbsent(database, ignored -> new DatabaseLockState()) + .acquireRequestLock(owner)); + return new DatabaseLock(this, orderedDatabases, owner); + } + private void releaseRequestLocks(final List orderedDatabases, final Thread requestOwner) { stateLock.lock(); try { @@ -157,8 +195,9 @@ private void releaseProcedureLock(final Procedure procedure, final String dat } private void wakeWaiters(final DatabaseLockState lockState) { - lockState.wakeWaitingProcedures(scheduler); - lockReleased.signalAll(); + if (!lockState.wakeNextWaitingProcedure(scheduler)) { + lockReleased.signalAll(); + } } private void removeIfIdle(final String database, final DatabaseLockState lockState) { @@ -214,6 +253,8 @@ private void acquireRequestLock(final Thread owner) { } /** + * Releases one request lock hold. + * * @return true when the lock became fully released */ private boolean releaseRequestLock(final Thread owner) { @@ -228,12 +269,16 @@ private boolean releaseRequestLock(final Thread owner) { return false; } - private boolean canAcquireProcedureLock(final Procedure procedure) { + private boolean canAcquireProcedureLock( + final Procedure procedure, final boolean hasWaiterPriority) { return requestOwner == null - && (procedureOwner == null || procedureOwner.getProcId() == procedure.getProcId()); + && (procedureOwner == null || procedureOwner.getProcId() == procedure.getProcId()) + && (hasWaiterPriority || waitingProcedures.isEmpty()); } /** + * Acquires the procedure lock when it is not already held by the same procedure. + * * @return true when this invocation newly acquired the lock */ private boolean acquireProcedureLock(final Procedure procedure) { @@ -245,6 +290,8 @@ private boolean acquireProcedureLock(final Procedure procedure) { } /** + * Releases the procedure lock. + * * @return true when the lock was released */ private boolean releaseProcedureLock(final Procedure procedure) { @@ -262,10 +309,30 @@ private void waitProcedure(final Procedure procedure) { } } - private void wakeWaitingProcedures(final ProcedureScheduler procedureScheduler) { - while (!waitingProcedures.isEmpty()) { - procedureScheduler.addFront(waitingProcedures.pollFirst()); + private boolean removeHeadWaiter(final Procedure procedure) { + if (isHeadWaiter(procedure)) { + waitingProcedures.pollFirst(); + return true; + } + return false; + } + + private boolean isHeadWaiter(final Procedure procedure) { + return !waitingProcedures.isEmpty() + && waitingProcedures.peekFirst().getProcId() == procedure.getProcId(); + } + + private boolean wakeNextWaitingProcedure(final ProcedureScheduler procedureScheduler) { + final Procedure waitingProcedure = waitingProcedures.peekFirst(); + if (waitingProcedure == null) { + return false; } + procedureScheduler.addFront(waitingProcedure); + return true; + } + + private boolean hasWaitingProcedures() { + return !waitingProcedures.isEmpty(); } private boolean isUnlocked() { diff --git a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManagerTest.java b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManagerTest.java index 366c58553ee7a..e30f4cde7909b 100644 --- a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManagerTest.java +++ b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManagerTest.java @@ -70,7 +70,7 @@ public void testRequestLocksAreScopedByDatabaseName() throws Exception { } @Test - public void testProcedureAndRequestShareTheSameLock() throws Exception { + public void testWaitingProcedureCannotBeOvertakenByRequest() throws Exception { final ProcedureScheduler scheduler = Mockito.mock(ProcedureScheduler.class); final DatabaseLifecycleLockManager lockManager = new DatabaseLifecycleLockManager(scheduler); final Procedure owner = procedure(1); @@ -84,22 +84,44 @@ public void testProcedureAndRequestShareTheSameLock() throws Exception { final ExecutorService executor = Executors.newSingleThreadExecutor(); final CountDownLatch requestAcquired = new CountDownLatch(1); try { - executor.submit( - () -> { - try (final DatabaseLock ignored = lockManager.acquireLocks(databases)) { - requestAcquired.countDown(); - } - }); + final Future requestFuture = + executor.submit( + () -> { + try (final DatabaseLock ignored = lockManager.acquireLocks(databases)) { + requestAcquired.countDown(); + } + }); Assert.assertFalse(requestAcquired.await(200, TimeUnit.MILLISECONDS)); lockManager.releaseLocks(owner, databases); Mockito.verify(scheduler).addFront(waiter); + Assert.assertFalse(requestAcquired.await(200, TimeUnit.MILLISECONDS)); + + Assert.assertNull(lockManager.tryLock(waiter, databases)); + Assert.assertFalse(requestAcquired.await(200, TimeUnit.MILLISECONDS)); + lockManager.releaseLocks(waiter, databases); Assert.assertTrue(requestAcquired.await(10, TimeUnit.SECONDS)); + requestFuture.get(10, TimeUnit.SECONDS); } finally { executor.shutdownNow(); } } + @Test + public void testTimedRequestLockAcquisitionHonorsTimeout() throws Exception { + final DatabaseLifecycleLockManager lockManager = + new DatabaseLifecycleLockManager(Mockito.mock(ProcedureScheduler.class)); + final Procedure owner = procedure(1); + final Set databases = Collections.singleton("root.sg"); + Assert.assertNull(lockManager.tryLock(owner, databases)); + + final long startNanos = System.nanoTime(); + Assert.assertNull(lockManager.tryAcquireLocks(databases, 100, TimeUnit.MILLISECONDS)); + Assert.assertTrue(TimeUnit.NANOSECONDS.toSeconds(System.nanoTime() - startNanos) < 5); + + lockManager.releaseLocks(owner, databases); + } + @Test public void testFailedMultiDatabaseAcquisitionDoesNotLeakPartialLocks() { final DatabaseLifecycleLockManager lockManager = From 8a6ce6be03f27c52acee80b646e588658c2c5664 Mon Sep 17 00:00:00 2001 From: Yongzao <532741407@qq.com> Date: Wed, 22 Jul 2026 23:04:10 +0800 Subject: [PATCH 3/5] [ConfigNode] Address remaining Sonar findings --- .../iotdb/confignode/manager/ProcedureManager.java | 14 ++++++-------- .../scheduler/DatabaseLifecycleLockManager.java | 1 + 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java index de0d311102e7a..08bd6c77a0ba7 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java @@ -377,14 +377,12 @@ private boolean submitDeleteDatabaseProcedure( } private boolean waitForDeleteDatabaseRetry() { - synchronized (this) { - try { - wait(PROCEDURE_WAIT_RETRY_TIMEOUT); - return true; - } catch (final InterruptedException e) { - Thread.currentThread().interrupt(); - return false; - } + try { + TimeUnit.MILLISECONDS.sleep(PROCEDURE_WAIT_RETRY_TIMEOUT); + return true; + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + return false; } } diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManager.java index 54e5786872f87..57e0cb2e0e142 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManager.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManager.java @@ -70,6 +70,7 @@ public DatabaseLock acquireLocks(final Set databaseNames) { * Tries to acquire database locks for a synchronous manager request within the given timeout. * * @return the acquired lock, or null if the timeout elapsed before every lock became available + * @throws InterruptedException if interrupted while waiting for the locks */ public DatabaseLock tryAcquireLocks( final Set databaseNames, final long timeout, final TimeUnit timeUnit) From 7255c45847328e59dded46be6138ad591b410ca2 Mon Sep 17 00:00:00 2001 From: Yongzao <532741407@qq.com> Date: Wed, 22 Jul 2026 23:10:48 +0800 Subject: [PATCH 4/5] [ConfigNode] Clean up Sonar line length findings --- .../persistence/partition/PartitionInfo.java | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java index 54a0bb497ca15..585bf0d070c6a 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java @@ -107,6 +107,9 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; +import static org.apache.iotdb.confignode.i18n.ConfigNodeMessages.MESSAGE_CREATE_REGIONGROUPS_FAILED_BECAUSE_DATABASE_ARG_DOES_NOT_EXIST_AF0F2440; +import static org.apache.iotdb.confignode.i18n.ConfigNodeMessages.MESSAGE_CREATE_REGIONGROUPS_FAILED_BECAUSE_DATABASE_ARG_IS_BEING_DELETED_651DB780; + /** * The {@link PartitionInfo} stores cluster PartitionTable. * @@ -218,24 +221,22 @@ public TSStatus validateCreateRegionGroups(final CreateRegionGroupsPlan plan) { ConfigNodeMessages .LOG_REJECT_CREATEREGIONGROUPSPLAN_BECAUSE_DATABASE_ARG_DOES_NOT_EXIST_616E0CDE, database); - return new TSStatus(TSStatusCode.DATABASE_NOT_EXIST.getStatusCode()) - .setMessage( - String.format( - ConfigNodeMessages - .MESSAGE_CREATE_REGIONGROUPS_FAILED_BECAUSE_DATABASE_ARG_DOES_NOT_EXIST_AF0F2440, - database)); + final String message = + String.format( + MESSAGE_CREATE_REGIONGROUPS_FAILED_BECAUSE_DATABASE_ARG_DOES_NOT_EXIST_AF0F2440, + database); + return new TSStatus(TSStatusCode.DATABASE_NOT_EXIST.getStatusCode()).setMessage(message); } if (!databasePartitionTable.isNotPreDeleted()) { LOGGER.warn( ConfigNodeMessages .LOG_REJECT_CREATEREGIONGROUPSPLAN_BECAUSE_DATABASE_ARG_IS_BEING_DELETED_C085AC01, database); - return new TSStatus(TSStatusCode.DATABASE_NOT_EXIST.getStatusCode()) - .setMessage( - String.format( - ConfigNodeMessages - .MESSAGE_CREATE_REGIONGROUPS_FAILED_BECAUSE_DATABASE_ARG_IS_BEING_DELETED_651DB780, - database)); + final String message = + String.format( + MESSAGE_CREATE_REGIONGROUPS_FAILED_BECAUSE_DATABASE_ARG_IS_BEING_DELETED_651DB780, + database); + return new TSStatus(TSStatusCode.DATABASE_NOT_EXIST.getStatusCode()).setMessage(message); } } From be7e3610a1995dfaa6cba6f771a7b16c5f7efca4 Mon Sep 17 00:00:00 2001 From: Yongzao <532741407@qq.com> Date: Thu, 23 Jul 2026 11:13:43 +0800 Subject: [PATCH 5/5] [ConfigNode] Split region group creation by database --- .../confignode/manager/ProcedureManager.java | 6 +- .../manager/partition/PartitionManager.java | 39 +++-- .../manager/schema/ClusterSchemaManager.java | 2 +- .../procedure/env/ConfigNodeProcedureEnv.java | 16 +-- ...ockManager.java => DatabaseLockQueue.java} | 14 +- .../manager/ClusterSchemaManagerTest.java | 11 +- .../partition/PartitionManagerTest.java | 136 ++++++++++++++++++ ...erTest.java => DatabaseLockQueueTest.java} | 68 ++++----- 8 files changed, 218 insertions(+), 74 deletions(-) rename iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/{DatabaseLifecycleLockManager.java => DatabaseLockQueue.java} (96%) create mode 100644 iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/partition/PartitionManagerTest.java rename iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/scheduler/{DatabaseLifecycleLockManagerTest.java => DatabaseLockQueueTest.java} (65%) diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java index 08bd6c77a0ba7..8735bff7cc0b6 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java @@ -134,7 +134,7 @@ import org.apache.iotdb.confignode.procedure.impl.testonly.CreateManyDatabasesProcedure; import org.apache.iotdb.confignode.procedure.impl.trigger.CreateTriggerProcedure; import org.apache.iotdb.confignode.procedure.impl.trigger.DropTriggerProcedure; -import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLifecycleLockManager.DatabaseLock; +import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLockQueue.DatabaseLock; import org.apache.iotdb.confignode.procedure.scheduler.ProcedureScheduler; import org.apache.iotdb.confignode.procedure.scheduler.SimpleProcedureScheduler; import org.apache.iotdb.confignode.procedure.store.ConfigProcedureStore; @@ -336,7 +336,7 @@ private boolean submitDeleteDatabaseProcedure( final DatabaseLock databaseLock; try { databaseLock = - env.getDatabaseLifecycleLockManager() + env.getDatabaseLockQueue() .tryAcquireLocks( Collections.singleton(databaseSchema.getName()), remainingNanos, @@ -1586,7 +1586,7 @@ public DatabaseLock acquireDatabaseLifecycleLock(final String database) { } public DatabaseLock acquireDatabaseLifecycleLocks(final Set databases) { - return env.getDatabaseLifecycleLockManager().acquireLocks(databases); + return env.getDatabaseLockQueue().acquireLocks(databases); } public boolean hasUnfinishedDatabaseLifecycleProcedure(final String database) { diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/partition/PartitionManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/partition/PartitionManager.java index d7ab72cbf6afe..8f92464e2f913 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/partition/PartitionManager.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/partition/PartitionManager.java @@ -88,7 +88,7 @@ import org.apache.iotdb.confignode.persistence.partition.maintainer.RegionCreateTask; import org.apache.iotdb.confignode.procedure.impl.partition.DataPartitionTableIntegrityCheckProcedure; import org.apache.iotdb.confignode.procedure.impl.region.CreateRegionGroupsProcedure; -import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLifecycleLockManager.DatabaseLock; +import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLockQueue.DatabaseLock; import org.apache.iotdb.confignode.rpc.thrift.TCountTimeSlotListReq; import org.apache.iotdb.confignode.rpc.thrift.TGetRegionGroupsByTimeReq; import org.apache.iotdb.confignode.rpc.thrift.TGetRegionIdReq; @@ -123,6 +123,7 @@ import java.util.Queue; import java.util.Set; import java.util.StringJoiner; +import java.util.TreeMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; @@ -749,29 +750,39 @@ private TSStatus autoExtendRegionGroupIfNecessary( return generateAndAllocateRegionGroups(allotmentMap, consensusGroupType); } - private TSStatus generateAndAllocateRegionGroups( + TSStatus generateAndAllocateRegionGroups( final Map allotmentMap, final TConsensusGroupType consensusGroupType) throws NotEnoughDataNodeException, DatabaseNotExistsException { - if (!allotmentMap.isEmpty()) { - final CreateRegionGroupsProcedure procedure; - // Cover both ID allocation and submission with the same per-database locks used by database - // creation and deletion. A delayed plan therefore cannot become invisible between deleting - // and recreating a database with the same name. + final List procedures = new ArrayList<>(allotmentMap.size()); + for (final Map.Entry entry : new TreeMap<>(allotmentMap).entrySet()) { + final String database = entry.getKey(); + // Cover both ID allocation and submission with the same database lock used by database + // creation and deletion. Submit every database before waiting so each Procedure can make + // progress independently of slow or deleting databases. try (final DatabaseLock ignored = - getProcedureManager().acquireDatabaseLifecycleLocks(allotmentMap.keySet())) { + getProcedureManager().acquireDatabaseLifecycleLock(database)) { final CreateRegionGroupsPlan createRegionGroupsPlan = - getLoadManager().allocateRegionGroups(allotmentMap, consensusGroupType); + getLoadManager() + .allocateRegionGroups( + Collections.singletonMap(database, entry.getValue()), consensusGroupType); LOGGER.info( ManagerMessages.CREATEREGIONGROUPS_STARTING_TO_CREATE_THE_FOLLOWING_REGIONGROUPS); createRegionGroupsPlan.planLog(LOGGER); - procedure = + procedures.add( getProcedureManager() - .submitCreateRegionGroups(consensusGroupType, createRegionGroupsPlan); + .submitCreateRegionGroups(consensusGroupType, createRegionGroupsPlan)); } - return getProcedureManager().waitCreateRegionGroups(procedure); - } else { - return RpcUtils.SUCCESS_STATUS; } + + TSStatus result = RpcUtils.SUCCESS_STATUS; + for (final CreateRegionGroupsProcedure procedure : procedures) { + final TSStatus status = getProcedureManager().waitCreateRegionGroups(procedure); + if (result.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode() + && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) { + result = status; + } + } + return result; } /** diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/schema/ClusterSchemaManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/schema/ClusterSchemaManager.java index d850c6d4f3482..21e33a3e08029 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/schema/ClusterSchemaManager.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/schema/ClusterSchemaManager.java @@ -100,7 +100,7 @@ import org.apache.iotdb.confignode.manager.partition.RegionGroupExtensionPolicy; import org.apache.iotdb.confignode.persistence.schema.ClusterSchemaInfo; import org.apache.iotdb.confignode.persistence.schema.ConfigSchemaStatistics; -import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLifecycleLockManager.DatabaseLock; +import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLockQueue.DatabaseLock; import org.apache.iotdb.confignode.rpc.thrift.TDatabaseInfo; import org.apache.iotdb.confignode.rpc.thrift.TDatabaseSchema; import org.apache.iotdb.confignode.rpc.thrift.TDescTable4InformationSchemaResp; diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/ConfigNodeProcedureEnv.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/ConfigNodeProcedureEnv.java index 2124761521c75..3ff7a7f856804 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/ConfigNodeProcedureEnv.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/ConfigNodeProcedureEnv.java @@ -56,7 +56,7 @@ import org.apache.iotdb.confignode.procedure.Procedure; import org.apache.iotdb.confignode.procedure.exception.ProcedureException; import org.apache.iotdb.confignode.procedure.impl.schema.SchemaUtils; -import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLifecycleLockManager; +import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLockQueue; import org.apache.iotdb.confignode.procedure.scheduler.LockQueue; import org.apache.iotdb.confignode.procedure.scheduler.ProcedureScheduler; import org.apache.iotdb.confignode.rpc.thrift.TAddConsensusGroupReq; @@ -117,7 +117,7 @@ public class ConfigNodeProcedureEnv { /** Add or remove node lock. */ private final LockQueue nodeLock = new LockQueue(); - private final DatabaseLifecycleLockManager databaseLifecycleLockManager; + private final DatabaseLockQueue databaseLockQueue; private final ReentrantLock schedulerLock = new ReentrantLock(true); @@ -136,7 +136,7 @@ public class ConfigNodeProcedureEnv { public ConfigNodeProcedureEnv(ConfigManager configManager, ProcedureScheduler scheduler) { this.configManager = configManager; this.scheduler = scheduler; - this.databaseLifecycleLockManager = new DatabaseLifecycleLockManager(scheduler); + this.databaseLockQueue = new DatabaseLockQueue(scheduler); this.regionMaintainHandler = new RegionMaintainHandler(configManager); this.removeDataNodeHandler = new RemoveDataNodeHandler(configManager); this.removeConfigNodeLock = new ReentrantLock(); @@ -1154,19 +1154,19 @@ public LockQueue getNodeLock() { * @return the first database whose lock is unavailable, or null when all locks are acquired */ public String tryLockDatabases(final Procedure procedure, final Set databaseNames) { - return databaseLifecycleLockManager.tryLock(procedure, databaseNames); + return databaseLockQueue.tryLock(procedure, databaseNames); } public void waitDatabaseLock(final Procedure procedure, final String databaseName) { - databaseLifecycleLockManager.waitProcedure(procedure, databaseName); + databaseLockQueue.waitProcedure(procedure, databaseName); } public void releaseDatabaseLocks(final Procedure procedure, final Set databaseNames) { - databaseLifecycleLockManager.releaseLocks(procedure, databaseNames); + databaseLockQueue.releaseLocks(procedure, databaseNames); } - public DatabaseLifecycleLockManager getDatabaseLifecycleLockManager() { - return databaseLifecycleLockManager; + public DatabaseLockQueue getDatabaseLockQueue() { + return databaseLockQueue; } public ProcedureScheduler getScheduler() { diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLockQueue.java similarity index 96% rename from iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManager.java rename to iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLockQueue.java index 57e0cb2e0e142..78ce645e2fbe5 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManager.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLockQueue.java @@ -40,14 +40,14 @@ * database deletion cannot bypass one another. Procedure ownership is identified by procedure id * rather than worker thread because a procedure can resume on a different executor thread. */ -public class DatabaseLifecycleLockManager { +public class DatabaseLockQueue { private final ProcedureScheduler scheduler; private final ReentrantLock stateLock = new ReentrantLock(true); private final Condition lockReleased = stateLock.newCondition(); private final Map lockStateMap = new HashMap<>(); - public DatabaseLifecycleLockManager(final ProcedureScheduler scheduler) { + public DatabaseLockQueue(final ProcedureScheduler scheduler) { this.scheduler = scheduler; } @@ -212,16 +212,14 @@ private static List orderedDatabases(final Set databaseNames) { } public static final class DatabaseLock implements AutoCloseable { - private final DatabaseLifecycleLockManager lockManager; + private final DatabaseLockQueue lockQueue; private final List databaseNames; private final Thread owner; private boolean closed; private DatabaseLock( - final DatabaseLifecycleLockManager lockManager, - final List databaseNames, - final Thread owner) { - this.lockManager = lockManager; + final DatabaseLockQueue lockQueue, final List databaseNames, final Thread owner) { + this.lockQueue = lockQueue; this.databaseNames = databaseNames; this.owner = owner; } @@ -230,7 +228,7 @@ private DatabaseLock( public void close() { if (!closed) { closed = true; - lockManager.releaseRequestLocks(databaseNames, owner); + lockQueue.releaseRequestLocks(databaseNames, owner); } } } diff --git a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/ClusterSchemaManagerTest.java b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/ClusterSchemaManagerTest.java index db40647f2377b..5cb999687a520 100644 --- a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/ClusterSchemaManagerTest.java +++ b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/ClusterSchemaManagerTest.java @@ -24,8 +24,8 @@ import org.apache.iotdb.confignode.manager.schema.ClusterSchemaManager; import org.apache.iotdb.confignode.manager.schema.ClusterSchemaQuotaStatistics; import org.apache.iotdb.confignode.persistence.schema.ClusterSchemaInfo; -import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLifecycleLockManager; -import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLifecycleLockManager.DatabaseLock; +import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLockQueue; +import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLockQueue.DatabaseLock; import org.apache.iotdb.confignode.procedure.scheduler.SimpleProcedureScheduler; import org.apache.iotdb.confignode.rpc.thrift.TDatabaseSchema; import org.apache.iotdb.rpc.TSStatusCode; @@ -62,8 +62,7 @@ public void testSetDatabaseWaitsForLifecycleAdmissionBeforeCheckingProcedures() final String database = "root.sg"; final IManager configManager = Mockito.mock(IManager.class); final ProcedureManager procedureManager = Mockito.mock(ProcedureManager.class); - final DatabaseLifecycleLockManager lockManager = - new DatabaseLifecycleLockManager(new SimpleProcedureScheduler()); + final DatabaseLockQueue lockQueue = new DatabaseLockQueue(new SimpleProcedureScheduler()); final CountDownLatch admissionAttempted = new CountDownLatch(1); final AtomicBoolean unfinishedProcedure = new AtomicBoolean(false); Mockito.when(configManager.getProcedureManager()).thenReturn(procedureManager); @@ -71,7 +70,7 @@ public void testSetDatabaseWaitsForLifecycleAdmissionBeforeCheckingProcedures() .thenAnswer( ignored -> { admissionAttempted.countDown(); - return lockManager.acquireLocks(Collections.singleton(database)); + return lockQueue.acquireLocks(Collections.singleton(database)); }); Mockito.when(procedureManager.hasUnfinishedDatabaseLifecycleProcedure(database)) .thenAnswer(ignored -> unfinishedProcedure.get()); @@ -87,7 +86,7 @@ public void testSetDatabaseWaitsForLifecycleAdmissionBeforeCheckingProcedures() final ExecutorService executor = Executors.newSingleThreadExecutor(); final Future statusFuture; - try (final DatabaseLock ignored = lockManager.acquireLocks(Collections.singleton(database))) { + try (final DatabaseLock ignored = lockQueue.acquireLocks(Collections.singleton(database))) { statusFuture = executor.submit(() -> schemaManager.setDatabase(plan, false)); Assert.assertTrue(admissionAttempted.await(10, TimeUnit.SECONDS)); unfinishedProcedure.set(true); diff --git a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/partition/PartitionManagerTest.java b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/partition/PartitionManagerTest.java new file mode 100644 index 0000000000000..a4c2fd2ee9ec7 --- /dev/null +++ b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/manager/partition/PartitionManagerTest.java @@ -0,0 +1,136 @@ +/* + * 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.iotdb.confignode.manager.partition; + +import org.apache.iotdb.common.rpc.thrift.TConsensusGroupId; +import org.apache.iotdb.common.rpc.thrift.TConsensusGroupType; +import org.apache.iotdb.common.rpc.thrift.TRegionReplicaSet; +import org.apache.iotdb.common.rpc.thrift.TSStatus; +import org.apache.iotdb.confignode.consensus.request.write.region.CreateRegionGroupsPlan; +import org.apache.iotdb.confignode.manager.IManager; +import org.apache.iotdb.confignode.manager.ProcedureManager; +import org.apache.iotdb.confignode.manager.load.LoadManager; +import org.apache.iotdb.confignode.persistence.partition.PartitionInfo; +import org.apache.iotdb.confignode.procedure.impl.region.CreateRegionGroupsProcedure; +import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLockQueue; +import org.apache.iotdb.confignode.procedure.scheduler.ProcedureScheduler; +import org.apache.iotdb.rpc.RpcUtils; +import org.apache.iotdb.rpc.TSStatusCode; + +import org.junit.Assert; +import org.junit.Test; +import org.mockito.InOrder; +import org.mockito.Mockito; + +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; + +public class PartitionManagerTest { + + @Test + public void testCreateRegionGroupsProceduresAreSubmittedPerDatabaseBeforeWaiting() + throws Exception { + final IManager configManager = Mockito.mock(IManager.class); + final LoadManager loadManager = Mockito.mock(LoadManager.class); + final ProcedureManager procedureManager = Mockito.mock(ProcedureManager.class); + final DatabaseLockQueue databaseLockQueue = + new DatabaseLockQueue(Mockito.mock(ProcedureScheduler.class)); + Mockito.when(configManager.getLoadManager()).thenReturn(loadManager); + Mockito.when(configManager.getProcedureManager()).thenReturn(procedureManager); + Mockito.when(procedureManager.acquireDatabaseLifecycleLock(Mockito.anyString())) + .thenAnswer( + invocation -> + databaseLockQueue.acquireLocks( + Collections.singleton((String) invocation.getArgument(0)))); + + final CreateRegionGroupsPlan databaseAPlan = new CreateRegionGroupsPlan(); + final CreateRegionGroupsPlan databaseBPlan = new CreateRegionGroupsPlan(); + databaseAPlan.addRegionGroup( + "root.a", + new TRegionReplicaSet( + new TConsensusGroupId(TConsensusGroupType.DataRegion, 1), Collections.emptyList())); + databaseBPlan.addRegionGroup( + "root.b", + new TRegionReplicaSet( + new TConsensusGroupId(TConsensusGroupType.DataRegion, 2), Collections.emptyList())); + final CreateRegionGroupsProcedure databaseAProcedure = + Mockito.mock(CreateRegionGroupsProcedure.class); + final CreateRegionGroupsProcedure databaseBProcedure = + Mockito.mock(CreateRegionGroupsProcedure.class); + Mockito.when( + loadManager.allocateRegionGroups( + Collections.singletonMap("root.a", 1), TConsensusGroupType.DataRegion)) + .thenReturn(databaseAPlan); + Mockito.when( + loadManager.allocateRegionGroups( + Collections.singletonMap("root.b", 2), TConsensusGroupType.DataRegion)) + .thenReturn(databaseBPlan); + Mockito.when( + procedureManager.submitCreateRegionGroups( + TConsensusGroupType.DataRegion, databaseAPlan)) + .thenReturn(databaseAProcedure); + Mockito.when( + procedureManager.submitCreateRegionGroups( + TConsensusGroupType.DataRegion, databaseBPlan)) + .thenReturn(databaseBProcedure); + Mockito.when(procedureManager.waitCreateRegionGroups(databaseAProcedure)) + .thenReturn(new TSStatus(TSStatusCode.CREATE_REGION_ERROR.getStatusCode())); + Mockito.when(procedureManager.waitCreateRegionGroups(databaseBProcedure)) + .thenReturn(RpcUtils.SUCCESS_STATUS); + + final PartitionManager partitionManager = + new PartitionManager(configManager, Mockito.mock(PartitionInfo.class)); + try { + final Map allotmentMap = new LinkedHashMap<>(); + allotmentMap.put("root.b", 2); + allotmentMap.put("root.a", 1); + + final TSStatus status = + partitionManager.generateAndAllocateRegionGroups( + allotmentMap, TConsensusGroupType.DataRegion); + + Assert.assertEquals(TSStatusCode.CREATE_REGION_ERROR.getStatusCode(), status.getCode()); + final InOrder inOrder = Mockito.inOrder(loadManager, procedureManager); + inOrder.verify(procedureManager).acquireDatabaseLifecycleLock("root.a"); + inOrder + .verify(loadManager) + .allocateRegionGroups( + Collections.singletonMap("root.a", 1), TConsensusGroupType.DataRegion); + inOrder + .verify(procedureManager) + .submitCreateRegionGroups(TConsensusGroupType.DataRegion, databaseAPlan); + inOrder.verify(procedureManager).acquireDatabaseLifecycleLock("root.b"); + inOrder + .verify(loadManager) + .allocateRegionGroups( + Collections.singletonMap("root.b", 2), TConsensusGroupType.DataRegion); + inOrder + .verify(procedureManager) + .submitCreateRegionGroups(TConsensusGroupType.DataRegion, databaseBPlan); + inOrder.verify(procedureManager).waitCreateRegionGroups(databaseAProcedure); + inOrder.verify(procedureManager).waitCreateRegionGroups(databaseBProcedure); + Mockito.verify(procedureManager, Mockito.never()) + .acquireDatabaseLifecycleLocks(Mockito.anySet()); + } finally { + partitionManager.getRegionMaintainer().shutdownNow(); + } + } +} diff --git a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManagerTest.java b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLockQueueTest.java similarity index 65% rename from iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManagerTest.java rename to iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLockQueueTest.java index e30f4cde7909b..09990f1961033 100644 --- a/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLifecycleLockManagerTest.java +++ b/iotdb-core/confignode/src/test/java/org/apache/iotdb/confignode/procedure/scheduler/DatabaseLockQueueTest.java @@ -20,7 +20,7 @@ package org.apache.iotdb.confignode.procedure.scheduler; import org.apache.iotdb.confignode.procedure.Procedure; -import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLifecycleLockManager.DatabaseLock; +import org.apache.iotdb.confignode.procedure.scheduler.DatabaseLockQueue.DatabaseLock; import org.junit.Assert; import org.junit.Test; @@ -34,20 +34,20 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; -public class DatabaseLifecycleLockManagerTest { +public class DatabaseLockQueueTest { @Test public void testRequestLocksAreScopedByDatabaseName() throws Exception { - final DatabaseLifecycleLockManager lockManager = - new DatabaseLifecycleLockManager(Mockito.mock(ProcedureScheduler.class)); + final DatabaseLockQueue lockQueue = + new DatabaseLockQueue(Mockito.mock(ProcedureScheduler.class)); final ExecutorService executor = Executors.newFixedThreadPool(2); final CountDownLatch sameDatabaseAcquired = new CountDownLatch(1); - try (final DatabaseLock ignored = lockManager.acquireLocks(Collections.singleton("root.sg"))) { + try (final DatabaseLock ignored = lockQueue.acquireLocks(Collections.singleton("root.sg"))) { final Future sameDatabaseFuture = executor.submit( () -> { try (final DatabaseLock sameDatabaseLock = - lockManager.acquireLocks(Collections.singleton("root.sg"))) { + lockQueue.acquireLocks(Collections.singleton("root.sg"))) { sameDatabaseAcquired.countDown(); } }); @@ -55,7 +55,7 @@ public void testRequestLocksAreScopedByDatabaseName() throws Exception { executor.submit( () -> { try (final DatabaseLock otherDatabaseLock = - lockManager.acquireLocks(Collections.singleton("root.other"))) { + lockQueue.acquireLocks(Collections.singleton("root.other"))) { // Acquiring a different database proves the lock is not cluster-global. } }); @@ -72,14 +72,14 @@ public void testRequestLocksAreScopedByDatabaseName() throws Exception { @Test public void testWaitingProcedureCannotBeOvertakenByRequest() throws Exception { final ProcedureScheduler scheduler = Mockito.mock(ProcedureScheduler.class); - final DatabaseLifecycleLockManager lockManager = new DatabaseLifecycleLockManager(scheduler); + final DatabaseLockQueue lockQueue = new DatabaseLockQueue(scheduler); final Procedure owner = procedure(1); final Procedure waiter = procedure(2); final Set databases = Collections.singleton("root.sg"); - Assert.assertNull(lockManager.tryLock(owner, databases)); - Assert.assertEquals("root.sg", lockManager.tryLock(waiter, databases)); - lockManager.waitProcedure(waiter, "root.sg"); + Assert.assertNull(lockQueue.tryLock(owner, databases)); + Assert.assertEquals("root.sg", lockQueue.tryLock(waiter, databases)); + lockQueue.waitProcedure(waiter, "root.sg"); final ExecutorService executor = Executors.newSingleThreadExecutor(); final CountDownLatch requestAcquired = new CountDownLatch(1); @@ -87,19 +87,19 @@ public void testWaitingProcedureCannotBeOvertakenByRequest() throws Exception { final Future requestFuture = executor.submit( () -> { - try (final DatabaseLock ignored = lockManager.acquireLocks(databases)) { + try (final DatabaseLock ignored = lockQueue.acquireLocks(databases)) { requestAcquired.countDown(); } }); Assert.assertFalse(requestAcquired.await(200, TimeUnit.MILLISECONDS)); - lockManager.releaseLocks(owner, databases); + lockQueue.releaseLocks(owner, databases); Mockito.verify(scheduler).addFront(waiter); Assert.assertFalse(requestAcquired.await(200, TimeUnit.MILLISECONDS)); - Assert.assertNull(lockManager.tryLock(waiter, databases)); + Assert.assertNull(lockQueue.tryLock(waiter, databases)); Assert.assertFalse(requestAcquired.await(200, TimeUnit.MILLISECONDS)); - lockManager.releaseLocks(waiter, databases); + lockQueue.releaseLocks(waiter, databases); Assert.assertTrue(requestAcquired.await(10, TimeUnit.SECONDS)); requestFuture.get(10, TimeUnit.SECONDS); } finally { @@ -109,50 +109,50 @@ public void testWaitingProcedureCannotBeOvertakenByRequest() throws Exception { @Test public void testTimedRequestLockAcquisitionHonorsTimeout() throws Exception { - final DatabaseLifecycleLockManager lockManager = - new DatabaseLifecycleLockManager(Mockito.mock(ProcedureScheduler.class)); + final DatabaseLockQueue lockQueue = + new DatabaseLockQueue(Mockito.mock(ProcedureScheduler.class)); final Procedure owner = procedure(1); final Set databases = Collections.singleton("root.sg"); - Assert.assertNull(lockManager.tryLock(owner, databases)); + Assert.assertNull(lockQueue.tryLock(owner, databases)); final long startNanos = System.nanoTime(); - Assert.assertNull(lockManager.tryAcquireLocks(databases, 100, TimeUnit.MILLISECONDS)); + Assert.assertNull(lockQueue.tryAcquireLocks(databases, 100, TimeUnit.MILLISECONDS)); Assert.assertTrue(TimeUnit.NANOSECONDS.toSeconds(System.nanoTime() - startNanos) < 5); - lockManager.releaseLocks(owner, databases); + lockQueue.releaseLocks(owner, databases); } @Test public void testFailedMultiDatabaseAcquisitionDoesNotLeakPartialLocks() { - final DatabaseLifecycleLockManager lockManager = - new DatabaseLifecycleLockManager(Mockito.mock(ProcedureScheduler.class)); + final DatabaseLockQueue lockQueue = + new DatabaseLockQueue(Mockito.mock(ProcedureScheduler.class)); final Procedure firstProcedure = procedure(1); final Procedure secondProcedure = procedure(2); - Assert.assertNull(lockManager.tryLock(firstProcedure, Collections.singleton("root.b"))); - Assert.assertEquals("root.b", lockManager.tryLock(secondProcedure, Set.of("root.a", "root.b"))); + Assert.assertNull(lockQueue.tryLock(firstProcedure, Collections.singleton("root.b"))); + Assert.assertEquals("root.b", lockQueue.tryLock(secondProcedure, Set.of("root.a", "root.b"))); - try (final DatabaseLock ignored = lockManager.acquireLocks(Collections.singleton("root.a"))) { + try (final DatabaseLock ignored = lockQueue.acquireLocks(Collections.singleton("root.a"))) { // The partial root.a acquisition of secondProcedure must have been released. } - lockManager.releaseLocks(firstProcedure, Collections.singleton("root.b")); + lockQueue.releaseLocks(firstProcedure, Collections.singleton("root.b")); } @Test public void testRequestLocksAreReentrantOnTheOwningThread() { - final DatabaseLifecycleLockManager lockManager = - new DatabaseLifecycleLockManager(Mockito.mock(ProcedureScheduler.class)); + final DatabaseLockQueue lockQueue = + new DatabaseLockQueue(Mockito.mock(ProcedureScheduler.class)); final Procedure procedure = procedure(1); final Set databases = Collections.singleton("root.sg"); - try (final DatabaseLock outerLock = lockManager.acquireLocks(databases)) { - try (final DatabaseLock innerLock = lockManager.acquireLocks(databases)) { - Assert.assertEquals("root.sg", lockManager.tryLock(procedure, databases)); + try (final DatabaseLock outerLock = lockQueue.acquireLocks(databases)) { + try (final DatabaseLock innerLock = lockQueue.acquireLocks(databases)) { + Assert.assertEquals("root.sg", lockQueue.tryLock(procedure, databases)); } - Assert.assertEquals("root.sg", lockManager.tryLock(procedure, databases)); + Assert.assertEquals("root.sg", lockQueue.tryLock(procedure, databases)); } - Assert.assertNull(lockManager.tryLock(procedure, databases)); - lockManager.releaseLocks(procedure, databases); + Assert.assertNull(lockQueue.tryLock(procedure, databases)); + lockQueue.releaseLocks(procedure, databases); } private static Procedure procedure(final long procedureId) {