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/06/25 01:53:56 UTC
[GitHub] [iotdb] qiaojialin commented on a diff in pull request #6405: [IOTDB-3124] Initialize ConfigNode with multi-seed mode
qiaojialin commented on code in PR #6405:
URL: https://github.com/apache/iotdb/pull/6405#discussion_r906138033
##########
thrift-confignode/src/main/thrift/confignode.thrift:
##########
@@ -211,6 +211,16 @@ struct TConfigNodeRegisterResp {
3: optional list<common.TConfigNodeLocation> configNodeList
}
+struct TConfigNodeConfigurationResp {
+ 1: required common.TSStatus status
+ 2: required common.TConsensusGroupId partitionRegionId
+ 3: required list<common.TConfigNodeLocation> configNodes
+ 4: required TGlobalConfig globalConfig
+ 5: required i64 defaultTTL
+ 6: required i32 schemaReplicationFactor
+ 7: required i32 dataReplicationFactor
Review Comment:
put these into TGlobalConfig
##########
confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeStartupCheck.java:
##########
@@ -405,6 +427,119 @@ private void createDirIfEmpty(File dir) throws IOException {
}
}
+ /** Check configuration */
+ public boolean checkConfigurations(TConsensusGroupId groupId) {
+ if (!isContainsRpcConfigNode()) {
+ return true;
+ }
+
+ List<TConfigNodeConfigurationResp> configNodeConfigurations = new ArrayList<>();
+ try {
+ List<TConfigNodeLocation> configNodeLocations =
+ NodeUrlUtils.parseTConfigNodeUrls(systemProperties.getProperty("confignode_list"));
+ for (TConfigNodeLocation nodeLocation : configNodeLocations) {
+ if (nodeLocation
+ .getInternalEndPoint()
+ .getIp()
+ .equals(systemProperties.getProperty("rpc_address"))) {
Review Comment:
Need to compare EndPoint, ip + port
##########
confignode/src/main/java/org/apache/iotdb/confignode/service/ConfigNode.java:
##########
@@ -133,6 +139,50 @@ public void active() {
"{} has successfully started and joined the cluster.", ConfigNodeConstant.GLOBAL_NAME);
}
+ private void checkSystemConfig() {
+ new Thread(
+ () -> {
+ if (!ConfigNodeStartupCheck.getInstance().isCheckAllConfigNodeParameter()) {
+ return;
+ }
+ try {
+ waitLeader();
+ TConsensusGroupId groupId =
+ configManager
+ .getConsensusManager()
+ .getConsensusGroupId()
+ .convertToTConsensusGroupId();
+ if (!ConfigNodeStartupCheck.getInstance().checkConfigurations(groupId)) {
+ stop();
+ LOGGER.info("Check configurations failed!");
+ return;
Review Comment:
```suggestion
```
##########
confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeStartupCheck.java:
##########
@@ -405,6 +427,119 @@ private void createDirIfEmpty(File dir) throws IOException {
}
}
+ /** Check configuration */
+ public boolean checkConfigurations(TConsensusGroupId groupId) {
Review Comment:
Merge with checkSystemProperties() in this class
##########
thrift-confignode/src/main/thrift/confignode.thrift:
##########
@@ -211,6 +211,16 @@ struct TConfigNodeRegisterResp {
3: optional list<common.TConfigNodeLocation> configNodeList
}
+struct TConfigNodeConfigurationResp {
+ 1: required common.TSStatus status
+ 2: required common.TConsensusGroupId partitionRegionId
Review Comment:
This is always 0, no need to return this
##########
confignode/src/test/resources/confignode2conf/iotdb-confignode.properties:
##########
@@ -20,7 +20,7 @@
rpc_address=0.0.0.0
rpc_port=22279
consensus_port=22280
-target_confignode=0.0.0.0:22277
+target_confignode=0.0.0.0:22277:22278
Review Comment:
Even if we add consensus port, need to make this be consistent with confignode/src/assembly/resources/conf/iotdb-confignode.properties
##########
confignode/src/main/java/org/apache/iotdb/confignode/service/ConfigNode.java:
##########
@@ -133,6 +139,50 @@ public void active() {
"{} has successfully started and joined the cluster.", ConfigNodeConstant.GLOBAL_NAME);
}
+ private void checkSystemConfig() {
+ new Thread(
+ () -> {
+ if (!ConfigNodeStartupCheck.getInstance().isCheckAllConfigNodeParameter()) {
+ return;
+ }
+ try {
+ waitLeader();
+ TConsensusGroupId groupId =
+ configManager
+ .getConsensusManager()
+ .getConsensusGroupId()
+ .convertToTConsensusGroupId();
+ if (!ConfigNodeStartupCheck.getInstance().checkConfigurations(groupId)) {
+ stop();
+ LOGGER.info("Check configurations failed!");
+ return;
+ }
+ } catch (IOException | InterruptedException e) {
+ LOGGER.error("Meet error when stop ConfigNode!", e);
+ }
+ })
+ .start();
Review Comment:
Using other threads to check is ok, but we should wait for the check result and then go on.
##########
confignode/src/main/java/org/apache/iotdb/confignode/persistence/NodeInfo.java:
##########
@@ -266,6 +272,55 @@ public List<TDataNodeInfo> getOnlineDataNodes(int dataNodeId) {
return result;
}
+ /**
+ * Get target ConfigNode configuration
+ *
+ * @param req GetConfigNodeConfigurationReq
+ * @return GET_CONFIGNODE_CONFIGURATION_FAILED if confignode-system.properties not exist
+ */
+ public ConfigNodeConfigurationResp getConfigNodeConfiguration(GetConfigNodeConfigurationReq req) {
+ ConfigNodeConfigurationResp result = new ConfigNodeConfigurationResp();
+ Properties systemProperties = new Properties();
+ configNodeInfoReadWriteLock.readLock().lock();
+
+ if (!systemPropertiesFile.exists()) {
+ LOGGER.info("The system properties file is not exists.");
+ result.setStatus(
+ new TSStatus(TSStatusCode.GET_CONFIGNODE_CONFIGURATION_FAILED.getStatusCode()));
+ return result;
+ }
+
+ try (FileInputStream inputStream = new FileInputStream(systemPropertiesFile)) {
+ systemProperties.load(inputStream);
+ result.setStatus(new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode()));
+ result.setConfigNodes(
+ NodeUrlUtils.parseTConfigNodeUrls(systemProperties.getProperty("confignode_list")));
+
+ ConfigNodeConfig conf = ConfigNodeDescriptor.getInstance().getConf();
+
+ TGlobalConfig globalConfig = new TGlobalConfig();
+ globalConfig.setDataRegionConsensusProtocolClass(
+ systemProperties.getProperty("data_region_consensus_protocol_class"));
+ globalConfig.setSchemaRegionConsensusProtocolClass(
+ systemProperties.getProperty("schema_region_consensus_protocol_class"));
+ globalConfig.setSeriesPartitionSlotNum(
+ Integer.parseInt(systemProperties.getProperty("series_partition_slot_num")));
+ globalConfig.setSeriesPartitionExecutorClass(
+ systemProperties.getProperty("series_partition_executor_class"));
+ globalConfig.setTimePartitionInterval(conf.getTimePartitionInterval());
+ result.setGlobalConfig(globalConfig);
+
+ result.setDefaultTTL(CommonDescriptor.getInstance().getConfig().getDefaultTTL());
+ result.setSchemaReplicationFactor(conf.getSchemaReplicationFactor());
+ result.setDataReplicationFactor(conf.getDataReplicationFactor());
Review Comment:
Put to TGlobalConfig
##########
confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeStartupCheck.java:
##########
@@ -163,21 +171,34 @@ private boolean isFirstStart() throws IOException {
* @return True if the target_confignode points to itself
*/
private boolean isSeedConfigNode() {
- boolean result =
- conf.getRpcAddress().equals(conf.getTargetConfigNode().getIp())
- && conf.getRpcPort() == conf.getTargetConfigNode().getPort();
+ boolean result = isContainsRpcConfigNode();
+
if (result) {
// TODO: Set PartitionRegionId from iotdb-confignode.properties
- conf.setConfigNodeList(
- Collections.singletonList(
- new TConfigNodeLocation(
- 0,
- new TEndPoint(conf.getRpcAddress(), conf.getRpcPort()),
- new TEndPoint(conf.getRpcAddress(), conf.getConsensusPort()))));
+ conf.setConfigNodeList(conf.getTargetConfigNodeList());
}
return result;
}
+ /**
+ * Check if the SeedConfigNode contains the current ConfigNode.
+ *
+ * @return True if the target_confignode points to itself
+ */
+ private boolean isContainsRpcConfigNode() {
Review Comment:
```suggestion
private boolean currentConfigNodeAlreadyRegistered() {
```
##########
confignode/src/test/resources/confignode3conf/iotdb-confignode.properties:
##########
@@ -20,7 +20,7 @@
rpc_address=0.0.0.0
rpc_port=22281
consensus_port=22282
-target_confignode=0.0.0.0:22277
+target_confignode=0.0.0.0:22277:22278
Review Comment:
Could we omit the consensus_port? Internal_port is enough.
##########
service-rpc/src/main/java/org/apache/iotdb/rpc/TSStatusCode.java:
##########
@@ -136,7 +136,8 @@ public enum TSStatusCode {
REGISTER_CONFIGNODE_FAILED(907),
REMOVE_CONFIGNODE_FAILED(908),
REMOVE_CONFIGNODE_DUPLICATION(909),
- STOP_CONOFIGNODE_FAILED(910);
+ STOP_CONOFIGNODE_FAILED(910),
+ GET_CONFIGNODE_CONFIGURATION_FAILED(911);
Review Comment:
```suggestion
SYSTEM_PROPERTIES_FILE_NOT_EXIST(911);
```
##########
confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeStartupCheck.java:
##########
@@ -163,21 +171,34 @@ private boolean isFirstStart() throws IOException {
* @return True if the target_confignode points to itself
*/
private boolean isSeedConfigNode() {
- boolean result =
- conf.getRpcAddress().equals(conf.getTargetConfigNode().getIp())
- && conf.getRpcPort() == conf.getTargetConfigNode().getPort();
+ boolean result = isContainsRpcConfigNode();
+
if (result) {
// TODO: Set PartitionRegionId from iotdb-confignode.properties
- conf.setConfigNodeList(
- Collections.singletonList(
- new TConfigNodeLocation(
- 0,
- new TEndPoint(conf.getRpcAddress(), conf.getRpcPort()),
- new TEndPoint(conf.getRpcAddress(), conf.getConsensusPort()))));
+ conf.setConfigNodeList(conf.getTargetConfigNodeList());
}
return result;
}
+ /**
+ * Check if the SeedConfigNode contains the current ConfigNode.
+ *
+ * @return True if the target_confignode points to itself
+ */
+ private boolean isContainsRpcConfigNode() {
+ AtomicBoolean result = new AtomicBoolean(false);
+ TEndPoint rpcConfigNode = new TEndPoint(conf.getRpcAddress(), conf.getRpcPort());
Review Comment:
```suggestion
TEndPoint currentConfigNode = new TEndPoint(conf.getRpcAddress(), conf.getRpcPort());
```
--
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