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/03/02 04:13:38 UTC

[GitHub] [iotdb] Alima777 opened a new pull request #2759: [ISSUE-2758] NullPointerException in QueryTimeManager.checkQueryAlive()

Alima777 opened a new pull request #2759:
URL: https://github.com/apache/iotdb/pull/2759


   #2758 


----------------------------------------------------------------
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] sonarcloud[bot] commented on pull request #2759: [ISSUE-2758] NullPointerException in QueryTimeManager.checkQueryAlive()

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2759:
URL: https://github.com/apache/iotdb/pull/2759#issuecomment-789610181


   Kudos, SonarCloud Quality Gate passed!
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90.png' alt='90.9%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2759&metric=new_coverage&view=list) [90.9% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2759&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2759&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2759&metric=new_duplicated_lines_density&view=list)
   
   


----------------------------------------------------------------
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] sonarcloud[bot] commented on pull request #2759: [ISSUE-2758] NullPointerException in QueryTimeManager.checkQueryAlive()

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2759:
URL: https://github.com/apache/iotdb/pull/2759#issuecomment-788618325


   Kudos, SonarCloud Quality Gate passed!
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90.png' alt='90.9%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2759&metric=new_coverage&view=list) [90.9% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2759&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2759&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2759&metric=new_duplicated_lines_density&view=list)
   
   


----------------------------------------------------------------
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] qiaojialin closed pull request #2759: [ISSUE-2758] NullPointerException in QueryTimeManager.checkQueryAlive()

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


   


----------------------------------------------------------------
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] qiaojialin commented on pull request #2759: [ISSUE-2758] NullPointerException in QueryTimeManager.checkQueryAlive()

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


   add more description and resubmit


----------------------------------------------------------------
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] sonarcloud[bot] removed a comment on pull request #2759: [ISSUE-2758] NullPointerException in QueryTimeManager.checkQueryAlive()

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #2759:
URL: https://github.com/apache/iotdb/pull/2759#issuecomment-788618325


   Kudos, SonarCloud Quality Gate passed!
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2759&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90.png' alt='90.9%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2759&metric=new_coverage&view=list) [90.9% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2759&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2759&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2759&metric=new_duplicated_lines_density&view=list)
   
   


----------------------------------------------------------------
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] Alima777 commented on a change in pull request #2759: [ISSUE-2758] NullPointerException in QueryTimeManager.checkQueryAlive()

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



