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 not agreement here, because changing error code checking
to `instanceof ` doesn't make much sense to me. This however have should not
block PR changes. Or is this a merge blocker ?
--
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]
