You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2022/03/11 02:45:53 UTC

[GitHub] [iotdb] CRZbulabula opened a new pull request #5199: Merge new_cluster into master

CRZbulabula opened a new pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199


   Changes:
   
   - Add confignode package
   - Add thrift-confignode
   - Add DeviceGroupHashExecutor interface


-- 
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



[GitHub] [iotdb] CRZbulabula commented on a change in pull request #5199: [New Cluster]Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#discussion_r825391596



##########
File path: thrift-confignode/src/main/thrift/confignode.thrift
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.
+ */
+
+include "rpc.thrift"
+namespace java org.apache.iotdb.confignode.rpc.thrift
+namespace py iotdb.thrift.confignode
+
+service ConfigIService {
+  rpc.TSStatus setStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroups(1:i64 sessionId, 2:list<string> storageGroups);
+
+  i32 getSchemaPartition(1:i64 sessionId, 2:string device)
+
+  list<i32> getDataPartition(1:i64 sessionId, 2:string device, 3:list<i64> times)

Review comment:
       This is the older interface. The new interface is defined in the following documentation:
   
   - [分区接口定义](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=199536930)
   - [ConfigNode thrift-rpc 接口定义](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=199538743)




-- 
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



[GitHub] [iotdb] sonarcloud[bot] removed a comment on pull request #5199: [To new_cluster] Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#issuecomment-1064747184


   SonarCloud Quality Gate failed.&nbsp; &nbsp; ![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=CODE_SMELL) [63 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=CODE_SMELL)
   
   [![44.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/40-16px.png '44.5%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=5199&metric=new_coverage&view=list) [44.5% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=5199&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=5199&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=5199&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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



[GitHub] [iotdb] sonarcloud[bot] commented on pull request #5199: Merge new_cluster into master

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#issuecomment-1064747184


   SonarCloud Quality Gate failed.&nbsp; &nbsp; ![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=CODE_SMELL) [63 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=CODE_SMELL)
   
   [![44.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/40-16px.png '44.5%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=5199&metric=new_coverage&view=list) [44.5% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=5199&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=5199&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=5199&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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



[GitHub] [iotdb] coveralls commented on pull request #5199: Merge new_cluster into master

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#issuecomment-1064746639


   
   [![Coverage Status](https://coveralls.io/builds/47265449/badge)](https://coveralls.io/builds/47265449)
   
   Coverage increased (+0.01%) to 67.897% when pulling **7cff204c008d6dffb70766b5324ce7ed65141238 on new_cluster** into **fd381934eba3d5e8ca04c8b830ef011416d5ff37 on master**.
   


-- 
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



[GitHub] [iotdb] neuyilan commented on a change in pull request #5199: [New Cluster]Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#discussion_r825263358



##########
File path: thrift-confignode/src/main/thrift/confignode.thrift
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.
+ */
+
+include "rpc.thrift"
+namespace java org.apache.iotdb.confignode.rpc.thrift
+namespace py iotdb.thrift.confignode
+
+service ConfigIService {
+  rpc.TSStatus setStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroups(1:i64 sessionId, 2:list<string> storageGroups);
+
+  i32 getSchemaPartition(1:i64 sessionId, 2:string device)
+
+  list<i32> getDataPartition(1:i64 sessionId, 2:string device, 3:list<i64> times)

Review comment:
       It is better to add one interface which can get a multi device's data partition.




-- 
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



[GitHub] [iotdb] CRZbulabula commented on a change in pull request #5199: [New Cluster]Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#discussion_r825391245



##########
File path: thrift-confignode/src/main/thrift/confignode.thrift
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.
+ */
+
+include "rpc.thrift"
+namespace java org.apache.iotdb.confignode.rpc.thrift
+namespace py iotdb.thrift.confignode
+
+service ConfigIService {
+  rpc.TSStatus setStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroups(1:i64 sessionId, 2:list<string> storageGroups);
+
+  i32 getSchemaPartition(1:i64 sessionId, 2:string device)
+
+  list<i32> getDataPartition(1:i64 sessionId, 2:string device, 3:list<i64> times)
+
+  list<i32> getLatestDataPartition(1:i64 sessionId, 2:string device)

Review comment:
       This is a redundant interface, I will delete it.




-- 
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



[GitHub] [iotdb] CRZbulabula commented on a change in pull request #5199: [New Cluster]Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#discussion_r825388107



##########
File path: thrift-confignode/src/main/thrift/confignode.thrift
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.
+ */
+
+include "rpc.thrift"
+namespace java org.apache.iotdb.confignode.rpc.thrift
+namespace py iotdb.thrift.confignode
+
+service ConfigIService {
+  rpc.TSStatus setStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroups(1:i64 sessionId, 2:list<string> storageGroups);
+
+  i32 getSchemaPartition(1:i64 sessionId, 2:string device)
+
+  list<i32> getDataPartition(1:i64 sessionId, 2:string device, 3:list<i64> times)
+
+  list<i32> getLatestDataPartition(1:i64 sessionId, 2:string device)

Review comment:
       "Latest" means the currently dataPartition on time.




-- 
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



[GitHub] [iotdb] qiaojialin commented on a change in pull request #5199: [To new_cluster] Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
qiaojialin commented on a change in pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#discussion_r825441265



##########
File path: confignode/src/assembly/resources/conf/iotdb-confignode.properties
##########
@@ -0,0 +1,37 @@
+#
+# 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.
+#
+
+####################
+### DeviceGroup Configuration
+####################
+
+# Number of DeviceGroups per StorageGroup
+# Datatype: int
+# device_group_count=10000
+
+# DeviceGroup hash algorithm
+# Datatype: String
+# These hashing algorithms are currently supported:
+# 1. org.apache.iotdb.confignode.manager.hash.BKDRHashExecutor(Default)
+# 2. org.apache.iotdb.confignode.manager.hash.APHashExecutor
+# 3. org.apache.iotdb.confignode.manager.hash.JSHashExecutor
+# 4. org.apache.iotdb.confignode.manager.hash.SDBMHashExecutor

Review comment:
       ok




-- 
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



[GitHub] [iotdb] CRZbulabula commented on a change in pull request #5199: [New Cluster]Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#discussion_r825387871



##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java
##########
@@ -0,0 +1,89 @@
+/*
+ * 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;
+
+import org.apache.iotdb.confignode.conf.ConfigNodeConf;
+import org.apache.iotdb.confignode.conf.ConfigNodeDescriptor;
+import org.apache.iotdb.confignode.manager.hash.DeviceGroupHashExecutor;
+import org.apache.iotdb.confignode.partition.PartitionTable;
+import org.apache.iotdb.confignode.service.balancer.LoadBalancer;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+/**
+ * ConfigManager Maintains consistency between PartitionTables in the ConfigNodeGroup. Expose the
+ * query interface for the PartitionTable
+ */
+public class ConfigManager {
+
+  private static final Logger LOGGER = LoggerFactory.getLogger(ConfigManager.class);
+
+  private DeviceGroupHashExecutor hashExecutor;
+
+  private Lock partitionTableLock;
+  private PartitionTable partitionTable;
+
+  private LoadBalancer loadBalancer;
+
+  public ConfigManager(String hashExecutorClass, int deviceGroupCount) {
+    setHashExecutor(hashExecutorClass, deviceGroupCount);
+  }
+
+  public ConfigManager() {
+    ConfigNodeConf config = ConfigNodeDescriptor.getInstance().getConf();
+
+    setHashExecutor(config.getDeviceGroupHashExecutorClass(), config.getDeviceGroupCount());
+
+    this.partitionTableLock = new ReentrantLock();
+    this.partitionTable = new PartitionTable();
+
+    this.loadBalancer = new LoadBalancer(partitionTableLock, partitionTable);
+  }

Review comment:
       This constructor may be changed latter.  Maybe there is not need to change the code structure for a while.




-- 
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



[GitHub] [iotdb] qiaojialin merged pull request #5199: [To new_cluster] Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
qiaojialin merged pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199


   


-- 
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



[GitHub] [iotdb] CRZbulabula commented on a change in pull request #5199: [New Cluster]Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#discussion_r825391752



##########
File path: confignode/src/assembly/resources/conf/iotdb-confignode.properties
##########
@@ -0,0 +1,37 @@
+#
+# 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.
+#
+
+####################
+### DeviceGroup Configuration
+####################
+
+# Number of DeviceGroups per StorageGroup
+# Datatype: int
+# device_group_count=10000

Review comment:
       The DeviceGroup hash algorithm also needs to be able be used by DataNode, so I think maybe a new PR is needed to create a common tool package between ConfigNode and DataNode. I will solve this problem in the new PR.




-- 
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



[GitHub] [iotdb] coveralls edited a comment on pull request #5199: [To new_cluster] Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#issuecomment-1064746639


   
   [![Coverage Status](https://coveralls.io/builds/47308556/badge)](https://coveralls.io/builds/47308556)
   
   Coverage increased (+0.1%) to 67.997% when pulling **4d06e99523ef8e2e64e480e63b6b3fd8ee48d354 on new_cluster** into **fd381934eba3d5e8ca04c8b830ef011416d5ff37 on master**.
   


-- 
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



[GitHub] [iotdb] CRZbulabula commented on a change in pull request #5199: [To new_cluster] Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#discussion_r825440083



##########
File path: thrift-confignode/src/main/thrift/confignode.thrift
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.
+ */
+
+include "rpc.thrift"
+namespace java org.apache.iotdb.confignode.rpc.thrift
+namespace py iotdb.thrift.confignode
+
+service ConfigIService {
+  rpc.TSStatus setStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroups(1:i64 sessionId, 2:list<string> storageGroups);
+
+  i32 getSchemaPartition(1:i64 sessionId, 2:string device)
+
+  list<i32> getDataPartition(1:i64 sessionId, 2:string device, 3:list<i64> times)
+
+  list<i32> getLatestDataPartition(1:i64 sessionId, 2:string device)

Review comment:
       This is the older interface. The new interface is defined in the following documentation:
   
   - [分区接口定义](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=199536930)
   - [ConfigNode thrift-rpc 接口定义](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=199538743)




-- 
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



[GitHub] [iotdb] neuyilan commented on a change in pull request #5199: Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#discussion_r825253123



##########
File path: confignode/src/assembly/resources/conf/iotdb-confignode.properties
##########
@@ -0,0 +1,37 @@
+#
+# 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.
+#
+
+####################
+### DeviceGroup Configuration
+####################
+
+# Number of DeviceGroups per StorageGroup
+# Datatype: int
+# device_group_count=10000
+
+# DeviceGroup hash algorithm
+# Datatype: String
+# These hashing algorithms are currently supported:
+# 1. org.apache.iotdb.confignode.manager.hash.BKDRHashExecutor(Default)
+# 2. org.apache.iotdb.confignode.manager.hash.APHashExecutor
+# 3. org.apache.iotdb.confignode.manager.hash.JSHashExecutor
+# 4. org.apache.iotdb.confignode.manager.hash.SDBMHashExecutor
+# Also, if you want to implement your own hash algorithm, you can inherit the DeviceGroupHashExecutor class and
+# modify this parameter to correspond to your Java class
+# device_group_hash_executor_class=org.apache.iotdb.confignode.manager.hash.BKDRHashExecutor

Review comment:
       the same as above

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java
##########
@@ -0,0 +1,89 @@
+/*
+ * 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;
+
+import org.apache.iotdb.confignode.conf.ConfigNodeConf;
+import org.apache.iotdb.confignode.conf.ConfigNodeDescriptor;
+import org.apache.iotdb.confignode.manager.hash.DeviceGroupHashExecutor;
+import org.apache.iotdb.confignode.partition.PartitionTable;
+import org.apache.iotdb.confignode.service.balancer.LoadBalancer;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+/**
+ * ConfigManager Maintains consistency between PartitionTables in the ConfigNodeGroup. Expose the
+ * query interface for the PartitionTable
+ */
+public class ConfigManager {
+
+  private static final Logger LOGGER = LoggerFactory.getLogger(ConfigManager.class);
+
+  private DeviceGroupHashExecutor hashExecutor;
+
+  private Lock partitionTableLock;
+  private PartitionTable partitionTable;
+
+  private LoadBalancer loadBalancer;
+
+  public ConfigManager(String hashExecutorClass, int deviceGroupCount) {
+    setHashExecutor(hashExecutorClass, deviceGroupCount);
+  }
+
+  public ConfigManager() {
+    ConfigNodeConf config = ConfigNodeDescriptor.getInstance().getConf();
+
+    setHashExecutor(config.getDeviceGroupHashExecutorClass(), config.getDeviceGroupCount());
+
+    this.partitionTableLock = new ReentrantLock();
+    this.partitionTable = new PartitionTable();
+
+    this.loadBalancer = new LoadBalancer(partitionTableLock, partitionTable);
+  }

Review comment:
       ```suggestion
     public ConfigManager() {
       ConfigNodeConf config = ConfigNodeDescriptor.getInstance().getConf();
       this(config.getDeviceGroupHashExecutorClass(), config.getDeviceGroupCount())
     }
   ```

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/partition/DataPartitionRule.java
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.partition;
+
+import org.apache.iotdb.tsfile.utils.Pair;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * DataPartitionRule is used to hold real-time write-load allocation rules i.e. rules = [(0, 0.3),
+ * (1, 0.2), (2. 0.5)] means allocate 30% of the write-load to VSGGroup-0 and 20% to vsgGroup-1 and
+ * 50% to VSGGroup-2
+ */
+public class DataPartitionRule {
+  // List<Pair<VSGGroupID, Write allocation ratio>>

Review comment:
       As we have changed VSG concept to region, it is better to change the comment also.

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java
##########
@@ -0,0 +1,89 @@
+/*
+ * 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;
+
+import org.apache.iotdb.confignode.conf.ConfigNodeConf;
+import org.apache.iotdb.confignode.conf.ConfigNodeDescriptor;
+import org.apache.iotdb.confignode.manager.hash.DeviceGroupHashExecutor;
+import org.apache.iotdb.confignode.partition.PartitionTable;
+import org.apache.iotdb.confignode.service.balancer.LoadBalancer;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+/**
+ * ConfigManager Maintains consistency between PartitionTables in the ConfigNodeGroup. Expose the
+ * query interface for the PartitionTable
+ */
+public class ConfigManager {
+
+  private static final Logger LOGGER = LoggerFactory.getLogger(ConfigManager.class);
+
+  private DeviceGroupHashExecutor hashExecutor;
+
+  private Lock partitionTableLock;
+  private PartitionTable partitionTable;
+
+  private LoadBalancer loadBalancer;
+
+  public ConfigManager(String hashExecutorClass, int deviceGroupCount) {
+    setHashExecutor(hashExecutorClass, deviceGroupCount);
+  }

Review comment:
       ```suggestion
     public ConfigManager(String hashExecutorClass, int deviceGroupCount) {
       setHashExecutor(hashExecutorClass, deviceGroupCount);
       this.partitionTableLock = new ReentrantLock();
       this.partitionTable = new PartitionTable();
       this.loadBalancer = new LoadBalancer(partitionTableLock, partitionTable);
     }
   ```
   
   
   

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java
##########
@@ -0,0 +1,89 @@
+/*
+ * 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;
+
+import org.apache.iotdb.confignode.conf.ConfigNodeConf;
+import org.apache.iotdb.confignode.conf.ConfigNodeDescriptor;
+import org.apache.iotdb.confignode.manager.hash.DeviceGroupHashExecutor;
+import org.apache.iotdb.confignode.partition.PartitionTable;
+import org.apache.iotdb.confignode.service.balancer.LoadBalancer;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+/**
+ * ConfigManager Maintains consistency between PartitionTables in the ConfigNodeGroup. Expose the

Review comment:
       Maintains -> maintains

##########
File path: thrift-confignode/src/main/thrift/confignode.thrift
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.
+ */
+
+include "rpc.thrift"
+namespace java org.apache.iotdb.confignode.rpc.thrift
+namespace py iotdb.thrift.confignode
+
+service ConfigIService {
+  rpc.TSStatus setStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroups(1:i64 sessionId, 2:list<string> storageGroups);
+
+  i32 getSchemaPartition(1:i64 sessionId, 2:string device)
+
+  list<i32> getDataPartition(1:i64 sessionId, 2:string device, 3:list<i64> times)
+
+  list<i32> getLatestDataPartition(1:i64 sessionId, 2:string device)

Review comment:
       What is the meaning of the Latest?

##########
File path: confignode/src/assembly/resources/conf/iotdb-confignode.properties
##########
@@ -0,0 +1,37 @@
+#
+# 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.
+#
+
+####################
+### DeviceGroup Configuration
+####################
+
+# Number of DeviceGroups per StorageGroup
+# Datatype: int
+# device_group_count=10000

Review comment:
       it would be better to note that this parameter can not be changed once set.




-- 
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



[GitHub] [iotdb] CRZbulabula commented on a change in pull request #5199: [New Cluster]Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#discussion_r825387796



##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java
##########
@@ -0,0 +1,89 @@
+/*
+ * 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;
+
+import org.apache.iotdb.confignode.conf.ConfigNodeConf;
+import org.apache.iotdb.confignode.conf.ConfigNodeDescriptor;
+import org.apache.iotdb.confignode.manager.hash.DeviceGroupHashExecutor;
+import org.apache.iotdb.confignode.partition.PartitionTable;
+import org.apache.iotdb.confignode.service.balancer.LoadBalancer;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+/**
+ * ConfigManager Maintains consistency between PartitionTables in the ConfigNodeGroup. Expose the
+ * query interface for the PartitionTable
+ */
+public class ConfigManager {
+
+  private static final Logger LOGGER = LoggerFactory.getLogger(ConfigManager.class);
+
+  private DeviceGroupHashExecutor hashExecutor;
+
+  private Lock partitionTableLock;
+  private PartitionTable partitionTable;
+
+  private LoadBalancer loadBalancer;
+
+  public ConfigManager(String hashExecutorClass, int deviceGroupCount) {
+    setHashExecutor(hashExecutorClass, deviceGroupCount);
+  }

Review comment:
       This is a test only constructor. There is no need to create PartitionTable and LoadBalancer for now




-- 
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



[GitHub] [iotdb] sonarcloud[bot] commented on pull request #5199: [To new_cluster] Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#issuecomment-1066108151


   SonarCloud Quality Gate failed.&nbsp; &nbsp; ![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=CODE_SMELL) [61 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=5199&resolved=false&types=CODE_SMELL)
   
   [![33.7%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/25-16px.png '33.7%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=5199&metric=new_coverage&view=list) [33.7% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=5199&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=5199&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=5199&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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



[GitHub] [iotdb] qiaojialin commented on a change in pull request #5199: [New Cluster]Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
qiaojialin commented on a change in pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#discussion_r825255224



##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/manager/hash/JSHash.java
##########
@@ -0,0 +1,40 @@
+/*
+ * 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.hash;
+
+public class JSHash extends DeviceGroupHashExecutor {

Review comment:
       ```suggestion
   public class JSHashExecutor extends DeviceGroupHashExecutor {
   ```
   
   Unify the name of JSHash, SDBMHash, APHashExecutor, BKDRHashExecutor

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/partition/DataPartitionRule.java
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.partition;
+
+import org.apache.iotdb.tsfile.utils.Pair;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * DataPartitionRule is used to hold real-time write-load allocation rules i.e. rules = [(0, 0.3),
+ * (1, 0.2), (2. 0.5)] means allocate 30% of the write-load to VSGGroup-0 and 20% to vsgGroup-1 and
+ * 50% to VSGGroup-2
+ */
+public class DataPartitionRule {
+  // List<Pair<VSGGroupID, Write allocation ratio>>

Review comment:
       ```suggestion
     // List<Pair<DataRegionGroupID, Write allocation ratio>>
   ```

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/partition/PartitionTable.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.partition;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * PartitionTable stores metadata partition table, data partition table, and real-time write load
+ * allocation rules
+ */
+public class PartitionTable {
+
+  // Map<StorageGroup, Map<DeviceGroupID, SchemaRegionGroupID>>
+  private final Map<String, Map<Integer, Integer>> metadataPartitionTable;
+  // Map<SchemaRegionGroupID, List<DataNodeID>>
+  private final Map<Integer, List<Integer>> schemaRegionGroupDataNodesMap;
+
+  // Map<StorageGroup, Map<DeviceGroupID, Map<TimeInterval, List<DataRegionGroupID>>>>
+  private final Map<String, Map<Integer, Map<Long, List<Integer>>>> dataPartitionTable;
+  // Map<DataRegionGroupID, List<DataNodeID>>
+  private final Map<Integer, List<Integer>> dataRegionGroupDataNodesMap;

Review comment:
       ```suggestion
     private final Map<Integer, List<Integer>> dataRegionDataNodesMap;
   ```

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/partition/DataPartitionRule.java
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.partition;
+
+import org.apache.iotdb.tsfile.utils.Pair;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * DataPartitionRule is used to hold real-time write-load allocation rules i.e. rules = [(0, 0.3),
+ * (1, 0.2), (2. 0.5)] means allocate 30% of the write-load to VSGGroup-0 and 20% to vsgGroup-1 and
+ * 50% to VSGGroup-2
+ */
+public class DataPartitionRule {
+  // List<Pair<VSGGroupID, Write allocation ratio>>
+  private final List<Pair<Integer, Double>> rules;
+
+  public DataPartitionRule() {
+    this.rules = new ArrayList<>();
+  }
+
+  public DataPartitionRule(List<Pair<Integer, Double>> rules) {
+    this.rules = rules;
+  }
+
+  public void addDataPartitionRule(int vsgGroupID, double ratio) {

Review comment:
       ```suggestion
     public void addDataPartitionRule(int dataRegionGroupID, double ratio) {
   ```

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/partition/PartitionTable.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.partition;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * PartitionTable stores metadata partition table, data partition table, and real-time write load

Review comment:
       ```suggestion
    * PartitionTable stores schema partition table, data partition table, and real-time write load
   ```

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/partition/PartitionTable.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.partition;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * PartitionTable stores metadata partition table, data partition table, and real-time write load
+ * allocation rules
+ */
+public class PartitionTable {
+
+  // Map<StorageGroup, Map<DeviceGroupID, SchemaRegionGroupID>>
+  private final Map<String, Map<Integer, Integer>> metadataPartitionTable;
+  // Map<SchemaRegionGroupID, List<DataNodeID>>
+  private final Map<Integer, List<Integer>> schemaRegionGroupDataNodesMap;

Review comment:
       ```suggestion
     private final Map<Integer, List<Integer>> schemaRegionDataNodesMap;
   ```

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/partition/PartitionTable.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.partition;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * PartitionTable stores metadata partition table, data partition table, and real-time write load
+ * allocation rules
+ */
+public class PartitionTable {
+
+  // Map<StorageGroup, Map<DeviceGroupID, SchemaRegionGroupID>>
+  private final Map<String, Map<Integer, Integer>> metadataPartitionTable;

Review comment:
       ```suggestion
     private final Map<String, Map<Integer, Integer>> schemaPartitionTable;
   ```

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/partition/DataPartitionRule.java
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.partition;
+
+import org.apache.iotdb.tsfile.utils.Pair;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * DataPartitionRule is used to hold real-time write-load allocation rules i.e. rules = [(0, 0.3),
+ * (1, 0.2), (2. 0.5)] means allocate 30% of the write-load to VSGGroup-0 and 20% to vsgGroup-1 and

Review comment:
       ```suggestion
    * (1, 0.2), (2. 0.5)] means allocate 30% of the write-load to DataRegion-0 and 20% to DataRegion-1 and
   ```

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/partition/DataPartitionRule.java
##########
@@ -0,0 +1,50 @@
+/*
+ * 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.partition;
+
+import org.apache.iotdb.tsfile.utils.Pair;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * DataPartitionRule is used to hold real-time write-load allocation rules i.e. rules = [(0, 0.3),
+ * (1, 0.2), (2. 0.5)] means allocate 30% of the write-load to VSGGroup-0 and 20% to vsgGroup-1 and
+ * 50% to VSGGroup-2

Review comment:
       ```suggestion
    * 50% to DataRegion-2
   ```

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java
##########
@@ -0,0 +1,89 @@
+/*
+ * 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;
+
+import org.apache.iotdb.confignode.conf.ConfigNodeConf;
+import org.apache.iotdb.confignode.conf.ConfigNodeDescriptor;
+import org.apache.iotdb.confignode.manager.hash.DeviceGroupHashExecutor;
+import org.apache.iotdb.confignode.partition.PartitionTable;
+import org.apache.iotdb.confignode.service.balancer.LoadBalancer;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+/**
+ * ConfigManager Maintains consistency between PartitionTables in the ConfigNodeGroup. Expose the
+ * query interface for the PartitionTable
+ */
+public class ConfigManager {
+
+  private static final Logger LOGGER = LoggerFactory.getLogger(ConfigManager.class);
+
+  private DeviceGroupHashExecutor hashExecutor;
+
+  private Lock partitionTableLock;
+  private PartitionTable partitionTable;
+
+  private LoadBalancer loadBalancer;
+
+  public ConfigManager(String hashExecutorClass, int deviceGroupCount) {
+    setHashExecutor(hashExecutorClass, deviceGroupCount);
+  }
+
+  public ConfigManager() {
+    ConfigNodeConf config = ConfigNodeDescriptor.getInstance().getConf();
+
+    setHashExecutor(config.getDeviceGroupHashExecutorClass(), config.getDeviceGroupCount());
+
+    this.partitionTableLock = new ReentrantLock();
+    this.partitionTable = new PartitionTable();
+
+    this.loadBalancer = new LoadBalancer(partitionTableLock, partitionTable);
+  }
+
+  private void setHashExecutor(String hashExecutorClass, int deviceGroupCount) {

Review comment:
       These two parameters should not be changed after first start, need to store them in a confignode.properties inside ConfigNode like IoTDBConfigCheck

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/partition/PartitionTable.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.partition;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * PartitionTable stores metadata partition table, data partition table, and real-time write load
+ * allocation rules
+ */
+public class PartitionTable {
+
+  // Map<StorageGroup, Map<DeviceGroupID, SchemaRegionGroupID>>
+  private final Map<String, Map<Integer, Integer>> metadataPartitionTable;
+  // Map<SchemaRegionGroupID, List<DataNodeID>>

Review comment:
       ```suggestion
     // Map<SchemaRegionID, List<DataNodeID>>
   ```

##########
File path: confignode/src/assembly/resources/conf/iotdb-confignode.properties
##########
@@ -0,0 +1,37 @@
+#
+# 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.
+#
+
+####################
+### DeviceGroup Configuration
+####################
+
+# Number of DeviceGroups per StorageGroup
+# Datatype: int
+# device_group_count=10000
+
+# DeviceGroup hash algorithm
+# Datatype: String
+# These hashing algorithms are currently supported:
+# 1. org.apache.iotdb.confignode.manager.hash.BKDRHashExecutor(Default)
+# 2. org.apache.iotdb.confignode.manager.hash.APHashExecutor
+# 3. org.apache.iotdb.confignode.manager.hash.JSHashExecutor
+# 4. org.apache.iotdb.confignode.manager.hash.SDBMHashExecutor

Review comment:
       characteristic of these hash function could be introduced somewhere: Here or Javadoc or UserGuide

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/partition/PartitionTable.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.partition;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * PartitionTable stores metadata partition table, data partition table, and real-time write load
+ * allocation rules
+ */
+public class PartitionTable {
+
+  // Map<StorageGroup, Map<DeviceGroupID, SchemaRegionGroupID>>

Review comment:
       ```suggestion
     // Map<StorageGroup, Map<DeviceGroupID, SchemaRegionID>>
   ```

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/partition/PartitionTable.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.partition;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * PartitionTable stores metadata partition table, data partition table, and real-time write load
+ * allocation rules
+ */
+public class PartitionTable {
+
+  // Map<StorageGroup, Map<DeviceGroupID, SchemaRegionGroupID>>
+  private final Map<String, Map<Integer, Integer>> metadataPartitionTable;
+  // Map<SchemaRegionGroupID, List<DataNodeID>>
+  private final Map<Integer, List<Integer>> schemaRegionGroupDataNodesMap;
+
+  // Map<StorageGroup, Map<DeviceGroupID, Map<TimeInterval, List<DataRegionGroupID>>>>
+  private final Map<String, Map<Integer, Map<Long, List<Integer>>>> dataPartitionTable;
+  // Map<DataRegionGroupID, List<DataNodeID>>

Review comment:
       ```suggestion
     // Map<DataRegionID, List<DataNodeID>>
   ```

##########
File path: thrift-confignode/src/main/thrift/confignode.thrift
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.
+ */
+
+include "rpc.thrift"
+namespace java org.apache.iotdb.confignode.rpc.thrift
+namespace py iotdb.thrift.confignode
+
+service ConfigIService {
+  rpc.TSStatus setStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroups(1:i64 sessionId, 2:list<string> storageGroups);
+
+  i32 getSchemaPartition(1:i64 sessionId, 2:string device)
+
+  list<i32> getDataPartition(1:i64 sessionId, 2:string device, 3:list<i64> times)
+
+  list<i32> getLatestDataPartition(1:i64 sessionId, 2:string device)

Review comment:
       what's this for?

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/partition/PartitionTable.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.confignode.partition;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * PartitionTable stores metadata partition table, data partition table, and real-time write load
+ * allocation rules
+ */
+public class PartitionTable {
+
+  // Map<StorageGroup, Map<DeviceGroupID, SchemaRegionGroupID>>
+  private final Map<String, Map<Integer, Integer>> metadataPartitionTable;
+  // Map<SchemaRegionGroupID, List<DataNodeID>>
+  private final Map<Integer, List<Integer>> schemaRegionGroupDataNodesMap;
+
+  // Map<StorageGroup, Map<DeviceGroupID, Map<TimeInterval, List<DataRegionGroupID>>>>

Review comment:
       ```suggestion
     // Map<StorageGroup, Map<DeviceGroupID, Map<TimeInterval, List<DataRegionID>>>>
   ```

##########
File path: thrift-confignode/src/main/thrift/confignode.thrift
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.
+ */
+
+include "rpc.thrift"
+namespace java org.apache.iotdb.confignode.rpc.thrift
+namespace py iotdb.thrift.confignode
+
+service ConfigIService {
+  rpc.TSStatus setStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroups(1:i64 sessionId, 2:list<string> storageGroups);
+
+  i32 getSchemaPartition(1:i64 sessionId, 2:string device)
+
+  list<i32> getDataPartition(1:i64 sessionId, 2:string device, 3:list<i64> times)

Review comment:
       (1) make the times meaning clear: is this the data timestamp or partition start time? If data timestamp, we need to calculate the start time, if already receives a partition start time, we could eliminate one calculation.
   (2) need to add two interfaces
     list<i32> getDataPartition(1:i64 sessionId, 2:string deviceGroupId, 3:list<i64> partitionStartTime)
     list<i32> getDataPartition(1:i64 sessionId, 2:list<string> deviceGroupId, 3:i64 partitionStartTime)

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/service/thrift/impl/ConfigServiceImpl.java
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.service.thrift.impl;
+
+import org.apache.iotdb.confignode.manager.ConfigManager;
+import org.apache.iotdb.confignode.rpc.thrift.ConfigIService;
+import org.apache.iotdb.service.rpc.thrift.TSStatus;
+
+import org.apache.thrift.TException;
+
+import java.util.List;
+
+/** ConfigServiceImpl exposes the interface that interacts with the DataNode */

Review comment:
       ```suggestion
   /** ConfigNodeRPCServer exposes the interface that interacts with the DataNode */
   ```

##########
File path: confignode/src/test/resources/iotdb-confignode.properties
##########
@@ -0,0 +1,31 @@
+#
+# 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.
+#
+
+####################
+### DeviceGroup Configuration
+####################
+
+# Number of DeviceGroups per StorageGroup
+# Datatype: int
+# device_group_count=10000
+
+# DeviceGroup hash algorithm
+# Datatype: string
+# Currently, BKDR, AP, JS or SDBM hash algorithm is available
+# device_group_hash_algorithm=BKDR

Review comment:
       Do you use this in tests? Keep consistent with the properties with the main resource.(full class name)

##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/service/thrift/impl/ConfigServiceImpl.java
##########
@@ -0,0 +1,76 @@
+/*
+ * 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.service.thrift.impl;
+
+import org.apache.iotdb.confignode.manager.ConfigManager;
+import org.apache.iotdb.confignode.rpc.thrift.ConfigIService;
+import org.apache.iotdb.service.rpc.thrift.TSStatus;
+
+import org.apache.thrift.TException;
+
+import java.util.List;
+
+/** ConfigServiceImpl exposes the interface that interacts with the DataNode */
+public class ConfigServiceImpl implements ConfigIService.Iface {

Review comment:
       ```suggestion
   public class ConfigNodeRPCServer implements ConfigIService.Iface {
   ```




-- 
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



[GitHub] [iotdb] CRZbulabula commented on a change in pull request #5199: [New Cluster]Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#discussion_r825388217



##########
File path: confignode/src/assembly/resources/conf/iotdb-confignode.properties
##########
@@ -0,0 +1,37 @@
+#
+# 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.
+#
+
+####################
+### DeviceGroup Configuration
+####################
+
+# Number of DeviceGroups per StorageGroup
+# Datatype: int
+# device_group_count=10000
+
+# DeviceGroup hash algorithm
+# Datatype: String
+# These hashing algorithms are currently supported:
+# 1. org.apache.iotdb.confignode.manager.hash.BKDRHashExecutor(Default)
+# 2. org.apache.iotdb.confignode.manager.hash.APHashExecutor
+# 3. org.apache.iotdb.confignode.manager.hash.JSHashExecutor
+# 4. org.apache.iotdb.confignode.manager.hash.SDBMHashExecutor

Review comment:
       According to the test results so far, different hashing algorithms perform similarly under random tests. Maybe later in UserGuide?




-- 
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



[GitHub] [iotdb] CRZbulabula commented on a change in pull request #5199: [New Cluster]Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#discussion_r825388107



##########
File path: thrift-confignode/src/main/thrift/confignode.thrift
##########
@@ -0,0 +1,38 @@
+/*
+ * 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.
+ */
+
+include "rpc.thrift"
+namespace java org.apache.iotdb.confignode.rpc.thrift
+namespace py iotdb.thrift.confignode
+
+service ConfigIService {
+  rpc.TSStatus setStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroup(1:i64 sessionId, 2:string storageGroup);
+
+  rpc.TSStatus deleteStorageGroups(1:i64 sessionId, 2:list<string> storageGroups);
+
+  i32 getSchemaPartition(1:i64 sessionId, 2:string device)
+
+  list<i32> getDataPartition(1:i64 sessionId, 2:string device, 3:list<i64> times)
+
+  list<i32> getLatestDataPartition(1:i64 sessionId, 2:string device)

Review comment:
       "Latest" means the currently dataPartition on time.




-- 
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



[GitHub] [iotdb] CRZbulabula commented on a change in pull request #5199: [New Cluster]Construct basic framework of ConfigNode in new_cluster

Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #5199:
URL: https://github.com/apache/iotdb/pull/5199#discussion_r825391343



##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java
##########
@@ -0,0 +1,89 @@
+/*
+ * 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;
+
+import org.apache.iotdb.confignode.conf.ConfigNodeConf;
+import org.apache.iotdb.confignode.conf.ConfigNodeDescriptor;
+import org.apache.iotdb.confignode.manager.hash.DeviceGroupHashExecutor;
+import org.apache.iotdb.confignode.partition.PartitionTable;
+import org.apache.iotdb.confignode.service.balancer.LoadBalancer;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+/**
+ * ConfigManager Maintains consistency between PartitionTables in the ConfigNodeGroup. Expose the
+ * query interface for the PartitionTable
+ */
+public class ConfigManager {
+
+  private static final Logger LOGGER = LoggerFactory.getLogger(ConfigManager.class);
+
+  private DeviceGroupHashExecutor hashExecutor;
+
+  private Lock partitionTableLock;
+  private PartitionTable partitionTable;
+
+  private LoadBalancer loadBalancer;
+
+  public ConfigManager(String hashExecutorClass, int deviceGroupCount) {
+    setHashExecutor(hashExecutorClass, deviceGroupCount);
+  }
+
+  public ConfigManager() {
+    ConfigNodeConf config = ConfigNodeDescriptor.getInstance().getConf();
+
+    setHashExecutor(config.getDeviceGroupHashExecutorClass(), config.getDeviceGroupCount());
+
+    this.partitionTableLock = new ReentrantLock();
+    this.partitionTable = new PartitionTable();
+
+    this.loadBalancer = new LoadBalancer(partitionTableLock, partitionTable);
+  }
+
+  private void setHashExecutor(String hashExecutorClass, int deviceGroupCount) {

Review comment:
       After discussion with 思屹, he thinks DataNode also needs to get the hash algorithm for the device group. I think maybe a new PR is needed to create a common tool package between DataNode and ConfigNode. And in this new PR I will solve this problem.




-- 
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