CRZbulabula commented on code in PR #17988:
URL: https://github.com/apache/iotdb/pull/17988#discussion_r3448171209
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/schema/ClusterSchemaManager.java:
##########
@@ -267,6 +268,27 @@ public TSStatus alterDatabase(
}
}
Review Comment:
Both min region group num params are deprecated, you can delete them.
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/schema/ClusterSchemaManager.java:
##########
@@ -507,60 +537,64 @@ public synchronized void adjustMaxRegionGroupNum() {
continue;
}
- // Adjust maxSchemaRegionGroupNum for each Database.
- // All Databases share the DataNodes equally.
- // The allocated SchemaRegionGroups will not be shrunk.
- final int allocatedSchemaRegionGroupCount;
- try {
- allocatedSchemaRegionGroupCount =
- getPartitionManager()
- .getRegionGroupCount(databaseSchema.getName(),
TConsensusGroupType.SchemaRegion);
- } catch (final DatabaseNotExistsException e) {
- // ignore the pre deleted database
- continue;
+ int maxSchemaRegionGroupNum =
databaseSchema.getMaxSchemaRegionGroupNum();
Review Comment:
Current implementation of this for-loop can be further optimized:
1. the adjustment logic for both schema and data region groups share lots of
codes, can be extracted to some common functions.
2. the `calcMaxRegionGroupNum` can be invoked at most twice.
##########
integration-test/src/test/java/org/apache/iotdb/relational/it/schema/IoTDBDatabaseMaxRegionGroupNumIT.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * 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.relational.it.schema;
+
+import org.apache.iotdb.db.it.utils.TestUtils;
+import org.apache.iotdb.it.env.EnvFactory;
+import org.apache.iotdb.it.framework.IoTDBTestRunner;
+import org.apache.iotdb.itbase.category.TableClusterIT;
+import org.apache.iotdb.itbase.category.TableLocalStandaloneIT;
+import org.apache.iotdb.itbase.env.BaseEnv;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.Collections;
+
+@RunWith(IoTDBTestRunner.class)
+@Category({TableLocalStandaloneIT.class, TableClusterIT.class})
+public class IoTDBDatabaseMaxRegionGroupNumIT {
+
+ @Before
+ public void setUp() throws Exception {
+ EnvFactory.getEnv()
+ .getConfig()
+ .getCommonConfig()
+ .setSchemaRegionGroupExtensionPolicy("CUSTOM")
+ .setDataRegionGroupExtensionPolicy("CUSTOM")
+ .setDefaultSchemaRegionGroupNumPerDatabase(1)
+ .setDefaultDataRegionGroupNumPerDatabase(2);
+ EnvFactory.getEnv().initClusterEnvironment();
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ EnvFactory.getEnv().cleanClusterEnvironment();
+ }
+
+ @Test
+ public void testCreateAndAlterMaxRegionGroupNum() throws SQLException {
+ try (final Connection connection =
+ EnvFactory.getEnv().getConnection(BaseEnv.TABLE_SQL_DIALECT);
+ final Statement statement = connection.createStatement()) {
+ statement.execute(
+ "create database test_create with(max_schema_region_group_num=3,
max_data_region_group_num=4)");
+ TestUtils.assertResultSetEqual(
+ statement.executeQuery(
+ "select database, max_schema_region_group_num,
max_data_region_group_num "
+ + "from information_schema.databases where database =
'test_create'"),
+ "database,max_schema_region_group_num,max_data_region_group_num,",
+ Collections.singleton("test_create,3,4,"));
Review Comment:
To verify the demand number is set correctly, u should not only test the
metadata (the number obtained via information_schema or show databases), but
also write some data and check if the region groups are created successfully.
Not sure if we have these CIs currently, you should check and append when
necessary.
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/ClusterSchemaInfo.java:
##########
@@ -267,6 +267,20 @@ public TSStatus alterDatabase(final DatabaseSchemaPlan
plan) {
currentSchema.getName(),
currentSchema.getMaxDataRegionGroupNum());
Review Comment:
Similar as above, these are useless codes now. You can delete them.
--
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]
