isapego commented on a change in pull request #8174:
URL: https://github.com/apache/ignite/pull/8174#discussion_r493745898
##########
File path:
modules/core/src/main/java/org/apache/ignite/client/ClientCompute.java
##########
@@ -55,9 +55,25 @@
* @return A Future representing pending completion of the task.
* @throws ClientException If task failed.
* @see ComputeTask for information about task execution.
+ * @deprecated Use {@link #executeAsync2(String, Object)} instead.
+ * This method calls {@link #executeAsync2(String, Object)} internally,
but returns a more limited
+ * Future interface.
*/
+ @Deprecated
Review comment:
By the way, do we ever need to leave an old method? I mean,
`IgniteClientFuture` implements`Future`. Are the any cases that we can break by
just changing return type to a child type?
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ClientComputeImpl.java
##########
@@ -115,6 +119,11 @@
return executeAsync0(taskName, arg, dfltGrp, (byte)0, 0L);
}
+ /** {@inheritDoc} */
+ @Override public <T, R> IgniteClientFuture<R> executeAsync2(String
taskName, @Nullable T arg) throws ClientException {
Review comment:
Do we need `executeAsync` at all?
##########
File path:
modules/core/src/main/java/org/apache/ignite/configuration/ClientConfiguration.java
##########
@@ -115,6 +118,9 @@
/** Reconnect throttling retries. See {@code reconnectThrottlingPeriod}. */
private int reconnectThrottlingRetries = 3;
+ /** Executor for async operations continuations. */
+ private Executor asyncContinuationExecutor;
Review comment:
My bad
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/client/thin/ReliableChannel.java
##########
@@ -210,14 +213,95 @@
throw failure;
}
+ /**
+ * Send request and handle response asynchronously.
+ */
+ public <T> IgniteClientFuture<T> serviceAsync(
+ ClientOperation op,
+ Consumer<PayloadOutputChannel> payloadWriter,
+ Function<PayloadInputChannel, T> payloadReader
+ ) throws ClientException, ClientError {
+ CompletableFuture<T> fut = new CompletableFuture<>();
+
+ ClientChannel ch = channel();
+
+ ch.serviceAsync(op, payloadWriter, payloadReader).handle((res, err) ->
+ handleServiceAsync(op, payloadWriter, payloadReader, fut,
null, null, ch, res, err));
+
+ return new IgniteClientFutureImpl<>(fut);
+ }
+
+ /**
+ * Handles serviceAsync results and retries as needed.
+ */
+ private <T> Object handleServiceAsync(ClientOperation op,
+ Consumer<PayloadOutputChannel>
payloadWriter,
+ Function<PayloadInputChannel, T>
payloadReader,
+ CompletableFuture<T> fut,
+ ClientConnectionException failure,
+ AtomicInteger chIdx,
+ ClientChannel ch,
+ T res,
+ Throwable err) {
+ if (err == null) {
+ fut.complete(res);
+ return null;
+ }
+
+ if (err instanceof ClientConnectionException) {
Review comment:
I think, only those operations that do not modify cache state should be
retried or we can get unexpected results, if the operation was successfully
performed, but we were unable to receive response.
Also, it is OK to retry if failure happens on send.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
