Re: [PR] Support some configurations about data partition base on database level [iotdb]

["CRZbulabula (via GitHub)" <gi...@apache.org>]

CRZbulabula commented on code in PR #17425:
URL: https://github.com/apache/iotdb/pull/17425#discussion_r3861141032


##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/service/thrift/ConfigNodeRPCServiceProcessor.java:
##########
@@ -537,6 +523,12 @@ public TSStatus setTimePartitionInterval(final TSetTimePartitionIntervalReq req)
         new SetTimePartitionIntervalPlan(req.getDatabase(), req.getTimePartitionInterval()));
   }
 
+  @Override
+  public TSStatus setTimePartitionOrigin(final TSetTimePartitionOriginReq req) throws TException {

Review Comment:
   Please do not expose a mutable time-partition setter through the RPC layer. `time_partition_origin` and `time_partition_interval` define the physical partition key; changing either one remaps timestamps to different `TTimePartitionSlot` / partition IDs, while existing `DataPartitionTable` entries and TsFiles are not migrated. These properties should be accepted only by `CREATE DATABASE`, and the general `ALTER DATABASE` request should reject them.



##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java:
##########
@@ -280,11 +284,21 @@ public TSStatus alterDatabase(final DatabaseSchemaPlan plan) {
             currentSchema.isNeedLastCache());
       }
 
+      if (alterSchema.isSetTimePartitionOrigin()) {
+        currentSchema.setTimePartitionOrigin(alterSchema.getTimePartitionOrigin());

Review Comment:
   Please remove this ALTER mutation. Updating only the schema and `TimePartitionUtils` changes the partition function for an existing database without migrating its data-partition table, region assignments, TsFile partition IDs, or recovery metadata. The CREATE path should persist either the explicit value or the resolved global default, and these values must remain immutable afterwards.



##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/schema/ClusterSchemaManager.java:
##########
@@ -485,6 +498,41 @@ public TSStatus setTimePartitionInterval(
     }
   }
 
+  public TSStatus setTimePartitionOrigin(SetTimePartitionOriginPlan setTimePartitionOriginPlan) {
+    final TSStatus validationResult =
+        validateTimePartitionConfig(
+            new TDatabaseSchema(setTimePartitionOriginPlan.getDatabase())
+                .setTimePartitionOrigin(setTimePartitionOriginPlan.getTimePartitionOrigin()));
+    if (validationResult != null) {
+      return validationResult;
+    }
+    try {
+      return getConsensusManager().write(setTimePartitionOriginPlan);

Review Comment:
   Please remove this setter path instead of adding DataNode notification. Because the contract is CREATE-only, origin/interval must not have consensus setter APIs that can mutate an existing database. Rejecting these fields at the ALTER boundary also avoids a distributed window where different DataNodes calculate different partition IDs.



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/executor/ClusterConfigTaskExecutor.java:
##########
@@ -524,9 +553,15 @@ public SettableFuture<ConfigTaskResult> alterDatabase(
         }
         future.setException(new IoTDBException(tsStatus));
       } else {
+        if (databaseSchema.isSetTimePartitionOrigin()
+            || databaseSchema.isSetTimePartitionInterval()
+            || databaseSchema.isSetNeedLastCache()) {
+          ClusterPartitionFetcher.getInstance().invalidAllCache();

Review Comment:
   Local cache invalidation cannot make this ALTER safe: it only refreshes the DataNode executing the statement, while other DataNodes can keep the old database configuration and route the same timestamp to a different partition ID. Please remove the ALTER handling end-to-end; cache population should happen only after CREATE, using explicit values or resolved global defaults.



-- 
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: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org