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/08 09:20:35 UTC
[GitHub] [iotdb] LebronAl opened a new pull request #2778: [IOTDB-1186] Remove redundant sync meta leader in query process for cluster module
LebronAl opened a new pull request #2778:
URL: https://github.com/apache/iotdb/pull/2778
see [1186](https://issues.apache.org/jira/browse/IOTDB-1186).
----------------------------------------------------------------
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 #2778: [IOTDB-1186] Remove redundant sync meta leader in query process for cluster module
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2778:
URL: https://github.com/apache/iotdb/pull/2778#issuecomment-792631314
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=2778&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=2778&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2778&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=2778&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=2778&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2778&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=2778&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=2778&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2778&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=2778&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=2778&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2778&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/25.png' alt='35.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2778&metric=new_coverage&view=list) [35.7% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2778&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=2778&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2778&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] mychaow commented on a change in pull request #2778: [IOTDB-1186] Remove redundant sync meta leader in query process for cluster module
Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #2778:
URL: https://github.com/apache/iotdb/pull/2778#discussion_r589861655
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/ClusterMain.java
##########
@@ -260,7 +260,7 @@ private static void preStartCustomize() {
@Override
public int calculateSlotByTime(String storageGroupName, long timestamp, int maxSlotNum) {
int sgSerialNum = extractSerialNumInSGName(storageGroupName) % k;
- if (sgSerialNum > 0) {
+ if (sgSerialNum >= 0) {
Review comment:
if sgSerialNum == 0, we will get ArithmeticException: / by zero
----------------------------------------------------------------
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] mychaow commented on a change in pull request #2778: [IOTDB-1186] Remove redundant sync meta leader in query process for cluster module
Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #2778:
URL: https://github.com/apache/iotdb/pull/2778#discussion_r589871972
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/ClusterMain.java
##########
@@ -260,7 +260,7 @@ private static void preStartCustomize() {
@Override
public int calculateSlotByTime(String storageGroupName, long timestamp, int maxSlotNum) {
int sgSerialNum = extractSerialNumInSGName(storageGroupName) % k;
- if (sgSerialNum > 0) {
+ if (sgSerialNum >= 0) {
Review comment:
Got it.
----------------------------------------------------------------
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 #2778: [IOTDB-1186] Remove redundant sync meta leader in query process for cluster module
Posted by GitBox <gi...@apache.org>.
neuyilan merged pull request #2778:
URL: https://github.com/apache/iotdb/pull/2778
----------------------------------------------------------------
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] LebronAl commented on a change in pull request #2778: [IOTDB-1186] Remove redundant sync meta leader in query process for cluster module
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #2778:
URL: https://github.com/apache/iotdb/pull/2778#discussion_r589873983
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/ClusterMain.java
##########
@@ -260,7 +260,7 @@ private static void preStartCustomize() {
@Override
public int calculateSlotByTime(String storageGroupName, long timestamp, int maxSlotNum) {
int sgSerialNum = extractSerialNumInSGName(storageGroupName) % k;
- if (sgSerialNum > 0) {
+ if (sgSerialNum >= 0) {
Review comment:
Actually, this is because the storage groups in benchmark will start with `root.group_0`， so it's better to change the `>` to `>=` here.
----------------------------------------------------------------
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 commented on a change in pull request #2778: [IOTDB-1186] Remove redundant sync meta leader in query process for cluster module
Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #2778:
URL: https://github.com/apache/iotdb/pull/2778#discussion_r589864270
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/ClusterMain.java
##########
@@ -260,7 +260,7 @@ private static void preStartCustomize() {
@Override
public int calculateSlotByTime(String storageGroupName, long timestamp, int maxSlotNum) {
int sgSerialNum = extractSerialNumInSGName(storageGroupName) % k;
- if (sgSerialNum > 0) {
+ if (sgSerialNum >= 0) {
Review comment:
As long as k is not zero, we will not get the ArithmeticException / by zero error.
----------------------------------------------------------------
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 #2778: [IOTDB-1186] Remove redundant sync meta leader in query process for cluster module
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #2778:
URL: https://github.com/apache/iotdb/pull/2778#discussion_r589320014
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/reader/ClusterTimeGenerator.java
##########
@@ -51,8 +52,9 @@ public ClusterTimeGenerator(
this.queryPlan = rawDataQueryPlan;
this.readerFactory = new ClusterReaderFactory(metaGroupMember);
try {
+ readerFactory.syncMetaGroup();
Review comment:
+1, 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] mychaow commented on a change in pull request #2778: [IOTDB-1186] Remove redundant sync meta leader in query process for cluster module
Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #2778:
URL: https://github.com/apache/iotdb/pull/2778#discussion_r589869916
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/ClusterMain.java
##########
@@ -260,7 +260,7 @@ private static void preStartCustomize() {
@Override
public int calculateSlotByTime(String storageGroupName, long timestamp, int maxSlotNum) {
int sgSerialNum = extractSerialNumInSGName(storageGroupName) % k;
- if (sgSerialNum > 0) {
+ if (sgSerialNum >= 0) {
Review comment:
In fact, extractSerialNumInSGName(storageGroupName) is possible to return 0
----------------------------------------------------------------
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