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 2020/09/07 09:51:27 UTC

[GitHub] [incubator-iotdb] neuyilan opened a new pull request #1700: [IOTDB-875]fix flush storage group error when no data in storage group

neuyilan opened a new pull request #1700:
URL: https://github.com/apache/incubator-iotdb/pull/1700


   


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



[GitHub] [incubator-iotdb] yuqi1129 commented on a change in pull request #1700: [IOTDB-875]fix flush storage group error when no data in storage group

Posted by GitBox <gi...@apache.org>.
yuqi1129 commented on a change in pull request #1700:
URL: https://github.com/apache/incubator-iotdb/pull/1700#discussion_r484439096



##########
File path: server/src/test/java/org/apache/iotdb/db/integration/IoTDBFlushQueryMergeIT.java
##########
@@ -154,4 +154,24 @@ public void testFlushGivenGroup() throws ClassNotFoundException {
       fail(e.getMessage());
     }
   }
+
+  // bug fix test, https://issues.apache.org/jira/browse/IOTDB-875
+  @Test
+  public void testFlushGivenGroupNoData() throws ClassNotFoundException {
+    Class.forName(Config.JDBC_DRIVER_NAME);
+    try (Connection connection = DriverManager
+        .getConnection(Config.IOTDB_URL_PREFIX + "127.0.0.1:6667/", "root", "root");
+        Statement statement = connection.createStatement()) {
+      statement.execute("SET STORAGE GROUP TO root.nodatagroup1");
+      statement.execute("SET STORAGE GROUP TO root.nodatagroup2");
+      statement.execute("SET STORAGE GROUP TO root.nodatagroup3");
+      statement.execute("FLUSH root.nodatagroup1");
+      statement.execute("FLUSH root.nodatagroup2");
+      statement.execute("FLUSH root.nodatagroup3");
+    } catch (Exception e) {
+      e.printStackTrace();

Review comment:
       `e.printStackTrace()` can be replaced with logger




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



[GitHub] [incubator-iotdb] neuyilan commented on a change in pull request #1700: [IOTDB-875]fix flush storage group error when no data in storage group

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



##########
File path: server/src/test/java/org/apache/iotdb/db/integration/IoTDBFlushQueryMergeIT.java
##########
@@ -154,4 +154,24 @@ public void testFlushGivenGroup() throws ClassNotFoundException {
       fail(e.getMessage());
     }
   }
+
+  // bug fix test, https://issues.apache.org/jira/browse/IOTDB-875
+  @Test
+  public void testFlushGivenGroupNoData() throws ClassNotFoundException {
+    Class.forName(Config.JDBC_DRIVER_NAME);
+    try (Connection connection = DriverManager
+        .getConnection(Config.IOTDB_URL_PREFIX + "127.0.0.1:6667/", "root", "root");
+        Statement statement = connection.createStatement()) {
+      statement.execute("SET STORAGE GROUP TO root.nodatagroup1");
+      statement.execute("SET STORAGE GROUP TO root.nodatagroup2");
+      statement.execute("SET STORAGE GROUP TO root.nodatagroup3");
+      statement.execute("FLUSH root.nodatagroup1");
+      statement.execute("FLUSH root.nodatagroup2");
+      statement.execute("FLUSH root.nodatagroup3");
+    } catch (Exception e) {
+      e.printStackTrace();

Review comment:
       Thanks for your friendly reminder.




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



[GitHub] [incubator-iotdb] neuyilan commented on a change in pull request #1700: [IOTDB-875]fix flush storage group error when no data in storage group

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



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/StorageEngine.java
##########
@@ -394,6 +413,15 @@ public void asyncCloseProcessor(PartialPath storageGroupPath, boolean isSeq)
         processor.writeUnlock();
       }
     } else {
+      // bug fix, for storage group do does not have data
+      // https://issues.apache.org/jira/browse/IOTDB-875
+      List<PartialPath> allStorageGroupPaths = MManager.getInstance()

Review comment:
       sure, thanks for your advice.




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



[GitHub] [incubator-iotdb] Genius-pig commented on a change in pull request #1700: [IOTDB-875]fix flush storage group error when no data in storage group

Posted by GitBox <gi...@apache.org>.
Genius-pig commented on a change in pull request #1700:
URL: https://github.com/apache/incubator-iotdb/pull/1700#discussion_r484386580



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/StorageEngine.java
##########
@@ -394,6 +413,15 @@ public void asyncCloseProcessor(PartialPath storageGroupPath, boolean isSeq)
         processor.writeUnlock();
       }
     } else {
+      // bug fix, for storage group do does not have data
+      // https://issues.apache.org/jira/browse/IOTDB-875
+      List<PartialPath> allStorageGroupPaths = MManager.getInstance()

Review comment:
       can you use `getStorageGroupPath` instead of getting all storage groups?




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



[GitHub] [incubator-iotdb] qiaojialin merged pull request #1700: [IOTDB-875]fix flush storage group error when no data in storage group

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


   


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