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/07/07 07:21:34 UTC

[GitHub] [iotdb] wangchao316 opened a new pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

wangchao316 opened a new pull request #3292:
URL: https://github.com/apache/iotdb/pull/3292


   https://issues.apache.org/jira/browse/IOTDB-1407
   5 node , 3 rep.
   
   1. Connect to the client.
   
   2. Run the show timeseries root.turbine where owner=user1 command.
   throw :   The key owner is not  a tag.
   
   reason:
   When the number of nodes is greater than the number of replicas, querying a sequence label is broadcast to all DataGroups. Some DataGroups do not contain the sequence. Therefore, an error is reported when querying the label. As a result, an error is reported for the entire query.
   
   solved:
   When the root sequence is queried, all DataGroup queries are broadcast.
   When a subsequence is queried, the data group of the corresponding subsequence is queried.


-- 
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] mychaow commented on a change in pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1690,7 +1690,15 @@ public MNode getMNode(MNode deviceMNode, String measurementName) {
     ExecutorService pool =
         new ThreadPoolExecutor(
             THREAD_POOL_SIZE, THREAD_POOL_SIZE, 0, TimeUnit.SECONDS, new LinkedBlockingDeque<>());
-    List<PartitionGroup> globalGroups = metaGroupMember.getPartitionTable().getGlobalGroups();
+
+    List<PartitionGroup> globalGroups = new ArrayList<>();
+    if (IoTDBConstant.PATH_ROOT.equals(plan.getPath().getFullPath())) {
+      globalGroups = metaGroupMember.getPartitionTable().getGlobalGroups();
+    } else {
+      PartitionGroup partitionGroup =
+          metaGroupMember.getPartitionTable().partitionByPathTime(plan.getPath(), 0);

Review comment:
       show timeseries root.* is not right?




-- 
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] [iotdb] wangchao316 commented on pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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


   @LebronAl @neuyilan @mychaow , hi ,could you please reivew?
   Thanks..


-- 
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] [iotdb] coveralls edited a comment on pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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


   
   [![Coverage Status](https://coveralls.io/builds/41177994/badge)](https://coveralls.io/builds/41177994)
   
   Coverage increased (+0.02%) to 68.123% when pulling **bc8035b4574cf1f06f2dc114a42a6531fe236ebd on wangchao316:IOTDB-1407** into **f1bc4e32ec11bc36dbae153e8b6a36e3a5505413 on apache: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] wangchao316 commented on a change in pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1717,7 +1726,7 @@ public MNode getMNode(MNode deviceMNode, String measurementName) {
               () -> {
                 try {
                   showTimeseries(group, plan, resultSet, context);
-                } catch (CheckConsistencyException e) {
+                } catch (CheckConsistencyException | MetadataException e) {

Review comment:
       1. We send requests to all DataGroups only for sequences that are not found in partitiontable.
   2. The general metadata exception is because the DataGroup does not contain the sequence. Therefore, you can skip the metadata exception by default. add only the queried sequences.




-- 
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] [iotdb] LebronAl commented on a change in pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1717,7 +1726,7 @@ public MNode getMNode(MNode deviceMNode, String measurementName) {
               () -> {
                 try {
                   showTimeseries(group, plan, resultSet, context);
-                } catch (CheckConsistencyException e) {
+                } catch (CheckConsistencyException | MetadataException e) {

Review comment:
       I feel like the previous design here was to ignore all MetaExceptions and read as many as we could.
   If we had no intention of changing the design, it would seem pointless to catch this exception?

##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -220,4 +220,61 @@ public void vectorCountTest() throws IoTDBConnectionException, StatementExecutio
     Assert.assertEquals(10, next.getFields().get(0).getLongV());
     Assert.assertEquals(10, next.getFields().get(1).getLongV());
   }
+
+  // test https://issues.apache.org/jira/browse/IOTDB-1407
+  @Test
+  public void showTimeseriesTagsTest() throws SQLException {
+    String createTimeSeries1 =
+        "create timeseries root.ln.wf01.wt1 WITH DATATYPE=DOUBLE, ENCODING=RLE, compression=SNAPPY tags(tag1=v1, tag2=v2)";
+    String createTimeSeries2 =
+        "create timeseries root.ln.wf01.wt2 WITH DATATYPE=DOUBLE, ENCODING=RLE, compression=SNAPPY tags(tag1=v1, tag2=v2)";
+    writeStatement.execute(createTimeSeries1);
+    writeStatement.execute(createTimeSeries2);
+    // try to read data on each node. select .*
+    for (Statement readStatement : readStatements) {

Review comment:
       what about adding some corner cases,such as `SHOW TIMESERIES root.ln.wf01.* where tag1=v3`, `HOW TIMESERIES root.ln.wf01.* where tag3=v1`




-- 
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] [iotdb] mychaow commented on a change in pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1717,7 +1726,7 @@ public MNode getMNode(MNode deviceMNode, String measurementName) {
               () -> {
                 try {
                   showTimeseries(group, plan, resultSet, context);
-                } catch (CheckConsistencyException e) {
+                } catch (CheckConsistencyException | MetadataException e) {

Review comment:
       LGTM. But there is one minor issues.  This will ignore some errors, resulting in query results have problems. Like we execute  show timeseries, some data group failed with other errors exclude tag not found, we will return result. 




-- 
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] [iotdb] wangchao316 commented on a change in pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1690,7 +1690,15 @@ public MNode getMNode(MNode deviceMNode, String measurementName) {
     ExecutorService pool =
         new ThreadPoolExecutor(
             THREAD_POOL_SIZE, THREAD_POOL_SIZE, 0, TimeUnit.SECONDS, new LinkedBlockingDeque<>());
-    List<PartitionGroup> globalGroups = metaGroupMember.getPartitionTable().getGlobalGroups();
+
+    List<PartitionGroup> globalGroups = new ArrayList<>();
+    if (IoTDBConstant.PATH_ROOT.equals(plan.getPath().getFullPath())) {
+      globalGroups = metaGroupMember.getPartitionTable().getGlobalGroups();
+    } else {
+      PartitionGroup partitionGroup =
+          metaGroupMember.getPartitionTable().partitionByPathTime(plan.getPath(), 0);

Review comment:
       Thanks @mychaow .  this is have a situation,  root.a.*
   I  will alter this.




-- 
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] [iotdb] wangchao316 commented on a change in pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1690,7 +1690,16 @@ public MNode getMNode(MNode deviceMNode, String measurementName) {
     ExecutorService pool =
         new ThreadPoolExecutor(
             THREAD_POOL_SIZE, THREAD_POOL_SIZE, 0, TimeUnit.SECONDS, new LinkedBlockingDeque<>());
-    List<PartitionGroup> globalGroups = metaGroupMember.getPartitionTable().getGlobalGroups();
+
+    List<PartitionGroup> globalGroups = new ArrayList<>();
+    if (IoTDBConstant.PATH_ROOT.equals(plan.getPath().getFullPath())
+        || plan.getPath().getFullPath().contains(IoTDBConstant.PATH_WILDCARD)) {

Review comment:
       This grammar....  I alter this, support this grammar...




-- 
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] [iotdb] wangchao316 commented on a change in pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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



##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -220,4 +220,61 @@ public void vectorCountTest() throws IoTDBConnectionException, StatementExecutio
     Assert.assertEquals(10, next.getFields().get(0).getLongV());
     Assert.assertEquals(10, next.getFields().get(1).getLongV());
   }
+
+  // test https://issues.apache.org/jira/browse/IOTDB-1407
+  @Test
+  public void showTimeseriesTagsTest() throws SQLException {
+    String createTimeSeries1 =
+        "create timeseries root.ln.wf01.wt1 WITH DATATYPE=DOUBLE, ENCODING=RLE, compression=SNAPPY tags(tag1=v1, tag2=v2)";
+    String createTimeSeries2 =
+        "create timeseries root.ln.wf01.wt2 WITH DATATYPE=DOUBLE, ENCODING=RLE, compression=SNAPPY tags(tag1=v1, tag2=v2)";
+    writeStatement.execute(createTimeSeries1);
+    writeStatement.execute(createTimeSeries2);
+    // try to read data on each node. select .*
+    for (Statement readStatement : readStatements) {

Review comment:
       Thanks, done.




-- 
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 #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1717,7 +1726,7 @@ public MNode getMNode(MNode deviceMNode, String measurementName) {
               () -> {
                 try {
                   showTimeseries(group, plan, resultSet, context);
-                } catch (CheckConsistencyException e) {
+                } catch (CheckConsistencyException | MetadataException e) {

Review comment:
       Thanks , I alter this exception , Same as before.




-- 
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 #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1717,7 +1726,7 @@ public MNode getMNode(MNode deviceMNode, String measurementName) {
               () -> {
                 try {
                   showTimeseries(group, plan, resultSet, context);
-                } catch (CheckConsistencyException e) {
+                } catch (CheckConsistencyException | MetadataException e) {

Review comment:
       1. We send requests to all DataGroups only for sequences that are not found in partitiontable.
   2. The general metadata exception is because the DataGroup does not contain the sequence. Therefore, you can skip the metadata exception by default. add only the queried sequences.




-- 
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] [iotdb] wangchao316 closed pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

Posted by GitBox <gi...@apache.org>.
wangchao316 closed pull request #3292:
URL: https://github.com/apache/iotdb/pull/3292


   


-- 
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] LebronAl commented on pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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


   Hi, it's better to add some ITs in testContainer for this function


-- 
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] [iotdb] mychaow merged pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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


   


-- 
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 closed pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

Posted by GitBox <gi...@apache.org>.
wangchao316 closed pull request #3292:
URL: https://github.com/apache/iotdb/pull/3292


   


-- 
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 #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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


   
   [![Coverage Status](https://coveralls.io/builds/41174900/badge)](https://coveralls.io/builds/41174900)
   
   Coverage increased (+0.005%) to 68.111% when pulling **6764b75aeaffcd65a9e0610e7557e476f0851a6b on wangchao316:IOTDB-1407** into **f1bc4e32ec11bc36dbae153e8b6a36e3a5505413 on apache: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] wangchao316 commented on pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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


   > Hi, it's better to add some ITs in testContainer for this function
   
   hi , I add some IT in test Container, please review...  thanks.


-- 
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] [iotdb] neuyilan commented on a change in pull request #3292: [IOTDB-1407] Filtering time series based on tags query fails Occasion…

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



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1690,7 +1690,16 @@ public MNode getMNode(MNode deviceMNode, String measurementName) {
     ExecutorService pool =
         new ThreadPoolExecutor(
             THREAD_POOL_SIZE, THREAD_POOL_SIZE, 0, TimeUnit.SECONDS, new LinkedBlockingDeque<>());
-    List<PartitionGroup> globalGroups = metaGroupMember.getPartitionTable().getGlobalGroups();
+
+    List<PartitionGroup> globalGroups = new ArrayList<>();
+    if (IoTDBConstant.PATH_ROOT.equals(plan.getPath().getFullPath())
+        || plan.getPath().getFullPath().contains(IoTDBConstant.PATH_WILDCARD)) {

Review comment:
       As we have supported special characters, how about  the timeseries contains asterisk like the following:
   `root.sg."device_1*".sensor1`




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