[ 
https://issues.apache.org/jira/browse/IGNITE-23939?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Pavel Pereslegin updated IGNITE-23939:
--------------------------------------
    Description: 
The test {{ItCancelScriptTest.cancelMustRollbackScriptTransaction}} sometime 
reproduces this issue 
https://ci.ignite.apache.org/project.html?projectId=ApacheIgnite3xGradle_Test_IntegrationTests&buildTypeId=&tab=testDetails&testNameId=7736497393505676669&order=TEST_STATUS_DESC&branch_ApacheIgnite3xGradle_Test_IntegrationTests=__all_branches__&itemsCount=50
 (see TODO related to this issue).

Issue can be reproduced by adding an extra sleep() just before deleting the 
query from the registry.
{code:java}
        onPhaseStarted(ExecutionPhase.TERMINATED).whenComplete((ignored, ex) -> 
{
            Thread.sleep(500); // extra sleep here

            runningQueries.remove(id);

            terminationDoneFuture.complete(null);
        });
{code}

And run the following test (add it to ItCancelScriptTest)
{code:java}
@Test
void cancelScriptX() {
    CancelHandle cancelHandle = CancelHandle.create();
    CancellationToken token = cancelHandle.token();

    AsyncSqlCursor<InternalSqlRow> cur = runScript(token,
            "SELECT 1; SELECT 2; SELECT 3;"
    );

    Awaitility.await().untilAsserted(() -> 
assertThat(queryProcessor().runningQueriesCount(), is(3 + /* SCRIPT */ 1)));

    cancelHandle.cancel();

    assertThat(queryProcessor().runningQueriesCount(), is(0));
}
{code}

we expecting that script cancellation will wait until all child queries will be 
removed from the registry, but this test fails

{noformat}
java.lang.AssertionError: 
Expected: is <0>
     but: was <1>
Expected :is <0>
Actual   :<1>
{noformat}

NOTE: the last query in the registry of running queries is usually "SELECT 1".

It is necessary to investigate the problem and fix it so that the parent query 
is removed from the registry of running queries only after all child queries 
are removed.


h3. UPDATE

We decided that the current script behavior is ok.

As part of the ticket, we need to:
1. simplify the code - we should not wait for the query to be removed from the 
registry - we only wait for the transition to the TERMINATED phase (see 
Query#terminationDoneFuture - this future must be removed)
2. rework tests that check that {{runningQueries}} counter is zero (add busy 
wait),
3. remove TODO in {{cancelMustRollbackScriptTransaction}} code.

  was:
The test {{ItCancelScriptTest.cancelMustRollbackScriptTransaction}} sometime 
reproduces this issue 
https://ci.ignite.apache.org/project.html?projectId=ApacheIgnite3xGradle_Test_IntegrationTests&buildTypeId=&tab=testDetails&testNameId=7736497393505676669&order=TEST_STATUS_DESC&branch_ApacheIgnite3xGradle_Test_IntegrationTests=__all_branches__&itemsCount=50
 (see TODO related to this issue).

Issue can be reproduced by adding an extra sleep() just before deleting the 
query from the registry.
{code:java}
        onPhaseStarted(ExecutionPhase.TERMINATED).whenComplete((ignored, ex) -> 
{
            Thread.sleep(500); // extra sleep here

            runningQueries.remove(id);

            terminationDoneFuture.complete(null);
        });
{code}

And run the following test (add it to ItCancelScriptTest)
{code:java}
@Test
void cancelScriptX() {
    CancelHandle cancelHandle = CancelHandle.create();
    CancellationToken token = cancelHandle.token();

    AsyncSqlCursor<InternalSqlRow> cur = runScript(token,
            "SELECT 1; SELECT 2; SELECT 3;"
    );

    Awaitility.await().untilAsserted(() -> 
assertThat(queryProcessor().runningQueriesCount(), is(3 + /* SCRIPT */ 1)));

    cancelHandle.cancel();

    assertThat(queryProcessor().runningQueriesCount(), is(0));
}
{code}

we expecting that script cancellation will wait until all child queries will be 
removed from the registry, but this test fails

{noformat}
java.lang.AssertionError: 
Expected: is <0>
     but: was <1>
Expected :is <0>
Actual   :<1>
{noformat}

NOTE: the last query in the registry of running queries is usually "SELECT 1".

It is necessary to investigate the problem and fix it so that the parent query 
is removed from the registry of running queries only after all child queries 
are removed.


h3. UPDATE

We decided that the current script behavior is ok.

As part of the ticket, we need to:
1. simplify the code - we should not wait for the query to be removed from the 
registry - we only wait for the transition to the TERMINATED phase (see 
Query#terminationDoneFuture - this future must be removed)
2. rework tests that check that {{runningQueries}} counter is zero (add busy 
wait),
3. remove TODO in code.


> Sql. Script cancellation doesn't wait until all child queries are removed 
> from the registry
> -------------------------------------------------------------------------------------------
>
>                 Key: IGNITE-23939
>                 URL: https://issues.apache.org/jira/browse/IGNITE-23939
>             Project: Ignite
>          Issue Type: Bug
>          Components: sql
>            Reporter: Pavel Pereslegin
>            Priority: Major
>              Labels: ignite-3
>
> The test {{ItCancelScriptTest.cancelMustRollbackScriptTransaction}} sometime 
> reproduces this issue 
> https://ci.ignite.apache.org/project.html?projectId=ApacheIgnite3xGradle_Test_IntegrationTests&buildTypeId=&tab=testDetails&testNameId=7736497393505676669&order=TEST_STATUS_DESC&branch_ApacheIgnite3xGradle_Test_IntegrationTests=__all_branches__&itemsCount=50
>  (see TODO related to this issue).
> Issue can be reproduced by adding an extra sleep() just before deleting the 
> query from the registry.
> {code:java}
>         onPhaseStarted(ExecutionPhase.TERMINATED).whenComplete((ignored, ex) 
> -> {
>             Thread.sleep(500); // extra sleep here
>             runningQueries.remove(id);
>             terminationDoneFuture.complete(null);
>         });
> {code}
> And run the following test (add it to ItCancelScriptTest)
> {code:java}
> @Test
> void cancelScriptX() {
>     CancelHandle cancelHandle = CancelHandle.create();
>     CancellationToken token = cancelHandle.token();
>     AsyncSqlCursor<InternalSqlRow> cur = runScript(token,
>             "SELECT 1; SELECT 2; SELECT 3;"
>     );
>     Awaitility.await().untilAsserted(() -> 
> assertThat(queryProcessor().runningQueriesCount(), is(3 + /* SCRIPT */ 1)));
>     cancelHandle.cancel();
>     assertThat(queryProcessor().runningQueriesCount(), is(0));
> }
> {code}
> we expecting that script cancellation will wait until all child queries will 
> be removed from the registry, but this test fails
> {noformat}
> java.lang.AssertionError: 
> Expected: is <0>
>      but: was <1>
> Expected :is <0>
> Actual   :<1>
> {noformat}
> NOTE: the last query in the registry of running queries is usually "SELECT 1".
> It is necessary to investigate the problem and fix it so that the parent 
> query is removed from the registry of running queries only after all child 
> queries are removed.
> h3. UPDATE
> We decided that the current script behavior is ok.
> As part of the ticket, we need to:
> 1. simplify the code - we should not wait for the query to be removed from 
> the registry - we only wait for the transition to the TERMINATED phase (see 
> Query#terminationDoneFuture - this future must be removed)
> 2. rework tests that check that {{runningQueries}} counter is zero (add busy 
> wait),
> 3. remove TODO in {{cancelMustRollbackScriptTransaction}} code.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to