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/19 14:35:50 UTC
[GitHub] [iotdb] wangchao316 opened a new pull request #5287: [IOTDB-2730] start config service
wangchao316 opened a new pull request #5287:
URL: https://github.com/apache/iotdb/pull/5287
start config servcie
--
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 #5287: [IOTDB-2730] start config service
Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #5287:
URL: https://github.com/apache/iotdb/pull/5287#discussion_r830605460
##########
File path: thrift-confignode/src/main/thrift/confignode.thrift
##########
@@ -21,7 +21,20 @@ include "rpc.thrift"
namespace java org.apache.iotdb.confignode.rpc.thrift
namespace py iotdb.thrift.confignode
-struct SetStorageGroupoReq {
+struct EndPoint {
+ 1: required string ip
+ 2: required i32 port
+}
+
+// The return status code and message in each response.
+struct TSStatus {
Review comment:
Then can we rename this TSStatus as ConfigNodeStatus in order to distinguish between the TSStatus in rpc.thrift?
--
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 #5287: [IOTDB-2730] start config service
Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #5287:
URL: https://github.com/apache/iotdb/pull/5287#discussion_r830542782
##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeDescriptor.java
##########
@@ -42,42 +43,52 @@ public ConfigNodeConf getConf() {
return conf;
}
- public String getPropsDir() {
Review comment:
Why delete this method? For distinguish ConfigNode's environment variables from DataNode's, I set CONFIGNODE_NODE_CONF and CONFIGNODE_NODE_HOME in the start-confignode scripts. Now we can see that the scripts output "WARN org.apache.iotdb.confignode.conf.ConfigNodeDescriptor - Cannot find IOTDB_HOME or IOTDB_CONF environment variable when loading config file iotdb-confignode.properties, use default configuration". This is a bug needs to be fixed.
##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeConfCheck.java
##########
@@ -44,17 +45,13 @@
private Properties specialProperties;
- public void checkConfig() throws RepeatConfigurationException, IOException {
-
- String propsDir = ConfigNodeDescriptor.getInstance().getPropsDir();
- if (propsDir == null) {
- // Skip configuration check when developer mode or test mode
- return;
- }
+ private ConfigNodeConfCheck() {
specialProperties = new Properties();
+ }
+ public void checkConfig() throws ConfigurationException, IOException, StartupException {
File specialPropertiesFile =
- new File(propsDir + File.separator + ConfigNodeConstant.SPECIAL_CONF_NAME);
+ new File(conf.getSystemDir() + File.separator + ConfigNodeConstant.SPECIAL_CONF_NAME);
Review comment:
It's ok for placing the special conf file to the system dir. But the system dir needs to be created first.
--
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 #5287: [IOTDB-2730] start config service
Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #5287:
URL: https://github.com/apache/iotdb/pull/5287#discussion_r830704890
##########
File path: thrift-confignode/src/main/thrift/confignode.thrift
##########
@@ -53,12 +53,37 @@ struct DataPartitionInfo {
2: required map<i32, list<i32>> dataRegionIDsMap
}
+struct DeviceGroupHashInfo {
+ 1: required i32 deviceGroupCount
+ 2: required string hashClass
+}
+
+struct DataNodeRegisterReq {
+ 1: required rpc.EndPoint endPoint
+}
+
+struct DataNodeRegisterResp {
Review comment:
How about fix it in next pr? There must exist other parameters that needs to be returned in DataNode registration.
--
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] wangchao316 commented on a change in pull request #5287: [IOTDB-2730] start config service
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #5287:
URL: https://github.com/apache/iotdb/pull/5287#discussion_r830599564
##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeDescriptor.java
##########
@@ -42,42 +43,52 @@ public ConfigNodeConf getConf() {
return conf;
}
- public String getPropsDir() {
Review comment:
fixed
##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeConfCheck.java
##########
@@ -44,17 +45,13 @@
private Properties specialProperties;
- public void checkConfig() throws RepeatConfigurationException, IOException {
-
- String propsDir = ConfigNodeDescriptor.getInstance().getPropsDir();
- if (propsDir == null) {
- // Skip configuration check when developer mode or test mode
- return;
- }
+ private ConfigNodeConfCheck() {
specialProperties = new Properties();
+ }
+ public void checkConfig() throws ConfigurationException, IOException, StartupException {
File specialPropertiesFile =
- new File(propsDir + File.separator + ConfigNodeConstant.SPECIAL_CONF_NAME);
+ new File(conf.getSystemDir() + File.separator + ConfigNodeConstant.SPECIAL_CONF_NAME);
Review comment:
fixed
--
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] wangchao316 commented on a change in pull request #5287: [IOTDB-2730] start config service
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #5287:
URL: https://github.com/apache/iotdb/pull/5287#discussion_r830599743
##########
File path: thrift-confignode/src/main/thrift/confignode.thrift
##########
@@ -21,7 +21,20 @@ include "rpc.thrift"
namespace java org.apache.iotdb.confignode.rpc.thrift
namespace py iotdb.thrift.confignode
-struct SetStorageGroupoReq {
+struct EndPoint {
+ 1: required string ip
+ 2: required i32 port
+}
+
+// The return status code and message in each response.
+struct TSStatus {
Review comment:
if delete struct TSStatus, compile will be failed. include "rpc.thrift" does not take effect.
--
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] OneSizeFitsQuorum commented on a change in pull request #5287: [IOTDB-2730] start config service
Posted by GitBox <gi...@apache.org>.
OneSizeFitsQuorum commented on a change in pull request #5287:
URL: https://github.com/apache/iotdb/pull/5287#discussion_r830619072
##########
File path: thrift-confignode/src/main/thrift/confignode.thrift
##########
@@ -53,12 +53,37 @@ struct DataPartitionInfo {
2: required map<i32, list<i32>> dataRegionIDsMap
}
+struct DeviceGroupHashInfo {
+ 1: required i32 deviceGroupCount
+ 2: required string hashClass
+}
+
+struct DataNodeRegisterReq {
+ 1: required rpc.EndPoint endPoint
+}
+
+struct DataNodeRegisterResp {
Review comment:
Maybe we can also return `DeviceGroupHashInfo` in this struct to save one extra rpc?
--
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] wangchao316 commented on a change in pull request #5287: [IOTDB-2730] start config service
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #5287:
URL: https://github.com/apache/iotdb/pull/5287#discussion_r830592732
##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeConf.java
##########
@@ -18,18 +18,21 @@
*/
package org.apache.iotdb.confignode.conf;
+import org.apache.iotdb.commons.conf.IoTDBConstant;
Review comment:
We put the same constants into IoTDBConstant， Different constants are stored separately
--
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] OneSizeFitsQuorum merged pull request #5287: [IOTDB-2730] start config service
Posted by GitBox <gi...@apache.org>.
OneSizeFitsQuorum merged pull request #5287:
URL: https://github.com/apache/iotdb/pull/5287
--
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 #5287: [IOTDB-2730] start config service
Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #5287:
URL: https://github.com/apache/iotdb/pull/5287#discussion_r830544583
##########
File path: thrift-confignode/src/main/thrift/confignode.thrift
##########
@@ -21,7 +21,20 @@ include "rpc.thrift"
namespace java org.apache.iotdb.confignode.rpc.thrift
namespace py iotdb.thrift.confignode
-struct SetStorageGroupoReq {
+struct EndPoint {
+ 1: required string ip
+ 2: required i32 port
+}
+
+// The return status code and message in each response.
+struct TSStatus {
Review comment:
The TSStatus is already defined in org.apache.iotdb.service.rpc.thrift. It can be just included. If there is a necessity to define a different Status code for ConfigNode, it should be renamed.
--
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 #5287: [IOTDB-2730] start config service
Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on a change in pull request #5287:
URL: https://github.com/apache/iotdb/pull/5287#discussion_r830542184
##########
File path: confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeConf.java
##########
@@ -18,18 +18,21 @@
*/
package org.apache.iotdb.confignode.conf;
+import org.apache.iotdb.commons.conf.IoTDBConstant;
Review comment:
Constants between DataNode and ConfigNode might different. Is that a good idea for blend them with a single IoTDBConstant class?
--
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