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/08/23 03:36:34 UTC

[GitHub] [iotdb] ericpai opened a new pull request, #7087: [IOTDB-4210] Fix deadlock during WALNode recover

ericpai opened a new pull request, #7087:
URL: https://github.com/apache/iotdb/pull/7087

   See JIRA: https://issues.apache.org/jira/projects/IOTDB/issues/IOTDB-4210?filter=myopenissues
   
   If there's no suitable walFiles to recover, the recover task will end without notifying the listeners.


-- 
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] sonarcloud[bot] commented on pull request #7087: [IOTDB-4210] Fix deadlock during WALNode recover

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

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_incubator-iotdb&pullRequest=7087)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=7087&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=7087&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=7087&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=7087&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=7087&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=7087&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=7087&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=7087&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=7087&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=7087&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=7087&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=7087&resolved=false&types=CODE_SMELL)
   
   [![66.7%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '66.7%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=7087&metric=new_coverage&view=list) [66.7% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=7087&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=7087&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=7087&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.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

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

[GitHub] [iotdb] ericpai commented on a diff in pull request #7087: [IOTDB-4210] Fix deadlock during WALNode recover

Posted by GitBox <gi...@apache.org>.
ericpai commented on code in PR #7087:
URL: https://github.com/apache/iotdb/pull/7087#discussion_r952124468


##########
server/src/main/java/org/apache/iotdb/db/wal/recover/WALNodeRecoverTask.java:
##########
@@ -226,6 +226,9 @@ private void recoverTsFiles() {
                 WALFileUtils.walFilenameFilter(dir, name)
                     && WALFileUtils.parseVersionId(name) >= firstValidVersionId);
     if (walFiles == null) {
+      for (UnsealedTsFileRecoverPerformer recoverPerformer : memTableId2RecoverPerformer.values()) {
+        recoverPerformer.getRecoverListener().succeed();
+      }

Review Comment:
   Fixed, with a little of refactors.



-- 
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] HeimingZ commented on a diff in pull request #7087: [IOTDB-4210] Fix deadlock during WALNode recover

Posted by GitBox <gi...@apache.org>.
HeimingZ commented on code in PR #7087:
URL: https://github.com/apache/iotdb/pull/7087#discussion_r952140192


##########
server/src/main/java/org/apache/iotdb/db/wal/recover/WALNodeRecoverTask.java:
##########
@@ -226,6 +226,9 @@ private void recoverTsFiles() {
                 WALFileUtils.walFilenameFilter(dir, name)
                     && WALFileUtils.parseVersionId(name) >= firstValidVersionId);
     if (walFiles == null) {
+      for (UnsealedTsFileRecoverPerformer recoverPerformer : memTableId2RecoverPerformer.values()) {
+        recoverPerformer.getRecoverListener().succeed();
+      }

Review Comment:
   LGTM 👍 



-- 
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] HeimingZ commented on a diff in pull request #7087: [IOTDB-4210] Fix deadlock during WALNode recover

Posted by GitBox <gi...@apache.org>.
HeimingZ commented on code in PR #7087:
URL: https://github.com/apache/iotdb/pull/7087#discussion_r952123111


##########
server/src/main/java/org/apache/iotdb/db/wal/recover/WALNodeRecoverTask.java:
##########
@@ -226,6 +226,9 @@ private void recoverTsFiles() {
                 WALFileUtils.walFilenameFilter(dir, name)
                     && WALFileUtils.parseVersionId(name) >= firstValidVersionId);
     if (walFiles == null) {
+      for (UnsealedTsFileRecoverPerformer recoverPerformer : memTableId2RecoverPerformer.values()) {
+        recoverPerformer.getRecoverListener().succeed();
+      }

Review Comment:
   Thank you for the bug fix, but UnsealedTsFileRecoverPerformer.endRecovery should be called before return success.
   
   ```suggestion
         for (UnsealedTsFileRecoverPerformer recoverPerformer : memTableId2RecoverPerformer.values()) {
         try {
           recoverPerformer.endRecovery();
           recoverPerformer.getRecoverListener().succeed();
         } catch (Exception e) {
           recoverPerformer.getRecoverListener().fail(e);
         }
       }
   ```



-- 
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 #7087: [IOTDB-4210] Fix deadlock during WALNode recover

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

   
   [![Coverage Status](https://coveralls.io/builds/51861892/badge)](https://coveralls.io/builds/51861892)
   
   Coverage decreased (-0.005%) to 42.341% when pulling **741189771abf0c27928338dcadafaf08f7b13e5e on bugfix/iotdb-4210** into **57428ae2b999154b39a496f908db26ce5d367baa on 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] ericpai merged pull request #7087: [IOTDB-4210] Fix deadlock during WALNode recover

Posted by GitBox <gi...@apache.org>.
ericpai merged PR #7087:
URL: https://github.com/apache/iotdb/pull/7087


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