ascherbakoff commented on code in PR #3704:
URL: https://github.com/apache/ignite-3/pull/3704#discussion_r1598334234
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaService.java:
##########
@@ -210,7 +225,14 @@ private <R> CompletableFuture<R> sendToReplica(String
targetNodeConsistentId, Re
return null;
});
} else {
- res.completeExceptionally(errResp.throwable());
+ if (retryExecutor != null &&
matchAny(unwrapCause(errResp.throwable()), ACQUIRE_LOCK_ERR, REPLICA_MISS_ERR))
{
Review Comment:
> I disagree, using Java classes that represent an exception is a widespread
practice. Error codes are a way to provide the user with an additional clue on
critical situations, especially in the case of thin clients that are not
supported exceptions.
Looks like we have no agreement here, because changing error code checking
to `instanceof ` doesn't make much sense to me. This however should not block
PR changes. Or is this a merge blocker ? We can fix it later then we have
strict rules on working with errors.
--
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]
