You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2020/03/12 08:46:21 UTC

[GitHub] [ignite] nizhikov opened a new pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

nizhikov opened a new pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520
 
 
   This PR contains the support of cancel by id command for resource-consuming entities.
   Cluster administrator supposed user for these commands.
   
   Cancellable entities:
   
     * Scan query.
     * SQL query.
     * Continuous query.
     * Transaction.
     * Compute task
     * Service

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393717443
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/visor/query/VisorQueryCancelTask.java
 ##########
 @@ -17,22 +17,25 @@
 
 package org.apache.ignite.internal.visor.query;
 
+import java.util.Collection;
 
 Review comment:
   What's wrong with it?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393729454
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/visor/query/VisorScanQueryCancelTaskArg.java
 ##########
 @@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.visor.query;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.util.UUID;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorDataTransferObject;
+
+/**
+ * Arguments of task for cancel SCAN query.
+ */
+@GridInternal
+public class VisorScanQueryCancelTaskArg extends VisorDataTransferObject {
 
 Review comment:
   Fixed.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393635791
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/QueryMXBeanImpl.java
 ##########
 @@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal;
+
+import java.util.UUID;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.internal.cluster.IgniteClusterImpl;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.internal.util.typedef.internal.A;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTaskArg;
+import org.apache.ignite.mxbean.QueryMXBean;
+
+import static org.apache.ignite.internal.sql.command.SqlKillQueryCommand.parseGlobalQueryId;
+
+/**
+ * QueryMXBean implementation.
+ */
+public class QueryMXBeanImpl implements QueryMXBean {
+    /** */
+    public static final String EXPECTED_GLOBAL_QRY_ID_FORMAT = "Global query id should have format " +
+        "'{node_id}_{query_id}', e.g. '6fa749ee-7cf8-4635-be10-36a1c75267a7_54321'";
+
+    /** */
+    private final GridKernalContext ctx;
+
+    /** */
+    private final IgniteLogger log;
+
+    /**
+     * @param ctx Context.
+     */
+    public QueryMXBeanImpl(GridKernalContext ctx) {
+        this.ctx = ctx;
+        this.log = ctx.log(QueryMXBeanImpl.class);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelContinuous(String routineId) {
+        A.notNull(routineId, "routineId");
+
+        cancelContinuous(UUID.fromString(routineId));
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelSQL(String id) {
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing sql query[id=" + id + ']');
+
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            T2<UUID, Long> ids = parseGlobalQueryId(id);
+
+            if (ids == null)
+                throw new IllegalArgumentException("Expected global query id. " + EXPECTED_GLOBAL_QRY_ID_FORMAT);
+
+            res = cluster.compute().execute(new VisorQueryCancelTask(),
+                new VisorTaskArgument<>(ids.get1(), new VisorQueryCancelTaskArg(ids.get1(), ids.get2()), false));
+        }
+        catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+        if (!res)
+            throw new RuntimeException("Query not found[id=" + id + ']');
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelScan(String originNodeId, String cacheName, Long id) {
+        A.notNullOrEmpty(originNodeId, "originNodeId");
+        A.notNullOrEmpty(cacheName, "cacheName");
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing scan query[id=" + id + ",originNodeId=" + originNodeId + ']');
+
+        cancelScan(UUID.fromString(originNodeId), cacheName, id);
+    }
+
+    /**
+     * Kills scan query by the identifiers.
+     *
+     * @param originNodeId Originating node id.
+     * @param cacheName Cache name.
+     * @param id Scan query id.
+     */
+    public void cancelScan(UUID originNodeId, String cacheName, long id) {
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            ClusterNode srv = U.randomServerNode(ctx);
+
+            IgniteCompute compute = cluster.compute();
+
+            res = compute.execute(new VisorScanQueryCancelTask(),
+                new VisorTaskArgument<>(srv.id(),
+                    new VisorScanQueryCancelTaskArg(originNodeId, cacheName, id), false));
+        }
+        catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+        if (!res) {
+            throw new RuntimeException("Query not found[originNodeId=" + originNodeId +
+                ",cacheName=" + cacheName + ",qryId=" + id + ']');
+        }
+    }
+
+    /**
+     * Kills continuous query by the identifier.
+     *
+     * @param routineId Routine id.
+     */
+    public void cancelContinuous(UUID routineId) {
+        if (log.isInfoEnabled())
+            log.info("Killing continuous query[routineId=" + routineId + ']');
+
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            IgniteCompute compute = cluster.compute();
+
+            ClusterNode srv = U.randomServerNode(ctx);
 
 Review comment:
   Why random node? Why not send it to the initiator?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393630294
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/visor/query/VisorQueryCancelTask.java
 ##########
 @@ -17,22 +17,25 @@
 
 package org.apache.ignite.internal.visor.query;
 
+import java.util.Collection;
 
 Review comment:
   Imports

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393737299
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/visor/query/VisorQueryCancelTask.java
 ##########
 @@ -17,22 +17,25 @@
 
 package org.apache.ignite.internal.visor.query;
 
+import java.util.Collection;
 
 Review comment:
   Ok, might be I missed it. You are right.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393618739
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StopRoutineDiscoveryMessage.java
 ##########
 @@ -29,11 +29,24 @@
     /** */
     private static final long serialVersionUID = 0L;
 
+    /** {@code True} if stop should be forced. */
+    private boolean force;
 
 Review comment:
   Could you explain the reason behind this flag? Why can't we cancel CQ without it?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393748169
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/visor/query/VisorContinuousQueryCancelTaskArg.java
 ##########
 @@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.visor.query;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.util.UUID;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorDataTransferObject;
+
+/**
+ * Arguments of task for cancel CONTINUOUS query.
+ */
+@GridInternal
+public class VisorContinuousQueryCancelTaskArg extends VisorDataTransferObject {
 
 Review comment:
   fixed.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393725638
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/visor/query/VisorQueryCancelTask.java
 ##########
 @@ -17,22 +17,25 @@
 
 package org.apache.ignite.internal.visor.query;
 
+import java.util.Collection;
 
 Review comment:
   Unused imports. At least my IDE thinks so.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393631424
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/visor/service/VisorCancelServiceTask.java
 ##########
 @@ -56,12 +60,30 @@ protected VisorCancelServiceJob(VisorCancelServiceTaskArg arg, boolean debug) {
         }
 
         /** {@inheritDoc} */
-        @Override protected Void run(final VisorCancelServiceTaskArg arg) {
+        @Override protected Boolean run(final VisorCancelServiceTaskArg arg) {
             IgniteServices services = ignite.services();
 
-            services.cancel(arg.getName());
+            String svcName = arg.getName();
 
-            return null;
+            Optional<ServiceDescriptor> svc = services.serviceDescriptors().stream()
+                .filter(d -> d.name().equalsIgnoreCase(svcName))
+                .findFirst();
+
+            if (!svc.isPresent())
+                return false;
+
+            try {
+                services.cancel(svcName);
+            }
+            catch (IgniteException e) {
+                IgniteLogger log = ignite.log().getLogger(VisorCancelServiceTask.class);
+
+                log.warning("Error on service cance.", e);
 
 Review comment:
   Typo

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393644041
 
 

 ##########
 File path: modules/indexing/src/test/java/org/apache/ignite/util/KillCommandsCommandShTest.java
 ##########
 @@ -0,0 +1,177 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.util;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+import org.apache.ignite.IgniteCache;
+import org.apache.ignite.cache.CacheAtomicityMode;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.lang.IgniteUuid;
+import org.junit.Test;
+
+import static org.apache.ignite.internal.commandline.CommandHandler.EXIT_CODE_OK;
+import static org.apache.ignite.internal.commandline.CommandHandler.EXIT_CODE_UNEXPECTED_ERROR;
+import static org.apache.ignite.util.KillCommandsTests.PAGE_SZ;
+import static org.apache.ignite.util.KillCommandsTests.doTestCancelComputeTask;
+import static org.apache.ignite.util.KillCommandsTests.doTestCancelContinuousQuery;
+import static org.apache.ignite.util.KillCommandsTests.doTestCancelSQLQuery;
+import static org.apache.ignite.util.KillCommandsTests.doTestCancelService;
+import static org.apache.ignite.util.KillCommandsTests.doTestCancelTx;
+import static org.apache.ignite.util.KillCommandsTests.doTestScanQueryCancel;
+
+/** Tests cancel of user created entities via control.sh. */
+public class KillCommandsCommandShTest extends GridCommandHandlerClusterByClassAbstractTest {
+    /** */
+    private static List<IgniteEx> srvs;
+
+    /** {@inheritDoc} */
+    @Override protected void beforeTestsStarted() throws Exception {
+        super.beforeTestsStarted();
+
+        srvs = new ArrayList<>();
+
+        for (int i = 0; i < SERVER_NODE_CNT; i++)
+            srvs.add(grid(i));
+
+        System.out.println("CancelCommandCommandShTest.beforeTestsStarted");
 
 Review comment:
   Debugging artifact?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393629975
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/visor/query/VisorContinuousQueryCancelTaskArg.java
 ##########
 @@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.visor.query;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.util.UUID;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorDataTransferObject;
+
+/**
+ * Arguments of task for cancel CONTINUOUS query.
+ */
+@GridInternal
+public class VisorContinuousQueryCancelTaskArg extends VisorDataTransferObject {
 
 Review comment:
   `VisorDataTransferObject` is deprecated.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393642792
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/ServiceMXBeanImpl.java
 ##########
 @@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal;
+
+import java.util.UUID;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.internal.cluster.IgniteClusterImpl;
+import org.apache.ignite.internal.util.typedef.internal.A;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+import org.apache.ignite.internal.visor.service.VisorCancelServiceTask;
+import org.apache.ignite.internal.visor.service.VisorCancelServiceTaskArg;
+import org.apache.ignite.mxbean.ServiceMXBean;
+
+/**
+ * ServiceMXBean implementation.
+ */
+public class ServiceMXBeanImpl implements ServiceMXBean {
+    /** */
+    private final GridKernalContext ctx;
+
+    /** */
+    private final IgniteLogger log;
+
+    /**
+     * @param ctx Context.
+     */
+    public ServiceMXBeanImpl(GridKernalContext ctx) {
+        this.ctx = ctx;
+        this.log = ctx.log(ServiceMXBeanImpl.class);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancel(String name) {
+        A.notNull(name, "name");
+
+        if (log.isInfoEnabled())
+            log.info("Canceling service[name=" + name + ']');
+
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            IgniteCompute compute = cluster.compute();
+
+            ClusterNode srv = U.randomServerNode(ctx);
+
+            res = compute.execute(new VisorCancelServiceTask(),
 
 Review comment:
   Why can't we run this task from the current node?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393742993
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/QueryMXBeanImpl.java
 ##########
 @@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal;
+
+import java.util.UUID;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.internal.cluster.IgniteClusterImpl;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.internal.util.typedef.internal.A;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTaskArg;
+import org.apache.ignite.mxbean.QueryMXBean;
+
+import static org.apache.ignite.internal.sql.command.SqlKillQueryCommand.parseGlobalQueryId;
+
+/**
+ * QueryMXBean implementation.
+ */
+public class QueryMXBeanImpl implements QueryMXBean {
+    /** */
+    public static final String EXPECTED_GLOBAL_QRY_ID_FORMAT = "Global query id should have format " +
+        "'{node_id}_{query_id}', e.g. '6fa749ee-7cf8-4635-be10-36a1c75267a7_54321'";
+
+    /** */
+    private final GridKernalContext ctx;
+
+    /** */
+    private final IgniteLogger log;
+
+    /**
+     * @param ctx Context.
+     */
+    public QueryMXBeanImpl(GridKernalContext ctx) {
+        this.ctx = ctx;
+        this.log = ctx.log(QueryMXBeanImpl.class);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelContinuous(String routineId) {
+        A.notNull(routineId, "routineId");
+
+        cancelContinuous(UUID.fromString(routineId));
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelSQL(String id) {
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing sql query[id=" + id + ']');
+
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            T2<UUID, Long> ids = parseGlobalQueryId(id);
+
+            if (ids == null)
+                throw new IllegalArgumentException("Expected global query id. " + EXPECTED_GLOBAL_QRY_ID_FORMAT);
+
+            res = cluster.compute().execute(new VisorQueryCancelTask(),
+                new VisorTaskArgument<>(ids.get1(), new VisorQueryCancelTaskArg(ids.get1(), ids.get2()), false));
+        }
+        catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+        if (!res)
+            throw new RuntimeException("Query not found[id=" + id + ']');
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelScan(String originNodeId, String cacheName, Long id) {
+        A.notNullOrEmpty(originNodeId, "originNodeId");
+        A.notNullOrEmpty(cacheName, "cacheName");
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing scan query[id=" + id + ",originNodeId=" + originNodeId + ']');
+
+        cancelScan(UUID.fromString(originNodeId), cacheName, id);
+    }
+
+    /**
+     * Kills scan query by the identifiers.
+     *
+     * @param originNodeId Originating node id.
+     * @param cacheName Cache name.
+     * @param id Scan query id.
+     */
+    public void cancelScan(UUID originNodeId, String cacheName, long id) {
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            ClusterNode srv = U.randomServerNode(ctx);
+
+            IgniteCompute compute = cluster.compute();
+
+            res = compute.execute(new VisorScanQueryCancelTask(),
+                new VisorTaskArgument<>(srv.id(),
+                    new VisorScanQueryCancelTaskArg(originNodeId, cacheName, id), false));
+        }
+        catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+        if (!res) {
+            throw new RuntimeException("Query not found[originNodeId=" + originNodeId +
+                ",cacheName=" + cacheName + ",qryId=" + id + ']');
+        }
+    }
+
+    /**
+     * Kills continuous query by the identifier.
+     *
+     * @param routineId Routine id.
+     */
+    public void cancelContinuous(UUID routineId) {
+        if (log.isInfoEnabled())
+            log.info("Killing continuous query[routineId=" + routineId + ']');
+
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            IgniteCompute compute = cluster.compute();
+
+            ClusterNode srv = U.randomServerNode(ctx);
 
 Review comment:
   > Why it's not a good idea to send cancel message to the initiator
   
   My opinion - canceling of the query is the "last line of defense".
   When an administrator decides to cancel a query it means something goes wrong.
   The whole initiator node can be flooded with the entries update.
   So I think it's not the best choice to send a cancel request to the initiator.
   
   > Why random node?
   
   CQ cancel implemented via discovery message so there is no difference what specific node receives request first. 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393629901
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/visor/query/VisorScanQueryCancelTaskArg.java
 ##########
 @@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.visor.query;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.util.UUID;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorDataTransferObject;
+
+/**
+ * Arguments of task for cancel SCAN query.
+ */
+@GridInternal
+public class VisorScanQueryCancelTaskArg extends VisorDataTransferObject {
 
 Review comment:
   `VisorDataTransferObject` is deprecated.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393641141
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/QueryMXBeanImpl.java
 ##########
 @@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal;
+
+import java.util.UUID;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.internal.cluster.IgniteClusterImpl;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.internal.util.typedef.internal.A;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTaskArg;
+import org.apache.ignite.mxbean.QueryMXBean;
+
+import static org.apache.ignite.internal.sql.command.SqlKillQueryCommand.parseGlobalQueryId;
+
+/**
+ * QueryMXBean implementation.
+ */
+public class QueryMXBeanImpl implements QueryMXBean {
+    /** */
+    public static final String EXPECTED_GLOBAL_QRY_ID_FORMAT = "Global query id should have format " +
+        "'{node_id}_{query_id}', e.g. '6fa749ee-7cf8-4635-be10-36a1c75267a7_54321'";
+
+    /** */
+    private final GridKernalContext ctx;
+
+    /** */
+    private final IgniteLogger log;
+
+    /**
+     * @param ctx Context.
+     */
+    public QueryMXBeanImpl(GridKernalContext ctx) {
+        this.ctx = ctx;
+        this.log = ctx.log(QueryMXBeanImpl.class);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelContinuous(String routineId) {
+        A.notNull(routineId, "routineId");
+
+        cancelContinuous(UUID.fromString(routineId));
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelSQL(String id) {
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing sql query[id=" + id + ']');
+
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            T2<UUID, Long> ids = parseGlobalQueryId(id);
+
+            if (ids == null)
+                throw new IllegalArgumentException("Expected global query id. " + EXPECTED_GLOBAL_QRY_ID_FORMAT);
+
+            res = cluster.compute().execute(new VisorQueryCancelTask(),
+                new VisorTaskArgument<>(ids.get1(), new VisorQueryCancelTaskArg(ids.get1(), ids.get2()), false));
+        }
+        catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+        if (!res)
+            throw new RuntimeException("Query not found[id=" + id + ']');
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelScan(String originNodeId, String cacheName, Long id) {
+        A.notNullOrEmpty(originNodeId, "originNodeId");
+        A.notNullOrEmpty(cacheName, "cacheName");
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing scan query[id=" + id + ",originNodeId=" + originNodeId + ']');
+
+        cancelScan(UUID.fromString(originNodeId), cacheName, id);
+    }
+
+    /**
+     * Kills scan query by the identifiers.
+     *
+     * @param originNodeId Originating node id.
+     * @param cacheName Cache name.
+     * @param id Scan query id.
+     */
+    public void cancelScan(UUID originNodeId, String cacheName, long id) {
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            ClusterNode srv = U.randomServerNode(ctx);
 
 Review comment:
   Why do we use random node here? IMO we need to send task to the origin node and run `GridCacheDistributedQueryFuture#cancelQuery` there.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r394175006
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/QueryMXBeanImpl.java
 ##########
 @@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal;
+
+import java.util.UUID;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.internal.cluster.IgniteClusterImpl;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.internal.util.typedef.internal.A;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTaskArg;
+import org.apache.ignite.mxbean.QueryMXBean;
+
+import static org.apache.ignite.internal.sql.command.SqlKillQueryCommand.parseGlobalQueryId;
+
+/**
+ * QueryMXBean implementation.
+ */
+public class QueryMXBeanImpl implements QueryMXBean {
+    /** */
+    public static final String EXPECTED_GLOBAL_QRY_ID_FORMAT = "Global query id should have format " +
+        "'{node_id}_{query_id}', e.g. '6fa749ee-7cf8-4635-be10-36a1c75267a7_54321'";
+
+    /** */
+    private final GridKernalContext ctx;
+
+    /** */
+    private final IgniteLogger log;
+
+    /**
+     * @param ctx Context.
+     */
+    public QueryMXBeanImpl(GridKernalContext ctx) {
+        this.ctx = ctx;
+        this.log = ctx.log(QueryMXBeanImpl.class);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelContinuous(String routineId) {
+        A.notNull(routineId, "routineId");
+
+        cancelContinuous(UUID.fromString(routineId));
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelSQL(String id) {
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing sql query[id=" + id + ']');
+
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            T2<UUID, Long> ids = parseGlobalQueryId(id);
+
+            if (ids == null)
+                throw new IllegalArgumentException("Expected global query id. " + EXPECTED_GLOBAL_QRY_ID_FORMAT);
+
+            res = cluster.compute().execute(new VisorQueryCancelTask(),
+                new VisorTaskArgument<>(ids.get1(), new VisorQueryCancelTaskArg(ids.get1(), ids.get2()), false));
+        }
+        catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+        if (!res)
+            throw new RuntimeException("Query not found[id=" + id + ']');
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelScan(String originNodeId, String cacheName, Long id) {
+        A.notNullOrEmpty(originNodeId, "originNodeId");
+        A.notNullOrEmpty(cacheName, "cacheName");
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing scan query[id=" + id + ",originNodeId=" + originNodeId + ']');
+
+        cancelScan(UUID.fromString(originNodeId), cacheName, id);
+    }
+
+    /**
+     * Kills scan query by the identifiers.
+     *
+     * @param originNodeId Originating node id.
+     * @param cacheName Cache name.
+     * @param id Scan query id.
+     */
+    public void cancelScan(UUID originNodeId, String cacheName, long id) {
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            ClusterNode srv = U.randomServerNode(ctx);
+
+            IgniteCompute compute = cluster.compute();
+
+            res = compute.execute(new VisorScanQueryCancelTask(),
+                new VisorTaskArgument<>(srv.id(),
+                    new VisorScanQueryCancelTaskArg(originNodeId, cacheName, id), false));
+        }
+        catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+        if (!res) {
+            throw new RuntimeException("Query not found[originNodeId=" + originNodeId +
+                ",cacheName=" + cacheName + ",qryId=" + id + ']');
+        }
+    }
+
+    /**
+     * Kills continuous query by the identifier.
+     *
+     * @param routineId Routine id.
+     */
+    public void cancelContinuous(UUID routineId) {
+        if (log.isInfoEnabled())
+            log.info("Killing continuous query[routineId=" + routineId + ']');
+
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            IgniteCompute compute = cluster.compute();
+
+            ClusterNode srv = U.randomServerNode(ctx);
 
 Review comment:
   I think the cancellation task should be delegated to the initiator. See the explanation below.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393731396
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/visor/query/VisorQueryCancelTask.java
 ##########
 @@ -17,22 +17,25 @@
 
 package org.apache.ignite.internal.visor.query;
 
+import java.util.Collection;
 
 Review comment:
   it used in the method `jobNodes`:
   
   ```
       @Override protected Collection<UUID> jobNodes(VisorTaskArgument<VisorQueryCancelTaskArg> arg) {
           return Collections.singleton(arg.getArgument().getNodeId());
       }
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r394196943
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/QueryMXBeanImpl.java
 ##########
 @@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal;
+
+import java.util.UUID;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.internal.cluster.IgniteClusterImpl;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.internal.util.typedef.internal.A;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTaskArg;
+import org.apache.ignite.mxbean.QueryMXBean;
+
+import static org.apache.ignite.internal.sql.command.SqlKillQueryCommand.parseGlobalQueryId;
+
+/**
+ * QueryMXBean implementation.
+ */
+public class QueryMXBeanImpl implements QueryMXBean {
+    /** */
+    public static final String EXPECTED_GLOBAL_QRY_ID_FORMAT = "Global query id should have format " +
+        "'{node_id}_{query_id}', e.g. '6fa749ee-7cf8-4635-be10-36a1c75267a7_54321'";
+
+    /** */
+    private final GridKernalContext ctx;
+
+    /** */
+    private final IgniteLogger log;
+
+    /**
+     * @param ctx Context.
+     */
+    public QueryMXBeanImpl(GridKernalContext ctx) {
+        this.ctx = ctx;
+        this.log = ctx.log(QueryMXBeanImpl.class);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelContinuous(String routineId) {
+        A.notNull(routineId, "routineId");
+
+        cancelContinuous(UUID.fromString(routineId));
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelSQL(String id) {
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing sql query[id=" + id + ']');
+
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            T2<UUID, Long> ids = parseGlobalQueryId(id);
+
+            if (ids == null)
+                throw new IllegalArgumentException("Expected global query id. " + EXPECTED_GLOBAL_QRY_ID_FORMAT);
+
+            res = cluster.compute().execute(new VisorQueryCancelTask(),
+                new VisorTaskArgument<>(ids.get1(), new VisorQueryCancelTaskArg(ids.get1(), ids.get2()), false));
+        }
+        catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+        if (!res)
+            throw new RuntimeException("Query not found[id=" + id + ']');
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelScan(String originNodeId, String cacheName, Long id) {
+        A.notNullOrEmpty(originNodeId, "originNodeId");
+        A.notNullOrEmpty(cacheName, "cacheName");
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing scan query[id=" + id + ",originNodeId=" + originNodeId + ']');
+
+        cancelScan(UUID.fromString(originNodeId), cacheName, id);
+    }
+
+    /**
+     * Kills scan query by the identifiers.
+     *
+     * @param originNodeId Originating node id.
+     * @param cacheName Cache name.
+     * @param id Scan query id.
+     */
+    public void cancelScan(UUID originNodeId, String cacheName, long id) {
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            ClusterNode srv = U.randomServerNode(ctx);
 
 Review comment:
   Every node in the cluster can be overloaded, even the one where we are going to send the kill task from. This is not an issue.
   We should have a single straightforward protocol for starting and cancelling the tasks. Existing protocol for cancelling tasks from the initiator is a good candidate for it. If instead we have a bunch of such protocols we will end up with a messy code and numerous subtle bugs which are not very easy to cope with. 
   I think that proposed approach is not bug-free and there are possible races between task start and task cancellation messages. What would happen if cancel task request arrived before start task request? I am not sure this situation will be gracefully handled.
   IMO we need to stick the approach with a single cancellation protocol from the initiator. It is solid and less erroneous solution.
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393747287
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/QueryMXBeanImpl.java
 ##########
 @@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal;
+
+import java.util.UUID;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.internal.cluster.IgniteClusterImpl;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.internal.util.typedef.internal.A;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTaskArg;
+import org.apache.ignite.mxbean.QueryMXBean;
+
+import static org.apache.ignite.internal.sql.command.SqlKillQueryCommand.parseGlobalQueryId;
+
+/**
+ * QueryMXBean implementation.
+ */
+public class QueryMXBeanImpl implements QueryMXBean {
+    /** */
+    public static final String EXPECTED_GLOBAL_QRY_ID_FORMAT = "Global query id should have format " +
+        "'{node_id}_{query_id}', e.g. '6fa749ee-7cf8-4635-be10-36a1c75267a7_54321'";
+
+    /** */
+    private final GridKernalContext ctx;
+
+    /** */
+    private final IgniteLogger log;
+
+    /**
+     * @param ctx Context.
+     */
+    public QueryMXBeanImpl(GridKernalContext ctx) {
+        this.ctx = ctx;
+        this.log = ctx.log(QueryMXBeanImpl.class);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelContinuous(String routineId) {
+        A.notNull(routineId, "routineId");
+
+        cancelContinuous(UUID.fromString(routineId));
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelSQL(String id) {
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing sql query[id=" + id + ']');
+
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            T2<UUID, Long> ids = parseGlobalQueryId(id);
+
+            if (ids == null)
+                throw new IllegalArgumentException("Expected global query id. " + EXPECTED_GLOBAL_QRY_ID_FORMAT);
+
+            res = cluster.compute().execute(new VisorQueryCancelTask(),
+                new VisorTaskArgument<>(ids.get1(), new VisorQueryCancelTaskArg(ids.get1(), ids.get2()), false));
+        }
+        catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+        if (!res)
+            throw new RuntimeException("Query not found[id=" + id + ']');
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelScan(String originNodeId, String cacheName, Long id) {
+        A.notNullOrEmpty(originNodeId, "originNodeId");
+        A.notNullOrEmpty(cacheName, "cacheName");
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing scan query[id=" + id + ",originNodeId=" + originNodeId + ']');
+
+        cancelScan(UUID.fromString(originNodeId), cacheName, id);
+    }
+
+    /**
+     * Kills scan query by the identifiers.
+     *
+     * @param originNodeId Originating node id.
+     * @param cacheName Cache name.
+     * @param id Scan query id.
+     */
+    public void cancelScan(UUID originNodeId, String cacheName, long id) {
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            ClusterNode srv = U.randomServerNode(ctx);
 
 Review comment:
   Same of previous:
   
   > Why it's not a good idea to send cancel message to the initiator
   
   My opinion - canceling of the query is the "last line of defense".
   When an administrator decides to cancel a query it means something goes wrong.
   The whole initiator node can be flooded and respond slowly.
   So I think it's not the best choice to send a cancel request to the initiator.
   
   > Why random node?
   
   Actually, scan query cancel executed on all cluster nodes it implemented in the task itself see VisorScanQueryCancelTask#jobNodes:
   ```
       /** {@inheritDoc} */
       @Override protected Collection<UUID> jobNodes(VisorTaskArgument<VisorScanQueryCancelTaskArg> arg) {
           return F.transform(ignite.cluster().nodes(), ClusterNode::id);
       }
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] nizhikov closed pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
nizhikov closed pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520
 
 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r394184783
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/QueryMXBeanImpl.java
 ##########
 @@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal;
+
+import java.util.UUID;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.internal.cluster.IgniteClusterImpl;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.internal.util.typedef.internal.A;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTaskArg;
+import org.apache.ignite.mxbean.QueryMXBean;
+
+import static org.apache.ignite.internal.sql.command.SqlKillQueryCommand.parseGlobalQueryId;
+
+/**
+ * QueryMXBean implementation.
+ */
+public class QueryMXBeanImpl implements QueryMXBean {
+    /** */
+    public static final String EXPECTED_GLOBAL_QRY_ID_FORMAT = "Global query id should have format " +
+        "'{node_id}_{query_id}', e.g. '6fa749ee-7cf8-4635-be10-36a1c75267a7_54321'";
+
+    /** */
+    private final GridKernalContext ctx;
+
+    /** */
+    private final IgniteLogger log;
+
+    /**
+     * @param ctx Context.
+     */
+    public QueryMXBeanImpl(GridKernalContext ctx) {
+        this.ctx = ctx;
+        this.log = ctx.log(QueryMXBeanImpl.class);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelContinuous(String routineId) {
+        A.notNull(routineId, "routineId");
+
+        cancelContinuous(UUID.fromString(routineId));
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelSQL(String id) {
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing sql query[id=" + id + ']');
+
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            T2<UUID, Long> ids = parseGlobalQueryId(id);
+
+            if (ids == null)
+                throw new IllegalArgumentException("Expected global query id. " + EXPECTED_GLOBAL_QRY_ID_FORMAT);
+
+            res = cluster.compute().execute(new VisorQueryCancelTask(),
+                new VisorTaskArgument<>(ids.get1(), new VisorQueryCancelTaskArg(ids.get1(), ids.get2()), false));
+        }
+        catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+        if (!res)
+            throw new RuntimeException("Query not found[id=" + id + ']');
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelScan(String originNodeId, String cacheName, Long id) {
+        A.notNullOrEmpty(originNodeId, "originNodeId");
+        A.notNullOrEmpty(cacheName, "cacheName");
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing scan query[id=" + id + ",originNodeId=" + originNodeId + ']');
+
+        cancelScan(UUID.fromString(originNodeId), cacheName, id);
+    }
+
+    /**
+     * Kills scan query by the identifiers.
+     *
+     * @param originNodeId Originating node id.
+     * @param cacheName Cache name.
+     * @param id Scan query id.
+     */
+    public void cancelScan(UUID originNodeId, String cacheName, long id) {
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            ClusterNode srv = U.randomServerNode(ctx);
 
 Review comment:
   > And this entry point should be located on the initiator.
   > Convenience: single entry point is easier to control
   
   The initiator is the reason for the issue.
   Imagine a case when CPU usage on the initiator is 100% because it flooded with the query results. In that case, the initiator will handle cancel message very slowly that can lead to a whole cluster disaster. 
   
   I think we shouldn't wait until the initiator can proceed cancel message.
   We should cancel it as fast as we can.
   
   > Effectiveness: we send cancellation task to all cluster nodes, but it might be excessive
   
   We can try to reduce the number of nodes to the number of affinity nodes.
   Anyway, cancellation of the query is not a performance-critical operation.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r394174120
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/QueryMXBeanImpl.java
 ##########
 @@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal;
+
+import java.util.UUID;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.internal.cluster.IgniteClusterImpl;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.internal.util.typedef.internal.A;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTaskArg;
+import org.apache.ignite.mxbean.QueryMXBean;
+
+import static org.apache.ignite.internal.sql.command.SqlKillQueryCommand.parseGlobalQueryId;
+
+/**
+ * QueryMXBean implementation.
+ */
+public class QueryMXBeanImpl implements QueryMXBean {
+    /** */
+    public static final String EXPECTED_GLOBAL_QRY_ID_FORMAT = "Global query id should have format " +
+        "'{node_id}_{query_id}', e.g. '6fa749ee-7cf8-4635-be10-36a1c75267a7_54321'";
+
+    /** */
+    private final GridKernalContext ctx;
+
+    /** */
+    private final IgniteLogger log;
+
+    /**
+     * @param ctx Context.
+     */
+    public QueryMXBeanImpl(GridKernalContext ctx) {
+        this.ctx = ctx;
+        this.log = ctx.log(QueryMXBeanImpl.class);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelContinuous(String routineId) {
+        A.notNull(routineId, "routineId");
+
+        cancelContinuous(UUID.fromString(routineId));
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelSQL(String id) {
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing sql query[id=" + id + ']');
+
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            T2<UUID, Long> ids = parseGlobalQueryId(id);
+
+            if (ids == null)
+                throw new IllegalArgumentException("Expected global query id. " + EXPECTED_GLOBAL_QRY_ID_FORMAT);
+
+            res = cluster.compute().execute(new VisorQueryCancelTask(),
+                new VisorTaskArgument<>(ids.get1(), new VisorQueryCancelTaskArg(ids.get1(), ids.get2()), false));
+        }
+        catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+        if (!res)
+            throw new RuntimeException("Query not found[id=" + id + ']');
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelScan(String originNodeId, String cacheName, Long id) {
+        A.notNullOrEmpty(originNodeId, "originNodeId");
+        A.notNullOrEmpty(cacheName, "cacheName");
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing scan query[id=" + id + ",originNodeId=" + originNodeId + ']');
+
+        cancelScan(UUID.fromString(originNodeId), cacheName, id);
+    }
+
+    /**
+     * Kills scan query by the identifiers.
+     *
+     * @param originNodeId Originating node id.
+     * @param cacheName Cache name.
+     * @param id Scan query id.
+     */
+    public void cancelScan(UUID originNodeId, String cacheName, long id) {
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            ClusterNode srv = U.randomServerNode(ctx);
 
 Review comment:
   I think we need to have a single entry point for the query cancellation. And this entry point should be located on the initiator. I can name a few reasons for it:
   
   1. Convenience: single entry point is easier to control. If we want to add extra functionality to it (i.e. track statistics),  we need to add it to the just one place.
   2. Clean code: it is much easier to understand the code which has only one entry point for each operation.
   3. Effectiveness: we send cancellation task to the all cluster nodes, but it might be excessive. The query can be finished on some nodes by the time of cancellation. Or query might be initially executed on a few nodes or a single node due to partition pruning. Only the initiator knows the actual query status and mapping, and therefore it should be responsible for the cancellation.
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393716433
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java
 ##########
 @@ -1962,6 +1962,21 @@ public String cacheName() {
         return cacheName;
     }
 
+    /**
+     * Cancel scan query.
+     *
+     * @param origNodeId Originating node id.
+     * @param qryId Query id.
+     */
+    public boolean cancelScanQuery(UUID origNodeId, long qryId) {
 
 Review comment:
   Fixed. Thanks.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393628835
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryManager.java
 ##########
 @@ -1962,6 +1962,21 @@ public String cacheName() {
         return cacheName;
     }
 
+    /**
+     * Cancel scan query.
+     *
+     * @param origNodeId Originating node id.
+     * @param qryId Query id.
+     */
+    public boolean cancelScanQuery(UUID origNodeId, long qryId) {
 
 Review comment:
   `GridCacheQueryManager#removeQueryResult`?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393747647
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/ServiceMXBeanImpl.java
 ##########
 @@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal;
+
+import java.util.UUID;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.internal.cluster.IgniteClusterImpl;
+import org.apache.ignite.internal.util.typedef.internal.A;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+import org.apache.ignite.internal.visor.service.VisorCancelServiceTask;
+import org.apache.ignite.internal.visor.service.VisorCancelServiceTaskArg;
+import org.apache.ignite.mxbean.ServiceMXBean;
+
+/**
+ * ServiceMXBean implementation.
+ */
+public class ServiceMXBeanImpl implements ServiceMXBean {
+    /** */
+    private final GridKernalContext ctx;
+
+    /** */
+    private final IgniteLogger log;
+
+    /**
+     * @param ctx Context.
+     */
+    public ServiceMXBeanImpl(GridKernalContext ctx) {
+        this.ctx = ctx;
+        this.log = ctx.log(ServiceMXBeanImpl.class);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancel(String name) {
+        A.notNull(name, "name");
+
+        if (log.isInfoEnabled())
+            log.info("Canceling service[name=" + name + ']');
+
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            IgniteCompute compute = cluster.compute();
+
+            ClusterNode srv = U.randomServerNode(ctx);
+
+            res = compute.execute(new VisorCancelServiceTask(),
 
 Review comment:
   Yes, we can.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r394374809
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/QueryMXBeanImpl.java
 ##########
 @@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal;
+
+import java.util.UUID;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.internal.cluster.IgniteClusterImpl;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.internal.util.typedef.internal.A;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTaskArg;
+import org.apache.ignite.mxbean.QueryMXBean;
+
+import static org.apache.ignite.internal.sql.command.SqlKillQueryCommand.parseGlobalQueryId;
+
+/**
+ * QueryMXBean implementation.
+ */
+public class QueryMXBeanImpl implements QueryMXBean {
+    /** */
+    public static final String EXPECTED_GLOBAL_QRY_ID_FORMAT = "Global query id should have format " +
+        "'{node_id}_{query_id}', e.g. '6fa749ee-7cf8-4635-be10-36a1c75267a7_54321'";
+
+    /** */
+    private final GridKernalContext ctx;
+
+    /** */
+    private final IgniteLogger log;
+
+    /**
+     * @param ctx Context.
+     */
+    public QueryMXBeanImpl(GridKernalContext ctx) {
+        this.ctx = ctx;
+        this.log = ctx.log(QueryMXBeanImpl.class);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelContinuous(String routineId) {
+        A.notNull(routineId, "routineId");
+
+        cancelContinuous(UUID.fromString(routineId));
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelSQL(String id) {
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing sql query[id=" + id + ']');
+
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            T2<UUID, Long> ids = parseGlobalQueryId(id);
+
+            if (ids == null)
+                throw new IllegalArgumentException("Expected global query id. " + EXPECTED_GLOBAL_QRY_ID_FORMAT);
+
+            res = cluster.compute().execute(new VisorQueryCancelTask(),
+                new VisorTaskArgument<>(ids.get1(), new VisorQueryCancelTaskArg(ids.get1(), ids.get2()), false));
+        }
+        catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+        if (!res)
+            throw new RuntimeException("Query not found[id=" + id + ']');
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelScan(String originNodeId, String cacheName, Long id) {
+        A.notNullOrEmpty(originNodeId, "originNodeId");
+        A.notNullOrEmpty(cacheName, "cacheName");
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing scan query[id=" + id + ",originNodeId=" + originNodeId + ']');
+
+        cancelScan(UUID.fromString(originNodeId), cacheName, id);
+    }
+
+    /**
+     * Kills scan query by the identifiers.
+     *
+     * @param originNodeId Originating node id.
+     * @param cacheName Cache name.
+     * @param id Scan query id.
+     */
+    public void cancelScan(UUID originNodeId, String cacheName, long id) {
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            ClusterNode srv = U.randomServerNode(ctx);
 
 Review comment:
   > Every node in the cluster can be overloaded, even the one where we are going to send the kill task from. 
   > This is not an issue.
   
   This is the issue I'm trying to solve with these changes.
   
   > I think that proposed approach is not bug-free
   
   Can you, please, clarify, what tests should be added to check the changes?
   
   > What would happen if cancel task request arrived before start task request? 
   
   Then we would get an error "Query not found".

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r395531026
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/QueryMXBeanImpl.java
 ##########
 @@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal;
+
+import java.util.UUID;
+import org.apache.ignite.IgniteCompute;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.cluster.ClusterNode;
+import org.apache.ignite.internal.cluster.IgniteClusterImpl;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.internal.util.typedef.internal.A;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorContinuousQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorQueryCancelTaskArg;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTask;
+import org.apache.ignite.internal.visor.query.VisorScanQueryCancelTaskArg;
+import org.apache.ignite.mxbean.QueryMXBean;
+
+import static org.apache.ignite.internal.sql.command.SqlKillQueryCommand.parseGlobalQueryId;
+
+/**
+ * QueryMXBean implementation.
+ */
+public class QueryMXBeanImpl implements QueryMXBean {
+    /** */
+    public static final String EXPECTED_GLOBAL_QRY_ID_FORMAT = "Global query id should have format " +
+        "'{node_id}_{query_id}', e.g. '6fa749ee-7cf8-4635-be10-36a1c75267a7_54321'";
+
+    /** */
+    private final GridKernalContext ctx;
+
+    /** */
+    private final IgniteLogger log;
+
+    /**
+     * @param ctx Context.
+     */
+    public QueryMXBeanImpl(GridKernalContext ctx) {
+        this.ctx = ctx;
+        this.log = ctx.log(QueryMXBeanImpl.class);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelContinuous(String routineId) {
+        A.notNull(routineId, "routineId");
+
+        cancelContinuous(UUID.fromString(routineId));
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelSQL(String id) {
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing sql query[id=" + id + ']');
+
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            T2<UUID, Long> ids = parseGlobalQueryId(id);
+
+            if (ids == null)
+                throw new IllegalArgumentException("Expected global query id. " + EXPECTED_GLOBAL_QRY_ID_FORMAT);
+
+            res = cluster.compute().execute(new VisorQueryCancelTask(),
+                new VisorTaskArgument<>(ids.get1(), new VisorQueryCancelTaskArg(ids.get1(), ids.get2()), false));
+        }
+        catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+
+        if (!res)
+            throw new RuntimeException("Query not found[id=" + id + ']');
+    }
+
+    /** {@inheritDoc} */
+    @Override public void cancelScan(String originNodeId, String cacheName, Long id) {
+        A.notNullOrEmpty(originNodeId, "originNodeId");
+        A.notNullOrEmpty(cacheName, "cacheName");
+        A.notNull(id, "id");
+
+        if (log.isInfoEnabled())
+            log.info("Killing scan query[id=" + id + ",originNodeId=" + originNodeId + ']');
+
+        cancelScan(UUID.fromString(originNodeId), cacheName, id);
+    }
+
+    /**
+     * Kills scan query by the identifiers.
+     *
+     * @param originNodeId Originating node id.
+     * @param cacheName Cache name.
+     * @param id Scan query id.
+     */
+    public void cancelScan(UUID originNodeId, String cacheName, long id) {
+        boolean res;
+
+        try {
+            IgniteClusterImpl cluster = ctx.cluster().get();
+
+            ClusterNode srv = U.randomServerNode(ctx);
 
 Review comment:
   We will end up with memory and resources leak. Because the cancel task message arrives first and we get  "Query not found" here. But later the start task message arrives, the task starts execution but no one will cancel it because cancel task will not be resent.
   
   > Then we would get an error "Query not found".
   
   You may use custom communication SPI to delay start task messages and reorder them with cancel task messages.
   > Can you, please, clarify, what tests should be added to check the changes?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393772907
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/processors/continuous/StopRoutineDiscoveryMessage.java
 ##########
 @@ -29,11 +29,24 @@
     /** */
     private static final long serialVersionUID = 0L;
 
+    /** {@code True} if stop should be forced. */
+    private boolean force;
 
 Review comment:
   fixed.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.

Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7520: IGNITE-12632: Support of cancel command for resource-consuming entities.
URL: https://github.com/apache/ignite/pull/7520#discussion_r393629199
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/visor/query/VisorContinuousQueryCancelTask.java
 ##########
 @@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.visor.query;
+
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.internal.IgniteInternalFuture;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.processors.task.GridVisorManagementTask;
+import org.apache.ignite.internal.visor.VisorJob;
+import org.apache.ignite.internal.visor.VisorOneNodeTask;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Task to cancel continuous query.
+ */
+@GridInternal
+@GridVisorManagementTask
+public class VisorContinuousQueryCancelTask extends VisorOneNodeTask<VisorContinuousQueryCancelTaskArg, Boolean> {
+    /** */
+    private static final long serialVersionUID = 0L;
+
+    /** {@inheritDoc} */
+    @Override protected VisorContinuousQueryCancelJob job(VisorContinuousQueryCancelTaskArg arg) {
+        return new VisorContinuousQueryCancelJob(arg, debug);
+    }
+
+    /**
+     * Job to cancel scan queries on node.
+     */
+    private static class VisorContinuousQueryCancelJob extends VisorJob<VisorContinuousQueryCancelTaskArg, Boolean> {
+        /** */
+        private static final long serialVersionUID = 0L;
+
+        /**
+         * Create job with specified argument.
+         *
+         * @param arg Job argument.
+         * @param debug Flag indicating whether debug information should be printed into node log.
+         */
+        protected VisorContinuousQueryCancelJob(@Nullable VisorContinuousQueryCancelTaskArg arg, boolean debug) {
+            super(arg, debug);
+        }
+
+        /** {@inheritDoc} */
+        @Override protected Boolean run(@Nullable VisorContinuousQueryCancelTaskArg arg) throws IgniteException {
+            IgniteLogger log = ignite.log().getLogger(VisorContinuousQueryCancelJob.class);
+
+            log.info("Cancelling continuous query[routineId=" + arg.getRoutineId() + ']');
 
 Review comment:
   `if (log.isInfoEnabled())`?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services