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 2020/11/06 08:55:40 UTC
[GitHub] [iotdb] asdf2014 opened a new pull request #1968: Fix the risk of deadlock by WeakReference
asdf2014 opened a new pull request #1968:
URL: https://github.com/apache/iotdb/pull/1968
----------------------------------------------------------------
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] asdf2014 commented on a change in pull request #1968: Fix the risk of deadlock by WeakReference
Posted by GitBox <gi...@apache.org>.
asdf2014 commented on a change in pull request #1968:
URL: https://github.com/apache/iotdb/pull/1968#discussion_r544906831
##########
File path: server/src/main/java/org/apache/iotdb/db/engine/cache/TimeSeriesMetadataCache.java
##########
@@ -60,6 +64,7 @@
private final ReadWriteLock lock = new ReentrantReadWriteLock();
+ private final Map<String, WeakReference<String>> devices = Collections.synchronizedMap(new WeakHashMap<>());
Review comment:
Hi, @JackieTien97 . FYI, https://stackoverflow.com/a/134154/4719867
----------------------------------------------------------------
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 #1968: Fix the risk of deadlock by WeakReference
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #1968:
URL: https://github.com/apache/iotdb/pull/1968#issuecomment-729476586
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=1968&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=1968&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=1968&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=1968&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=1968&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=1968&resolved=false&types=VULNERABILITY) (and [<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/issues?id=apache_incubator-iotdb&pullRequest=1968&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/i
ssues?id=apache_incubator-iotdb&pullRequest=1968&resolved=false&types=SECURITY_HOTSPOT) to review)
[<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=1968&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=1968&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=1968&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=1968) No Coverage information
[<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=1968&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=1968&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] asdf2014 commented on a change in pull request #1968: Fix the risk of deadlock by WeakReference
Posted by GitBox <gi...@apache.org>.
asdf2014 commented on a change in pull request #1968:
URL: https://github.com/apache/iotdb/pull/1968#discussion_r519156136
##########
File path: server/src/main/java/org/apache/iotdb/db/engine/cache/TimeSeriesMetadataCache.java
##########
@@ -60,6 +64,7 @@
private final ReadWriteLock lock = new ReentrantReadWriteLock();
+ private final Map<String, WeakReference<String>> devices = Collections.synchronizedMap(new WeakHashMap<>());
Review comment:
Hi, @jixuan1989 . Thanks for your reply. Indeed, taking this into consideration, so WeakReference is used here, which is more GC friendly. Moreover, I think the previous method has the risk of deadlock and the memory usage is not small.
----------------------------------------------------------------
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] HTHou merged pull request #1968: Fix the risk of deadlock by WeakReference
Posted by GitBox <gi...@apache.org>.
HTHou merged pull request #1968:
URL: https://github.com/apache/iotdb/pull/1968
----------------------------------------------------------------
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] JackieTien97 commented on a change in pull request #1968: Fix the risk of deadlock by WeakReference
Posted by GitBox <gi...@apache.org>.
JackieTien97 commented on a change in pull request #1968:
URL: https://github.com/apache/iotdb/pull/1968#discussion_r521301780
##########
File path: server/src/main/java/org/apache/iotdb/db/engine/cache/TimeSeriesMetadataCache.java
##########
@@ -60,6 +64,7 @@
private final ReadWriteLock lock = new ReentrantReadWriteLock();
+ private final Map<String, WeakReference<String>> devices = Collections.synchronizedMap(new WeakHashMap<>());
Review comment:
Hi, can you give more details about the mentioned deadlock thing?
----------------------------------------------------------------
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] jixuan1989 commented on a change in pull request #1968: Fix the risk of deadlock by WeakReference
Posted by GitBox <gi...@apache.org>.
jixuan1989 commented on a change in pull request #1968:
URL: https://github.com/apache/iotdb/pull/1968#discussion_r519085877
##########
File path: server/src/main/java/org/apache/iotdb/db/engine/cache/TimeSeriesMetadataCache.java
##########
@@ -60,6 +64,7 @@
private final ReadWriteLock lock = new ReentrantReadWriteLock();
+ private final Map<String, WeakReference<String>> devices = Collections.synchronizedMap(new WeakHashMap<>());
Review comment:
this should be discussed.
We may have too many devices and the memory is limited..
----------------------------------------------------------------
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