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/30 02:11:14 UTC
[GitHub] [iotdb] cmlmakahts opened a new pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries sta…
cmlmakahts opened a new pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474
Error was thrown in cluster mode when executing delete timeseries statement.
It happended frequently in the condition that ReplicateNum greater than node count.
It seems that batch exception is not handled correctly, timeseries does not exist exception should be skipped when delete timeseries plan is executing.
More details in https://issues.apache.org/jira/browse/IOTDB-1456
--
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] cmlmakahts commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
cmlmakahts commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r672775050
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
2. 506
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
I handle the exception to prevent this error message
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
I handle the exception to prevent this error message, skip all the timeseries_not_exist exception.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
I handle the exception to prevent this error message, skip all the timeseries_not_exist exception.
Or codes in forwardPlan should cover BatchProcessException?
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
I handle the exception to prevent this error message, skip all the timeseries_not_exist exception.
Or codes in `forwardPlan` should cover BatchProcessException?
--
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] cmlmakahts commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
cmlmakahts commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r672775050
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
I handle the exception to prevent this error message, skip all the timeseries_not_exist exception.
--
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] cmlmakahts commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
cmlmakahts commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r667984081
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
1. First mistake has been modified.(status.getCode() == TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode())
2.Delete operation won't be performed once again, but if exeception is thrown directly here, batch exception won't be catched correctly in coordinator.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
1. First mistake has been modified.(status.getCode() == TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode())
2.Delete operation won't be performed once again, but if exeception is thrown directly here, batch exception won't be catched correctly in coordinator.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
1. First mistake has been modified.(status.getCode() == TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode())
2.Delete operation won't be performed once again, but if exeception is thrown directly here, batch exception won't be catched in coordinator that leads to the error mentioned in the issue.
--
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] LebronAl commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r672765773
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Sorry I don't get your meaning.
It seems once we set `needThrow` to `true` here, It will always be thrown in the last? below codes are irrelevant with delete operations, so I still don't know why we can not throw this exception here or before below codes?
```
boolean needRetry = false;
for (int i = 0, failingStatusLength = failingStatus.length; i < failingStatusLength; i++) {
TSStatus status = failingStatus[i];
if (status != null
&& (status.getCode() == TSStatusCode.STORAGE_GROUP_NOT_EXIST.getStatusCode()
|| status.getCode() == TSStatusCode.UNDEFINED_TEMPLATE.getStatusCode())
&& plan instanceof BatchPlan) {
((BatchPlan) plan).unsetIsExecuted(i);
needRetry = true;
}
}
if (needRetry) {
executeAfterSync(plan);
return;
}
```
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Sorry I don't get your meaning.
It seems once we set `needThrow` to `true` here, It will always be thrown in the last? these codes are irrelevant with delete operations, so I still don't know why we can not throw this exception here or before codes below?
```
boolean needRetry = false;
for (int i = 0, failingStatusLength = failingStatus.length; i < failingStatusLength; i++) {
TSStatus status = failingStatus[i];
if (status != null
&& (status.getCode() == TSStatusCode.STORAGE_GROUP_NOT_EXIST.getStatusCode()
|| status.getCode() == TSStatusCode.UNDEFINED_TEMPLATE.getStatusCode())
&& plan instanceof BatchPlan) {
((BatchPlan) plan).unsetIsExecuted(i);
needRetry = true;
}
}
if (needRetry) {
executeAfterSync(plan);
return;
}
```
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
OK, I see your meaning : DeleteOpearion can not throw exception here.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
OK, I see your meaning : DeleteOperation can not throw exception 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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] cmlmakahts commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
cmlmakahts commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r672775050
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
I handle the exception to prevent this error message, skip all the timeseries_not_exist exception.
Or codes in forwardPlan should cover BatchProcessException?
--
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 #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#issuecomment-871069400
[](https://coveralls.io/builds/41295950)
Coverage remained the same at 68.15% when pulling **d5e1ea4af8952fe63685936a9f8bd40f0dcb9977 on cmlmakahts:master** into **0a3f21b230e72340a9baff139ff1bb30f50a67d7 on apache: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] coveralls edited a comment on pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#issuecomment-871069400
[](https://coveralls.io/builds/41349219)
Coverage increased (+0.01%) to 68.186% when pulling **5c59e52f4afc93f2827ab19745ec2bc2b3bb34cb on cmlmakahts:master** into **5e020b5880a97c8530b39986b504c5f28fcbcd33 on apache: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] LebronAl commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r672780041
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
OK, I see your meaning : DeleteOpearion can not throw exception 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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] LebronAl commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r672765773
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Sorry I don't get your meaning.
It seems once we set `needThrow` to `true` here, It will always be thrown in the last? these codes are irrelevant with delete operations, so I still don't know why we can not throw this exception here or before codes below?
```
boolean needRetry = false;
for (int i = 0, failingStatusLength = failingStatus.length; i < failingStatusLength; i++) {
TSStatus status = failingStatus[i];
if (status != null
&& (status.getCode() == TSStatusCode.STORAGE_GROUP_NOT_EXIST.getStatusCode()
|| status.getCode() == TSStatusCode.UNDEFINED_TEMPLATE.getStatusCode())
&& plan instanceof BatchPlan) {
((BatchPlan) plan).unsetIsExecuted(i);
needRetry = true;
}
}
if (needRetry) {
executeAfterSync(plan);
return;
}
```
--
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] LebronAl commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r672765773
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Sorry I don't get your meaning.
It seems once we set `needThrow` to `true` here, It will always be thrown in the last? below codes are irrelevant with delete operations, so I still don't know why we can not throw this exception here or before below codes?
```
boolean needRetry = false;
for (int i = 0, failingStatusLength = failingStatus.length; i < failingStatusLength; i++) {
TSStatus status = failingStatus[i];
if (status != null
&& (status.getCode() == TSStatusCode.STORAGE_GROUP_NOT_EXIST.getStatusCode()
|| status.getCode() == TSStatusCode.UNDEFINED_TEMPLATE.getStatusCode())
&& plan instanceof BatchPlan) {
((BatchPlan) plan).unsetIsExecuted(i);
needRetry = true;
}
}
if (needRetry) {
executeAfterSync(plan);
return;
}
```
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Sorry I don't get your meaning.
It seems once we set `needThrow` to `true` here, It will always be thrown in the last? these codes are irrelevant with delete operations, so I still don't know why we can not throw this exception here or before codes below?
```
boolean needRetry = false;
for (int i = 0, failingStatusLength = failingStatus.length; i < failingStatusLength; i++) {
TSStatus status = failingStatus[i];
if (status != null
&& (status.getCode() == TSStatusCode.STORAGE_GROUP_NOT_EXIST.getStatusCode()
|| status.getCode() == TSStatusCode.UNDEFINED_TEMPLATE.getStatusCode())
&& plan instanceof BatchPlan) {
((BatchPlan) plan).unsetIsExecuted(i);
needRetry = true;
}
}
if (needRetry) {
executeAfterSync(plan);
return;
}
```
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
OK, I see your meaning : DeleteOpearion can not throw exception here.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
OK, I see your meaning : DeleteOperation can not throw exception 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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] LebronAl commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r672780041
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
OK, I see your meaning : DeleteOperation can not throw exception 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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] cmlmakahts commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
cmlmakahts commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r672775050
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
2. 506
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
I handle the exception to prevent this error message
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
I handle the exception to prevent this error message, skip all the timeseries_not_exist exception.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
I handle the exception to prevent this error message, skip all the timeseries_not_exist exception.
Or codes in forwardPlan should cover BatchProcessException?
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
I handle the exception to prevent this error message, skip all the timeseries_not_exist exception.
Or codes in `forwardPlan` should cover BatchProcessException?
--
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 merged pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
wangchao316 merged pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474
--
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] cmlmakahts commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
cmlmakahts commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r672775050
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
I handle the exception to prevent this error message, skip all the timeseries_not_exist exception.
Or codes in `forwardPlan` should cover BatchProcessException?
--
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] cmlmakahts commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
cmlmakahts commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r672775050
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
I handle the exception to prevent this error message
--
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] cmlmakahts commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
cmlmakahts commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r672775050
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
2. 506
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Delete operations indeed succeed, but the client may receives a error message which may confuse user.
--
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 #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#issuecomment-871069400
[](https://coveralls.io/builds/41328237)
Coverage increased (+0.04%) to 68.189% when pulling **a82bce1b35f72afd7b78f29c8ba11fdb1c3a55c1 on cmlmakahts:master** into **0a3f21b230e72340a9baff139ff1bb30f50a67d7 on apache: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] LebronAl commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r663780703
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
Review comment:
'status.getCode() == TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()' ?
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
As delete operation doesn't need to be performd once again, maybe we can directly throw exception 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.
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 #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#issuecomment-871069400
[](https://coveralls.io/builds/41354973)
Coverage decreased (-0.003%) to 68.172% when pulling **df88eec8c79e5923b352e7d25a4a88724480d9d1 on cmlmakahts:master** into **5e020b5880a97c8530b39986b504c5f28fcbcd33 on apache: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] coveralls edited a comment on pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#issuecomment-871069400
[](https://coveralls.io/builds/41419206)
Coverage increased (+0.02%) to 68.15% when pulling **426f529974da5cce548bdd56d818c4a352b6164c on cmlmakahts:master** into **25f7f5587bd583300f761329d94f3c6c8af32ddf on apache: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] coveralls edited a comment on pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#issuecomment-871069400
[](https://coveralls.io/builds/41334508)
Coverage decreased (-0.004%) to 68.171% when pulling **14e5d80bd1ccfa178cc3ffd996425ca84ee3e264 on cmlmakahts:master** into **5e020b5880a97c8530b39986b504c5f28fcbcd33 on apache: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] coveralls edited a comment on pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#issuecomment-871069400
[](https://coveralls.io/builds/41328534)
Coverage decreased (-0.01%) to 68.136% when pulling **a82bce1b35f72afd7b78f29c8ba11fdb1c3a55c1 on cmlmakahts:master** into **0a3f21b230e72340a9baff139ff1bb30f50a67d7 on apache: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] coveralls commented on pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#issuecomment-871069400
[](https://coveralls.io/builds/40990379)
Coverage decreased (-0.009%) to 67.131% when pulling **ad8fa228133e75b1e5e6f4b8f911916d0a49575d on cmlmakahts:master** into **4a6f4b04d3e83fa76f16816f6dffd0cd6e02578c on apache: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] LebronAl commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r672765773
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
Sorry I don't get your meaning.
It seems once we set `needThrow` to `true` here, It will always be thrown in the last? below codes are irrelevant with delete operations, so I still don't know why we can not throw this exception here or before below codes?
```
boolean needRetry = false;
for (int i = 0, failingStatusLength = failingStatus.length; i < failingStatusLength; i++) {
TSStatus status = failingStatus[i];
if (status != null
&& (status.getCode() == TSStatusCode.STORAGE_GROUP_NOT_EXIST.getStatusCode()
|| status.getCode() == TSStatusCode.UNDEFINED_TEMPLATE.getStatusCode())
&& plan instanceof BatchPlan) {
((BatchPlan) plan).unsetIsExecuted(i);
needRetry = true;
}
}
if (needRetry) {
executeAfterSync(plan);
return;
}
```
--
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 merged pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
wangchao316 merged pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474
--
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 merged pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
wangchao316 merged pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474
--
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] LebronAl commented on a change in pull request #3474: [IOTDB-1456] Fix Error occurred while executing delete timeseries statement
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3474:
URL: https://github.com/apache/iotdb/pull/3474#discussion_r663780703
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
Review comment:
'status.getCode() == TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()' ?
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -102,6 +104,16 @@ private void handleBatchProcessException(BatchProcessException e, PhysicalPlan p
&& plan instanceof BatchPlan) {
((BatchPlan) plan).setIsExecuted(i);
}
+
+ if (plan instanceof DeleteTimeSeriesPlan) {
+ if (status != null && status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+ if (status.getCode() != TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ logger.info("{} doesn't exist, it may has been deleted.", plan.getPaths().get(i));
+ } else {
+ needThrow = true;
Review comment:
As delete operation doesn't need to be performd once again, maybe we can directly throw exception 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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org