xtern commented on code in PR #4898: URL: https://github.com/apache/ignite-3/pull/4898#discussion_r1888039254
########## modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/kill/ItSqlKillCommandTest.java: ########## @@ -95,46 +126,141 @@ public void killQueryFromLocal() { assertThat(queries.size(), is(1)); UUID targetQueryId = queries.get(0).id(); - checkKillQuery(node, targetQueryId, true); + assertThat(executeKillSqlQuery(node, targetQueryId), is(true)); assertThat(runningQueries(), is(empty())); expectQueryCancelled(new DrainCursor(cursor)); - checkKillQuery(node, targetQueryId, false); - checkKillQuery(node, targetQueryId, true, true); + assertThat(executeKillSqlQuery(node, targetQueryId), is(false)); + assertThat(executeKill(node, QUERY, targetQueryId, true), is(true)); + } + + @Test + public void killComputeJobFromLocal() { + Ignite node = CLUSTER.aliveNode(); + JobDescriptor<Void, Void> job = JobDescriptor.builder(InfiniteJob.class).units(List.of()).build(); + JobExecution<Void> execution = node.compute().submit(JobTarget.node(clusterNode(node)), job, null); + + Awaitility.await().until(execution::stateAsync, willBe(jobStateWithStatus(EXECUTING))); + + UUID jobId = await(execution.idAsync()); + assertThat(jobId, not(nullValue())); + assertThat(executeKillJob(node, jobId), is(true)); + + Awaitility.await().until(execution::stateAsync, willBe(jobStateWithStatus(CANCELED))); + + assertThat(executeKillJob(node, jobId), is(false)); + assertThat(executeKill(node, COMPUTE, jobId, true), is(true)); } @Test public void killQueryFromRemote() { Ignite local = CLUSTER.node(0); Ignite remote = CLUSTER.node(2); - AsyncSqlCursor<InternalSqlRow> cursor = executeQueryInternal(local, "SELECT 1"); + { + AsyncSqlCursor<InternalSqlRow> cursor = executeQueryInternal(local, "SELECT 1"); - List<QueryInfo> queries = runningQueries(); - assertThat(queries.size(), is(1)); - UUID targetQueryId = queries.get(0).id(); + List<QueryInfo> queries = runningQueries(); + assertThat(queries.size(), is(1)); + UUID targetQueryId = queries.get(0).id(); - checkKillQuery(remote, targetQueryId, true); + assertThat(executeKillSqlQuery(remote, targetQueryId), is(true)); - assertThat(runningQueries(), is(empty())); - expectQueryCancelled(new DrainCursor(cursor)); + assertThat(runningQueries(), is(empty())); + expectQueryCancelled(new DrainCursor(cursor)); + + assertThat(executeKillSqlQuery(remote, targetQueryId), is(false)); + assertThat(executeKillSqlQuery(local, targetQueryId), is(false)); + } + + // No wait. + { + AsyncSqlCursor<InternalSqlRow> cursor = executeQueryInternal(local, "SELECT 1"); - checkKillQuery(remote, targetQueryId, false); - checkKillQuery(local, targetQueryId, false); + List<QueryInfo> queries = runningQueries(); + assertThat(queries.size(), is(1)); + UUID targetQueryId = queries.get(0).id(); + + assertThat(executeKill(remote, QUERY, targetQueryId, true), is(true)); + + Awaitility.await().untilAsserted(() -> assertThat(runningQueries(), is(empty()))); + expectQueryCancelled(new DrainCursor(cursor)); + } } - private static void checkKillQuery(Ignite node, UUID queryId, boolean expectedResult) { - checkKillQuery(node, queryId, expectedResult, false); + @Test + public void killComputeJobFromRemote() { + Ignite local = CLUSTER.node(0); + Ignite remote = CLUSTER.node(2); + + // Single execution. + { + JobDescriptor<Void, Void> job = JobDescriptor.builder(InfiniteJob.class).units(List.of()).build(); + JobExecution<Void> execution = local.compute().submit(JobTarget.node(clusterNode(local)), job, null); + + Awaitility.await().until(execution::stateAsync, willBe(jobStateWithStatus(EXECUTING))); + + UUID jobId = await(execution.idAsync()); + assertThat(jobId, not(nullValue())); + assertThat(executeKillJob(remote, jobId), is(true)); + + Awaitility.await().until(execution::stateAsync, willBe(jobStateWithStatus(CANCELED))); + + assertThat(executeKillJob(remote, jobId), is(false)); + assertThat(executeKillJob(local, jobId), is(false)); + } + + // Single execution with nowait. + { + JobDescriptor<Void, Void> job = JobDescriptor.builder(InfiniteJob.class).units(List.of()).build(); + JobExecution<Void> execution = local.compute().submit(JobTarget.node(clusterNode(local)), job, null); + + Awaitility.await().until(execution::stateAsync, willBe(jobStateWithStatus(EXECUTING))); + + UUID jobId = await(execution.idAsync()); + assertThat(jobId, not(nullValue())); + assertThat(executeKill(remote, COMPUTE, jobId, true), is(true)); + + Awaitility.await().until(execution::stateAsync, willBe(jobStateWithStatus(CANCELED))); + + assertThat(executeKill(remote, COMPUTE, jobId, true), is(true)); + } + + // Multiple executions. + { + JobDescriptor<Void, Void> job = JobDescriptor.builder(InfiniteJob.class).units(List.of()).build(); + Map<ClusterNode, JobExecution<Void>> executions = local.compute().submitBroadcast( + Set.of(clusterNode(CLUSTER.node(0)), clusterNode(CLUSTER.node(1))), job, null); + + executions.forEach((node, execution) -> { + UUID jobId = await(execution.idAsync()); + assertThat(jobId, not(nullValue())); + assertThat("Node=" + node.name(), executeKillJob(remote, jobId), is(true)); + + Awaitility.await().until(execution::stateAsync, willBe(jobStateWithStatus(CANCELED))); + + assertThat("Node=" + node.name(), executeKillJob(remote, jobId), is(false)); + assertThat("Node=" + node.name(), executeKillJob(local, jobId), is(false)); + }); + } } - private static void checkKillQuery(Ignite node, UUID queryId, boolean expectedResult, boolean noWait) { - String query = IgniteStringFormatter - .format("KILL QUERY '{}'{}", queryId, noWait ? " NO WAIT" : ""); + private static boolean executeKillSqlQuery(Ignite node, UUID queryId) { + return executeKill(node, QUERY, queryId, false); + } + + private static boolean executeKillJob(Ignite node, UUID jonId) { + return executeKill(node, COMPUTE, jonId, false); Review Comment: Fixed, thanks ########## modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/kill/KillCommandHandler.java: ########## @@ -102,7 +102,7 @@ public CompletableFuture<Boolean> handle(KillCommand cmd) { CompletableFuture<Boolean> killFut = invokeCancel(handler, cmd.operationId()); - if (killFut.isDone() || !cmd.noWait()) { + if (killFut.isCompletedExceptionally() || !cmd.noWait()) { Review Comment: Yes > if noWait == true - we must return immediatelly i.e. return CompletableFuture.completedFuture(true); ...except handling exception described in javadoc `@throws IllegalArgumentException If the operation identifier is not in the correct format.` For example ``KILL QUERY '123' NO WAIT` throws exception instead of returning true :thinking: -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org