xtern commented on code in PR #3953:
URL: https://github.com/apache/ignite-3/pull/3953#discussion_r1650610552
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/QueryCancel.java:
##########
@@ -32,21 +36,66 @@ public class QueryCancel {
private boolean canceled;
+ private boolean timeout;
+
/**
- * Adds a cancel action.
+ * Adds a cancel action. If operation has already been canceled, throws a
{@link QueryCancelledException}.
+ *
+ * <p>NOTE: If the operation is cancelled, this method will immediately
invoke the given action
+ * and then throw a {@link QueryCancelledException}.
*
- * @param clo Add cancel action.
+ * @param clo Cancel action.
*/
public synchronized void add(Cancellable clo) throws
QueryCancelledException {
assert clo != null;
if (canceled) {
- throw new QueryCancelledException();
+ // Immediately invoke a cancel action, if already cancelled.
+ // Otherwise the caller is required to catch
QueryCancelledException and call an action manually.
+ try {
+ clo.cancel(timeout);
+ } catch (Exception ignore) {
+ // Do nothing
+ }
+
+ String message = timeout ? QueryCancelledException.TIMEOUT_MSG :
QueryCancelledException.CANCEL_MSG;
+ throw new QueryCancelledException(message);
}
cancelActions.add(clo);
}
+ /**
+ * Removes the given callback.
+ *
+ * @param clo Callback.
+ */
+ public synchronized void remove(Cancellable clo) {
+ assert clo != null;
+
+ cancelActions.remove(clo);
+ }
+
+ /**
+ * Schedules a timeout action (a call to {@link #timeout()}) after {@code
timeoutMillis} milliseconds.
+ *
+ * @param scheduler Scheduler to trigger an action.
+ * @param timeoutMillis Timeout in milliseconds.
+ * @return Future that will be completed when the timeout is reached.
+ */
+ public CompletableFuture<Void> addTimeout(ScheduledExecutorService
scheduler, long timeoutMillis) {
+ CompletableFuture<Void> fut = new CompletableFuture<>();
+ fut.thenAccept((r) -> timeout());
+
+ ScheduledFuture<?> f = scheduler.schedule(() -> {
+ fut.complete(null);
+ }, timeoutMillis, MILLISECONDS);
+
+ add((timeout) -> f.cancel(false));
Review Comment:
> When a statement is cancelled its timeout should be cancelled as well,
Yes, but when timeout occurred then this future `f` must be already
completed and we don't need to cancel it again?
```
add((timeout) -> {
assert timeout == false || f.isDone(); // <-- is this correct?
f.cancel(false);
}
```
p.s. May be it's not so trivial change (`if (!timeout)`), so I don't insist
to do it, I just want to understand how it works.
--
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]
