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/06/22 01:55:35 UTC
[GitHub] [iotdb] jt2594838 opened a new pull request #3430: [IOTDB-1447] ClientPool is blocking other nodes when one node fails
jt2594838 opened a new pull request #3430:
URL: https://github.com/apache/iotdb/pull/3430
https://issues.apache.org/jira/browse/IOTDB-1447
--
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 #3430: [IOTDB-1447] ClientPool is blocking other nodes when one node fails
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #3430:
URL: https://github.com/apache/iotdb/pull/3430#discussion_r656700152
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/client/async/AsyncClientPool.java
##########
@@ -195,17 +195,17 @@ void onComplete(Node node) {
void recreateClient(Node node) {
ClusterNode clusterNode = new ClusterNode(node);
- synchronized (this) {
- Deque<AsyncClient> clientStack =
- clientCaches.computeIfAbsent(clusterNode, n -> new ArrayDeque<>());
+ Deque<AsyncClient> clientStack =
+ clientCaches.computeIfAbsent(clusterNode, n -> new ArrayDeque<>());
+ synchronized (clientStack) {
try {
AsyncClient asyncClient = asyncClientFactory.getAsyncClient(node, this);
clientStack.push(asyncClient);
} catch (IOException e) {
logger.error("Cannot create a new client for {}", node, e);
nodeClientNumMap.computeIfPresent(clusterNode, (n, cnt) -> cnt - 1);
}
- this.notifyAll();
+ clientStack.notifyAll();
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] neuyilan merged pull request #3430: [IOTDB-1447] ClientPool is blocking other nodes when one node fails
Posted by GitBox <gi...@apache.org>.
neuyilan merged pull request #3430:
URL: https://github.com/apache/iotdb/pull/3430
--
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 edited a comment on pull request #3430: [IOTDB-1447] ClientPool is blocking other nodes when one node fails
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3430:
URL: https://github.com/apache/iotdb/pull/3430#issuecomment-865487991
[](https://coveralls.io/builds/40799328)
Coverage decreased (-0.01%) to 67.875% when pulling **5219bee1483169043cb7268b53938c01dfe4f4ec on jira_1447** into **d8b4ce8e6fbc4e25a55c7244a9e86592b8487002 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] sonarcloud[bot] commented on pull request #3430: [IOTDB-1447] ClientPool is blocking other nodes when one node fails
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #3430:
URL: https://github.com/apache/iotdb/pull/3430#issuecomment-866474810
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=3430&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=3430&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3430&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=3430&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=3430&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3430&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=3430&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=3430&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3430&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=3430&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=3430&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3430&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90.png' alt='91.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3430&metric=new_coverage&view=list) [91.7% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3430&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=3430&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3430&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] jt2594838 commented on a change in pull request #3430: [IOTDB-1447] ClientPool is blocking other nodes when one node fails
Posted by GitBox <gi...@apache.org>.
jt2594838 commented on a change in pull request #3430:
URL: https://github.com/apache/iotdb/pull/3430#discussion_r656696476
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/client/async/AsyncClientPool.java
##########
@@ -195,17 +195,17 @@ void onComplete(Node node) {
void recreateClient(Node node) {
ClusterNode clusterNode = new ClusterNode(node);
- synchronized (this) {
- Deque<AsyncClient> clientStack =
- clientCaches.computeIfAbsent(clusterNode, n -> new ArrayDeque<>());
+ Deque<AsyncClient> clientStack =
+ clientCaches.computeIfAbsent(clusterNode, n -> new ArrayDeque<>());
+ synchronized (clientStack) {
try {
AsyncClient asyncClient = asyncClientFactory.getAsyncClient(node, this);
clientStack.push(asyncClient);
} catch (IOException e) {
logger.error("Cannot create a new client for {}", node, e);
nodeClientNumMap.computeIfPresent(clusterNode, (n, cnt) -> cnt - 1);
}
- this.notifyAll();
+ clientStack.notifyAll();
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls commented on pull request #3430: [IOTDB-1447] ClientPool is blocking other nodes when one node fails
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3430:
URL: https://github.com/apache/iotdb/pull/3430#issuecomment-865487991
[](https://coveralls.io/builds/40764534)
Coverage increased (+0.005%) to 67.892% when pulling **12f7dd6eb7d962eb7ab4de7e5990d80cfb18db6f on jira_1447** into **d8b4ce8e6fbc4e25a55c7244a9e86592b8487002 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] wangchao316 commented on a change in pull request #3430: [IOTDB-1447] ClientPool is blocking other nodes when one node fails
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #3430:
URL: https://github.com/apache/iotdb/pull/3430#discussion_r656144825
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/client/async/AsyncClientPool.java
##########
@@ -195,17 +195,17 @@ void onComplete(Node node) {
void recreateClient(Node node) {
ClusterNode clusterNode = new ClusterNode(node);
- synchronized (this) {
- Deque<AsyncClient> clientStack =
- clientCaches.computeIfAbsent(clusterNode, n -> new ArrayDeque<>());
+ Deque<AsyncClient> clientStack =
+ clientCaches.computeIfAbsent(clusterNode, n -> new ArrayDeque<>());
+ synchronized (clientStack) {
try {
AsyncClient asyncClient = asyncClientFactory.getAsyncClient(node, this);
clientStack.push(asyncClient);
} catch (IOException e) {
logger.error("Cannot create a new client for {}", node, e);
nodeClientNumMap.computeIfPresent(clusterNode, (n, cnt) -> cnt - 1);
}
- this.notifyAll();
+ clientStack.notifyAll();
Review comment:
hi ，this is good job.
this is a minor issue . "this.notifyAll()" is related "this.wait()".
please see 126 line.
--
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 commented on pull request #3430: [IOTDB-1447] ClientPool is blocking other nodes when one node fails
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3430:
URL: https://github.com/apache/iotdb/pull/3430#issuecomment-865487991
[](https://coveralls.io/builds/40764534)
Coverage increased (+0.005%) to 67.892% when pulling **12f7dd6eb7d962eb7ab4de7e5990d80cfb18db6f on jira_1447** into **d8b4ce8e6fbc4e25a55c7244a9e86592b8487002 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org