You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "OneSizeFitsQuorum (via GitHub)" <gi...@apache.org> on 2023/08/09 13:19:54 UTC

[GitHub] [iotdb] OneSizeFitsQuorum opened a new pull request, #10822: [IOTDB-6106] Fixed the timeout parameter not working in thrift asyncClient

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

   see [jira](https://issues.apache.org/jira/browse/IOTDB-6106)


-- 
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] OneSizeFitsQuorum commented on a diff in pull request #10822: [IOTDB-6106] Fixed the timeout parameter not working in thrift asyncClient

Posted by "OneSizeFitsQuorum (via GitHub)" <gi...@apache.org>.
OneSizeFitsQuorum commented on code in PR #10822:
URL: https://github.com/apache/iotdb/pull/10822#discussion_r1290035705


##########
iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/client/ClientManagerTest.java:
##########
@@ -461,6 +487,96 @@ public void borrowNullTest() {
     Assert.assertEquals(0, asyncClusterManager.getPool().getNumIdle(endPoint));
   }
 
+  public void syncClientTimeoutTest() throws Exception {
+    // init syncClientManager
+    ClientManager<TEndPoint, SyncDataNodeInternalServiceClient> syncClusterManager =
+        (ClientManager<TEndPoint, SyncDataNodeInternalServiceClient>)
+            new IClientManager.Factory<TEndPoint, SyncDataNodeInternalServiceClient>()
+                .createClientManager(new TestSyncDataNodeInternalServiceClientPoolFactory());
+
+    // normal RPC
+    try (SyncDataNodeInternalServiceClient syncClient = syncClusterManager.borrowClient(endPoint)) {
+      syncClient.merge();
+    } catch (Exception e) {
+      Assert.fail("There should be no timeout here");
+    }
+    Assert.assertEquals(0, syncClusterManager.getPool().getNumActive(endPoint));
+    Assert.assertEquals(1, syncClusterManager.getPool().getNumIdle(endPoint));
+
+    // timeout RPC
+    try (SyncDataNodeInternalServiceClient syncClient = syncClusterManager.borrowClient(endPoint)) {
+      syncClient.clearCache();
+      Assert.fail("A timeout exception should occur here");
+    } catch (Exception ignored) {
+    }

Review Comment:
   Fixed



-- 
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 #10822: [IOTDB-6106] Fixed the timeout parameter not working in thrift asyncClient

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #10822:
URL: https://github.com/apache/iotdb/pull/10822#issuecomment-1673124390

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=10822)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=10822&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_iotdb&pullRequest=10822&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=10822&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_iotdb&pullRequest=10822&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_iotdb&pullRequest=10822&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=10822&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_iotdb&pullRequest=10822&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_iotdb&pullRequest=10822&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_iotdb&pullRequest=10822&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_iotdb&pullRequest=10822&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_iotdb&pullRequest=10822&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=10822&resolved=false&types=CODE_SMELL)
   
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=10822&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=10822&metric=new_coverage&view=list)  
   [![3.1%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/5-16px.png '3.1%')](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=10822&metric=new_duplicated_lines_density&view=list) [3.1% Duplication](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=10822&metric=new_duplicated_lines_density&view=list)
   
   
   
   ![idea](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/light_bulb-16px.png 'idea') Catch issues before they fail your Quality Gate with our IDE extension ![sonarlint](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/sonarlint-16px.png 'sonarlint') [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=sonarcloud-welcome)


-- 
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 #10822: [IOTDB-6106] Fixed the timeout parameter not working in thrift asyncClient

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #10822:
URL: https://github.com/apache/iotdb/pull/10822#issuecomment-1671331067

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=10822)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=10822&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_iotdb&pullRequest=10822&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=10822&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_iotdb&pullRequest=10822&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_iotdb&pullRequest=10822&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=10822&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_iotdb&pullRequest=10822&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_iotdb&pullRequest=10822&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_iotdb&pullRequest=10822&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_iotdb&pullRequest=10822&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_iotdb&pullRequest=10822&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=10822&resolved=false&types=CODE_SMELL)
   
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=10822&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=10822&metric=new_coverage&view=list)  
   [![3.1%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/5-16px.png '3.1%')](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=10822&metric=new_duplicated_lines_density&view=list) [3.1% Duplication](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=10822&metric=new_duplicated_lines_density&view=list)
   
   
   
   ![idea](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/light_bulb-16px.png 'idea') Catch issues before they fail your Quality Gate with our IDE extension ![sonarlint](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/sonarlint-16px.png 'sonarlint') [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=sonarcloud-welcome)


-- 
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] OneSizeFitsQuorum merged pull request #10822: [IOTDB-6106] Fixed the timeout parameter not working in thrift asyncClient

Posted by "OneSizeFitsQuorum (via GitHub)" <gi...@apache.org>.
OneSizeFitsQuorum merged PR #10822:
URL: https://github.com/apache/iotdb/pull/10822


-- 
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 diff in pull request #10822: [IOTDB-6106] Fixed the timeout parameter not working in thrift asyncClient

Posted by "wangchao316 (via GitHub)" <gi...@apache.org>.
wangchao316 commented on code in PR #10822:
URL: https://github.com/apache/iotdb/pull/10822#discussion_r1289719251


##########
iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/client/ClientManagerTest.java:
##########
@@ -461,6 +487,96 @@ public void borrowNullTest() {
     Assert.assertEquals(0, asyncClusterManager.getPool().getNumIdle(endPoint));
   }
 
+  public void syncClientTimeoutTest() throws Exception {
+    // init syncClientManager
+    ClientManager<TEndPoint, SyncDataNodeInternalServiceClient> syncClusterManager =
+        (ClientManager<TEndPoint, SyncDataNodeInternalServiceClient>)
+            new IClientManager.Factory<TEndPoint, SyncDataNodeInternalServiceClient>()
+                .createClientManager(new TestSyncDataNodeInternalServiceClientPoolFactory());
+
+    // normal RPC
+    try (SyncDataNodeInternalServiceClient syncClient = syncClusterManager.borrowClient(endPoint)) {
+      syncClient.merge();
+    } catch (Exception e) {
+      Assert.fail("There should be no timeout here");
+    }
+    Assert.assertEquals(0, syncClusterManager.getPool().getNumActive(endPoint));
+    Assert.assertEquals(1, syncClusterManager.getPool().getNumIdle(endPoint));
+
+    // timeout RPC
+    try (SyncDataNodeInternalServiceClient syncClient = syncClusterManager.borrowClient(endPoint)) {
+      syncClient.clearCache();
+      Assert.fail("A timeout exception should occur here");
+    } catch (Exception ignored) {
+    }

Review Comment:
   [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=10822&resolved=false&types=CODE_SMELL)



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