JackieTien97 commented on code in PR #18301:
URL: https://github.com/apache/iotdb/pull/18301#discussion_r3644312659
##########
iotdb-client/service-rpc/src/main/java/org/apache/iotdb/rpc/TSStatusCode.java:
##########
@@ -90,7 +90,8 @@ public enum TSStatusCode {
TYPE_NOT_FOUND(528),
DATABASE_CONFLICT(529),
DATABASE_MODEL(530),
- METADATA_LEASE_FENCED(531),
+ METADATA_LEASE_FENCED(535),
Review Comment:
why change this status code?
##########
iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/ApplicationStateMachineProxy.java:
##########
@@ -161,12 +162,18 @@ public CompletableFuture<Message>
applyTransaction(TransactionContext trx) {
new
TSStatus(TSStatusCode.INTERNAL_SERVER_ERROR.getStatusCode())
.setMessage(RatisMessages.INTERNAL_ERROR_STATEMACHINE_RUNTIME_EXCEPTION + rte));
if (Utils.stallApply(consensusGroupType)) {
- waitUntilSystemAllowApply();
+ if (!waitUntilSystemAllowApply()) {
Review Comment:
you won't catch exceptio here, because you wrap exception into a TSStatus
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/metadata/fetcher/cache/TreeDeviceSchemaCacheManager.java:
##########
@@ -74,7 +75,7 @@ public static TreeDeviceSchemaCacheManager getInstance() {
}
void failIfMetadataLeaseFenced() {
- MetadataLeaseManager.getInstance().failIfMetadataLeaseFenced();
+
MetadataLeaseManager.getInstance().failIfMetadataLeaseFenced(LeaseFencedRetryPolicy.NONE);
Review Comment:
why we need NONE?
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/consensus/statemachine/dataregion/DataRegionStateMachine.java:
##########
@@ -330,10 +330,15 @@ public File getSnapshotRoot() {
}
}
- public static boolean needRetry(int statusCode) {
- // To fix the atomicity problem, we only need to add retry for system
reject.
+ public static boolean needRetryForSpecificCases(int statusCode, PlanNode
planNode) {
+ // To fix the atomicity problem, retry system rejection and a fenced
metadata lease that
+ // explicitly requires retry.
// In other cases, such as readonly, we can return directly because there
are retries at the
// consensus layer.
- return statusCode == TSStatusCode.WRITE_PROCESS_REJECT.getStatusCode();
+ if (statusCode == TSStatusCode.WRITE_PROCESS_REJECT.getStatusCode()
+ && !planNode.isGeneratedByPipe()) {
+ return true;
+ }
+ return statusCode ==
TSStatusCode.METADATA_LEASE_FENCED_RETRY_REQUIRED.getStatusCode();
Review Comment:
```suggestion
return statusCode ==
TSStatusCode.METADATA_LEASE_FENCED_RETRY_REQUIRED.getStatusCode() &&
!planNode.isGeneratedByPipe();
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