##########
File path: server/src/main/java/org/apache/iotdb/db/query/control/QueryTimeManager.java
##########
@@ -90,20 +90,18 @@ public AtomicBoolean unRegisterQuery(long queryId) {
         queryId,
         (k, v) -> {
           successRemoved.set(true);
-          return null;
-        });
-    queryScheduledTaskMap.computeIfPresent(
-        queryId,
-        (k, v) -> {
-          queryScheduledTaskMap.get(queryId).cancel(false);
+          ScheduledFuture<?> scheduledFuture = queryScheduledTaskMap.remove(queryId);
+          if (scheduledFuture != null) {
+            scheduledFuture.cancel(false);
+          }
           return null;
         });
     return successRemoved;
   }
 
   public static void checkQueryAlive(long queryId) {
-    if (getInstance().queryInfoMap.get(queryId) != null
-        && getInstance().queryInfoMap.get(queryId).isInterrupted()) {
+    QueryInfo queryInfo = getInstance().queryInfoMap.get(queryId);
+    if (queryInfo != null && queryInfo.isInterrupted()) {
       if (getInstance().unRegisterQuery(queryId).get()) {

Review comment:
       It may return false but never return null. Am i wrong?

##########
File path: server/src/main/java/org/apache/iotdb/db/query/control/QueryTimeManager.java
##########
@@ -90,20 +90,18 @@ public AtomicBoolean unRegisterQuery(long queryId) {
         queryId,
         (k, v) -> {
           successRemoved.set(true);
-          return null;
-        });
-    queryScheduledTaskMap.computeIfPresent(
-        queryId,
-        (k, v) -> {
-          queryScheduledTaskMap.get(queryId).cancel(false);
+          ScheduledFuture<?> scheduledFuture = queryScheduledTaskMap.remove(queryId);
+          if (scheduledFuture != null) {
+            scheduledFuture.cancel(false);
+          }
           return null;
         });
     return successRemoved;
   }
 
   public static void checkQueryAlive(long queryId) {
-    if (getInstance().queryInfoMap.get(queryId) != null
-        && getInstance().queryInfoMap.get(queryId).isInterrupted()) {
+    QueryInfo queryInfo = getInstance().queryInfoMap.get(queryId);
+    if (queryInfo != null && queryInfo.isInterrupted()) {
       if (getInstance().unRegisterQuery(queryId).get()) {

Review comment:
       It may return false but never return null. Am i wrong?




----------------------------------------------------------------
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 #2759: [ISSUE-2758] NullPointerException in QueryTimeManager.checkQueryAlive()

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



##########
File path: server/src/main/java/org/apache/iotdb/db/query/control/QueryTimeManager.java
##########
@@ -90,20 +90,18 @@ public AtomicBoolean unRegisterQuery(long queryId) {
         queryId,
         (k, v) -> {
           successRemoved.set(true);
-          return null;
-        });
-    queryScheduledTaskMap.computeIfPresent(
-        queryId,
-        (k, v) -> {
-          queryScheduledTaskMap.get(queryId).cancel(false);
+          ScheduledFuture<?> scheduledFuture = queryScheduledTaskMap.remove(queryId);
+          if (scheduledFuture != null) {
+            scheduledFuture.cancel(false);
+          }
           return null;

Review comment:
       maybe return null.

##########
File path: server/src/main/java/org/apache/iotdb/db/query/control/QueryTimeManager.java
##########
@@ -90,20 +90,18 @@ public AtomicBoolean unRegisterQuery(long queryId) {
         queryId,
         (k, v) -> {
           successRemoved.set(true);
-          return null;
-        });
-    queryScheduledTaskMap.computeIfPresent(
-        queryId,
-        (k, v) -> {
-          queryScheduledTaskMap.get(queryId).cancel(false);
+          ScheduledFuture<?> scheduledFuture = queryScheduledTaskMap.remove(queryId);
+          if (scheduledFuture != null) {
+            scheduledFuture.cancel(false);
+          }
           return null;
         });
     return successRemoved;
   }
 
   public static void checkQueryAlive(long queryId) {
-    if (getInstance().queryInfoMap.get(queryId) != null
-        && getInstance().queryInfoMap.get(queryId).isInterrupted()) {
+    QueryInfo queryInfo = getInstance().queryInfoMap.get(queryId);
+    if (queryInfo != null && queryInfo.isInterrupted()) {
       if (getInstance().unRegisterQuery(queryId).get()) {

Review comment:
       hi,   getInstance().unRegisterQuery(queryId) maybe return null, so #get() will throw nullpointerexception




----------------------------------------------------------------
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 #2759: [ISSUE-2758] NullPointerException in QueryTimeManager.checkQueryAlive()

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



##########
File path: server/src/main/java/org/apache/iotdb/db/query/control/QueryTimeManager.java
##########
@@ -90,20 +90,18 @@ public AtomicBoolean unRegisterQuery(long queryId) {
         queryId,
         (k, v) -> {
           successRemoved.set(true);
-          return null;
-        });
-    queryScheduledTaskMap.computeIfPresent(
-        queryId,
-        (k, v) -> {
-          queryScheduledTaskMap.get(queryId).cancel(false);
+          ScheduledFuture<?> scheduledFuture = queryScheduledTaskMap.remove(queryId);
+          if (scheduledFuture != null) {
+            scheduledFuture.cancel(false);
+          }
           return null;
         });
     return successRemoved;
   }
 
   public static void checkQueryAlive(long queryId) {
-    if (getInstance().queryInfoMap.get(queryId) != null
-        && getInstance().queryInfoMap.get(queryId).isInterrupted()) {
+    QueryInfo queryInfo = getInstance().queryInfoMap.get(queryId);
+    if (queryInfo != null && queryInfo.isInterrupted()) {
       if (getInstance().unRegisterQuery(queryId).get()) {

Review comment:
       sorry, I see fault..   map k-v return null...  
   unRegisterQuery return boolean..
   you are 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 a change in pull request #2759: [ISSUE-2758] NullPointerException in QueryTimeManager.checkQueryAlive()

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



##########
File path: server/src/main/java/org/apache/iotdb/db/query/control/QueryTimeManager.java
##########
@@ -90,20 +90,18 @@ public AtomicBoolean unRegisterQuery(long queryId) {
         queryId,
         (k, v) -> {
           successRemoved.set(true);
-          return null;
-        });
-    queryScheduledTaskMap.computeIfPresent(
-        queryId,
-        (k, v) -> {
-          queryScheduledTaskMap.get(queryId).cancel(false);
+          ScheduledFuture<?> scheduledFuture = queryScheduledTaskMap.remove(queryId);

Review comment:
       remove queryId from Map, this is solution..
   good... 




----------------------------------------------------------------
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] Alima777 merged pull request #2759: [ISSUE-2758] NullPointerException in QueryTimeManager.checkQueryAlive()

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


   


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