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 2021/01/20 06:49:20 UTC

[GitHub] [iotdb] neuyilan commented on a change in pull request #2526: IOTDB-1111 load configuration -global command do not support

neuyilan commented on a change in pull request #2526:
URL: https://github.com/apache/iotdb/pull/2526#discussion_r560713379



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPhysicalGenerator.java
##########
@@ -73,26 +73,29 @@ private CMManager getCMManager() {
     return ((CMManager) IoTDB.metaManager).getMatchedDevices(path);
   }
 
-
   @Override
   protected PhysicalPlan generateLoadConfigurationPlan(LoadConfigurationOperatorType type)
       throws QueryProcessException {
     if (type == LoadConfigurationOperatorType.GLOBAL) {
       Properties[] properties = new Properties[2];
       properties[0] = new Properties();
       URL iotdbEnginePropertiesUrl = IoTDBDescriptor.getInstance().getPropsUrl();
-      try (InputStream inputStream = new FileInputStream(new File(iotdbEnginePropertiesUrl.toString()))) {
+      if (iotdbEnginePropertiesUrl == null) {
+        logger.error("Fail to find config file {}");

Review comment:
       ```suggestion
           logger.error("Fail to find the engine config file");
   ```

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPhysicalGenerator.java
##########
@@ -73,26 +73,29 @@ private CMManager getCMManager() {
     return ((CMManager) IoTDB.metaManager).getMatchedDevices(path);
   }
 
-
   @Override
   protected PhysicalPlan generateLoadConfigurationPlan(LoadConfigurationOperatorType type)
       throws QueryProcessException {
     if (type == LoadConfigurationOperatorType.GLOBAL) {
       Properties[] properties = new Properties[2];
       properties[0] = new Properties();
       URL iotdbEnginePropertiesUrl = IoTDBDescriptor.getInstance().getPropsUrl();
-      try (InputStream inputStream = new FileInputStream(new File(iotdbEnginePropertiesUrl.toString()))) {
+      if (iotdbEnginePropertiesUrl == null) {
+        logger.error("Fail to find config file {}");
+        throw new QueryProcessException("Fail to find config file");

Review comment:
       the same as above

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPhysicalGenerator.java
##########
@@ -73,26 +73,29 @@ private CMManager getCMManager() {
     return ((CMManager) IoTDB.metaManager).getMatchedDevices(path);
   }
 
-
   @Override
   protected PhysicalPlan generateLoadConfigurationPlan(LoadConfigurationOperatorType type)
       throws QueryProcessException {
     if (type == LoadConfigurationOperatorType.GLOBAL) {
       Properties[] properties = new Properties[2];
       properties[0] = new Properties();
       URL iotdbEnginePropertiesUrl = IoTDBDescriptor.getInstance().getPropsUrl();
-      try (InputStream inputStream = new FileInputStream(new File(iotdbEnginePropertiesUrl.toString()))) {
+      if (iotdbEnginePropertiesUrl == null) {
+        logger.error("Fail to find config file {}");
+        throw new QueryProcessException("Fail to find config file");
+      }
+      try (InputStream inputStream = iotdbEnginePropertiesUrl.openStream()) {
         properties[0].load(inputStream);
       } catch (IOException e) {
-        logger.warn("Fail to find config file {}", iotdbEnginePropertiesUrl, e);
+        logger.error("Fail to find config file {}", iotdbEnginePropertiesUrl, e);
         throw new QueryProcessException("Fail to find iotdb-engine config file.");
       }
       String clusterPropertiesUrl = ClusterDescriptor.getInstance().getPropsUrl();
       properties[1] = new Properties();
       try (InputStream inputStream = new FileInputStream(new File(clusterPropertiesUrl))) {
         properties[1].load(inputStream);
       } catch (IOException e) {
-        logger.warn("Fail to find config file {}", clusterPropertiesUrl, e);
+        logger.error("Fail to find config file {}", clusterPropertiesUrl, e);

Review comment:
       the same as above

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPhysicalGenerator.java
##########
@@ -73,26 +73,29 @@ private CMManager getCMManager() {
     return ((CMManager) IoTDB.metaManager).getMatchedDevices(path);
   }
 
-
   @Override
   protected PhysicalPlan generateLoadConfigurationPlan(LoadConfigurationOperatorType type)
       throws QueryProcessException {
     if (type == LoadConfigurationOperatorType.GLOBAL) {
       Properties[] properties = new Properties[2];
       properties[0] = new Properties();
       URL iotdbEnginePropertiesUrl = IoTDBDescriptor.getInstance().getPropsUrl();
-      try (InputStream inputStream = new FileInputStream(new File(iotdbEnginePropertiesUrl.toString()))) {
+      if (iotdbEnginePropertiesUrl == null) {
+        logger.error("Fail to find config file {}");
+        throw new QueryProcessException("Fail to find config file");
+      }
+      try (InputStream inputStream = iotdbEnginePropertiesUrl.openStream()) {
         properties[0].load(inputStream);
       } catch (IOException e) {
-        logger.warn("Fail to find config file {}", iotdbEnginePropertiesUrl, e);
+        logger.error("Fail to find config file {}", iotdbEnginePropertiesUrl, e);

Review comment:
       the error should be more clear, this may due to open the engine file failed, rather than can not find the config file.




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

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