timoninmaxim commented on code in PR #12169:
URL: https://github.com/apache/ignite/pull/12169#discussion_r2183186319
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java:
##########
@@ -68,8 +68,8 @@ final class ReliableChannel implements AutoCloseable {
/** Client channel holders for each configured address. */
private volatile List<ClientChannelHolder> channels;
- /** Limit of attempts to execute each service. */
- private volatile int attemptsLimit;
+ /** Limit of channels to execute each {@link #service}. */
Review Comment:
Let's mention that each channel is tried twice, and these attempts counts as
1.
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java:
##########
@@ -272,36 +273,87 @@ private <T> Object applyOnClientChannelAsync(
return null;
}
- if (err instanceof ClientConnectionException) {
- ClientConnectionException failure0 =
(ClientConnectionException)err;
+ if (!(err instanceof ClientConnectionException)) {
+ fut.completeExceptionally(err instanceof ClientException ?
err : new ClientException(err));
- failures.add(failure0);
+ return null;
+ }
- try {
- // Will try to reinit channels if topology changed.
- onChannelFailure(ch, err, failures);
- }
- catch (Throwable ex) {
- fut.completeExceptionally(ex);
+ ClientConnectionException connEx =
(ClientConnectionException)err;
- return null;
- }
+ if (!shouldRetry(op, failures.size() - 1, connEx)) {
+ failures.add(connEx);
- if (failures.size() < attemptsLimit && shouldRetry(op,
failures.size() - 1, failure0)) {
- handleServiceAsync(fut, op, payloadWriter,
payloadReader, failures);
+ fut.completeExceptionally(composeException(failures));
- return null;
- }
+ return null;
+ }
- fut.completeExceptionally(composeException(failures));
+ // Retry use same channel in case of connection exception.
+ UUID nodeId = ch.serverNodeId();
+
+ ClientChannel retryCh = null;
+
+ ClientChannelHolder hld;
+
+ try {
+ hld = (nodeId != null) ? nodeChannels.get(nodeId) : null;
Review Comment:
`ClientChannelHolder hld = ...`
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java:
##########
@@ -272,36 +273,87 @@ private <T> Object applyOnClientChannelAsync(
return null;
}
- if (err instanceof ClientConnectionException) {
- ClientConnectionException failure0 =
(ClientConnectionException)err;
+ if (!(err instanceof ClientConnectionException)) {
+ fut.completeExceptionally(err instanceof ClientException ?
err : new ClientException(err));
- failures.add(failure0);
+ return null;
+ }
- try {
- // Will try to reinit channels if topology changed.
- onChannelFailure(ch, err, failures);
- }
- catch (Throwable ex) {
- fut.completeExceptionally(ex);
+ ClientConnectionException connEx =
(ClientConnectionException)err;
- return null;
- }
+ if (!shouldRetry(op, failures.size() - 1, connEx)) {
+ failures.add(connEx);
- if (failures.size() < attemptsLimit && shouldRetry(op,
failures.size() - 1, failure0)) {
- handleServiceAsync(fut, op, payloadWriter,
payloadReader, failures);
+ fut.completeExceptionally(composeException(failures));
- return null;
- }
+ return null;
+ }
- fut.completeExceptionally(composeException(failures));
+ // Retry use same channel in case of connection exception.
+ UUID nodeId = ch.serverNodeId();
+
+ ClientChannel retryCh = null;
Review Comment:
This is no need to initialize this variable
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java:
##########
@@ -68,8 +68,8 @@ final class ReliableChannel implements AutoCloseable {
/** Client channel holders for each configured address. */
private volatile List<ClientChannelHolder> channels;
- /** Limit of attempts to execute each service. */
- private volatile int attemptsLimit;
+ /** Limit of channels to execute each {@link #service}. */
+ private volatile int srvcChannelsLimit;
Review Comment:
let's revert naming.
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java:
##########
@@ -243,27 +243,28 @@ private <T> void handleServiceAsync(
List<ClientConnectionException> failures
) {
try {
- applyOnDefaultChannel(
- channel -> applyOnClientChannelAsync(fut, channel, op,
payloadWriter, payloadReader, failures),
- null,
- failures
- );
+ ClientChannel ch = applyOnDefaultChannel(Function.identity(),
null, failures);
+
+ applyOnClientChannelAsync(fut, ch, op, payloadWriter,
payloadReader, failures);
}
catch (Throwable ex) {
fut.completeExceptionally(ex);
}
}
- /** */
- private <T> Object applyOnClientChannelAsync(
+ /**
+ * Retries an async operation on the same channel if it fails with a
connection exception
Review Comment:
Let's mention that this method performs operation asynchronously, and in
case of ClientConnectionException tries the same channel first.
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java:
##########
@@ -922,19 +964,46 @@ private <T> T applyOnNodeChannelWithFallback(UUID
tryNodeId, Function<ClientChan
return function.apply(channel);
}
catch (ClientConnectionException e) {
- failures = new ArrayList<>();
- failures.add(e);
+ if (!shouldRetry(op, 0, e))
+ throw e;
+
+ try {
+ channel = getRetryChannel(hld, channel);
- onChannelFailure(hld, channel, e, failures);
+ return function.apply(channel);
+ }
- if (attemptsLimit == 1 || !shouldRetry(op, 0, e))
- throw e;
+ catch (ClientConnectionException err) {
+ failures = new ArrayList<>();
+
+ failures.add(err);
+
+ onChannelFailure(hld, channel, err, failures);
+
+ if (srvcChannelsLimit == 1 || !shouldRetry(op, 1, e))
+ throw err;
+ }
}
}
return applyOnDefaultChannel(function, op, failures);
}
+ /**
+ * Returns the client channel that should be used to retry sending the
request.
+ * Unlike onChannelFailure, invoked only for a reconnection attempt on the
same channel without channels initialization.
+ */
+ private ClientChannel getRetryChannel(ClientChannelHolder hld,
ClientChannel ch) {
Review Comment:
Code duplication with `onChannelFailure`
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java:
##########
@@ -272,36 +273,87 @@ private <T> Object applyOnClientChannelAsync(
return null;
}
- if (err instanceof ClientConnectionException) {
- ClientConnectionException failure0 =
(ClientConnectionException)err;
+ if (!(err instanceof ClientConnectionException)) {
+ fut.completeExceptionally(err instanceof ClientException ?
err : new ClientException(err));
- failures.add(failure0);
+ return null;
+ }
- try {
- // Will try to reinit channels if topology changed.
- onChannelFailure(ch, err, failures);
- }
- catch (Throwable ex) {
- fut.completeExceptionally(ex);
+ ClientConnectionException connEx =
(ClientConnectionException)err;
- return null;
- }
+ if (!shouldRetry(op, failures.size() - 1, connEx)) {
+ failures.add(connEx);
- if (failures.size() < attemptsLimit && shouldRetry(op,
failures.size() - 1, failure0)) {
- handleServiceAsync(fut, op, payloadWriter,
payloadReader, failures);
+ fut.completeExceptionally(composeException(failures));
- return null;
- }
+ return null;
+ }
- fut.completeExceptionally(composeException(failures));
+ // Retry use same channel in case of connection exception.
+ UUID nodeId = ch.serverNodeId();
Review Comment:
Move nodeId definition inside the following try block (closer to place where
it's used).
##########
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java:
##########
@@ -272,36 +273,87 @@ private <T> Object applyOnClientChannelAsync(
return null;
}
- if (err instanceof ClientConnectionException) {
- ClientConnectionException failure0 =
(ClientConnectionException)err;
+ if (!(err instanceof ClientConnectionException)) {
+ fut.completeExceptionally(err instanceof ClientException ?
err : new ClientException(err));
- failures.add(failure0);
+ return null;
+ }
- try {
- // Will try to reinit channels if topology changed.
- onChannelFailure(ch, err, failures);
Review Comment:
Now the method `onChannelFailure(ClientChannel, Throwable, List)` is unused
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
