timoninmaxim commented on code in PR #12394:
URL: https://github.com/apache/ignite/pull/12394#discussion_r2622301136


##########
modules/indexing/src/test/java/org/apache/ignite/util/KillCommandsTests.java:
##########
@@ -176,6 +187,8 @@ public static void checkScanQueryCancelBeforeFetching(
         checkScanQueryResources(cli, srvs, qryInfo.get3());
 
         qry2.close();
+
+        checkRequestFutureMapEmpty(cli, srvs, qryInfo.get1());

Review Comment:
   Can we check it after `qryCanceler.accept(qryInfo)` and validate that this 
query is cleared immediately after cancelling?



##########
modules/indexing/src/test/java/org/apache/ignite/util/KillCommandsTests.java:
##########
@@ -254,6 +267,40 @@ private static void checkScanQueryResources(IgniteEx cli, 
List<IgniteEx> srvs, l
         }
     }
 
+    /**
+     * Checks that RequestFutureMap is empty on all nodes after query 
cancellation.
+     *
+     * @param cli Client node.
+     * @param srvs Server nodes.
+     * @param originNodeId Origin node ID.
+     */
+    private static void checkRequestFutureMapEmpty(
+            IgniteEx cli,
+            List<IgniteEx> srvs,
+            UUID originNodeId
+    ) {
+        List<IgniteEx> allNodes = new ArrayList<>(srvs);
+        allNodes.add(cli);
+
+        for (IgniteEx node : allNodes) {
+            int cacheId = CU.cacheId(DEFAULT_CACHE_NAME);
+            GridCacheContext<?, ?> ctx = 
node.context().cache().context().cacheContext(cacheId);
+
+            if (ctx == null) {
+                continue;

Review Comment:
   Actually this happens for `cli` node, then you don't need add it to check.



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheDistributedQueryManager.java:
##########
@@ -228,6 +229,17 @@ protected void removeQueryFuture(long reqId) {
                     }
                 }
             }
+            else {
+                GridCacheQueryResponse res = new GridCacheQueryResponse(
+                        cctx.cacheId(),
+                        req.id(),
+                        new QueryCancelledException("Iterator has been 
closed."),
+                        cctx.deploymentEnabled()
+                );
+
+                sendQueryResponse(sndId, res, 0);
+

Review Comment:
   remove this NL



##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheDistributedQueryManager.java:
##########
@@ -228,6 +229,17 @@ protected void removeQueryFuture(long reqId) {
                     }
                 }
             }
+            else {
+                GridCacheQueryResponse res = new GridCacheQueryResponse(
+                        cctx.cacheId(),
+                        req.id(),
+                        new QueryCancelledException("Iterator has been 
closed."),

Review Comment:
   Query has been cancelled



##########
modules/core/src/main/java/org/apache/ignite/internal/QueryMXBeanImpl.java:
##########
@@ -173,7 +175,10 @@ private static class CancelScan implements 
IgniteClosure<T3<UUID, String, Long>,
                 return null;
             }
 
-            ctx.queries().removeQueryResult(arg.get1(), arg.get3());
+            GridCacheQueryRequest cancelReq = 
GridCacheQueryRequest.cancelRequest(ctx, arg.get3(), false);
+
+            GridCacheDistributedQueryManager distQryMgr = 
(GridCacheDistributedQueryManager)ctx.queries();
+            distQryMgr.processQueryRequest(arg.get1(), cancelReq);

Review Comment:
   You can inline all vars as all of them are used only once.



-- 
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]

Reply via email to