Copilot commented on code in PR #18273:
URL: https://github.com/apache/iotdb/pull/18273#discussion_r3627697539
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java:
##########
@@ -192,37 +201,97 @@ 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));
+ }
+
+ final long expectedGeneration = plan.getDatabaseGeneration(database);
+ final long currentGeneration =
databasePartitionTable.getDatabaseGeneration();
+ if (plan.isDatabaseGenerationSet(database) && expectedGeneration !=
currentGeneration) {
+ LOGGER.warn(
Review Comment:
The lifecycle generation fence is currently only enforced when the plan
explicitly contains a generation entry
(`plan.isDatabaseGenerationSet(database)`). For legacy / partially-deserialized
`CreateRegionGroupsPlan` blobs (no generation map), this condition is false, so
a stale plan could still be accepted and mutate a later database incarnation
with the same name, which defeats the race fix described in the PR.
Since `getDatabaseGeneration()` already defaults to
`DATABASE_GENERATION_NOT_SET` when absent, you can safely enforce the fence
unconditionally by comparing expected vs current generations.
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/partition/PartitionInfo.java:
##########
@@ -1014,13 +1083,17 @@ public boolean processTakeSnapshot(File snapshotDir)
throws TException, IOExcept
TProtocol protocol = new TBinaryProtocol(tioStreamTransport);
// serialize nextRegionGroupId
+ ReadWriteIOUtils.write(SNAPSHOT_WITH_DATABASE_GENERATION_MAGIC,
bufferedOutputStream);
ReadWriteIOUtils.write(nextRegionGroupId.get(), bufferedOutputStream);
+ ReadWriteIOUtils.write(nextDatabaseGeneration.get(),
bufferedOutputStream);
Review Comment:
This comment is now slightly misleading: the snapshot writes a magic header
value first (to detect generation-aware snapshots) and then writes both
`nextRegionGroupId` and `nextDatabaseGeneration`. Updating the comment makes
the snapshot format clearer for future maintenance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
