Skip to content

IGNITE-28049 Fix DurableBackgroundTasksProcessorSelfTest#testConvertAfterRestoreIfNeeded#12848

Merged
zstan merged 6 commits intoapache:masterfrom
chesnokoff:ignite-28049
Mar 6, 2026
Merged

IGNITE-28049 Fix DurableBackgroundTasksProcessorSelfTest#testConvertAfterRestoreIfNeeded#12848
zstan merged 6 commits intoapache:masterfrom
chesnokoff:ignite-28049

Conversation

@chesnokoff
Copy link
Contributor

Thank you for submitting the pull request to the Apache Ignite.

In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:

The Contribution Checklist

  • There is a single JIRA ticket related to the pull request.
  • The web-link to the pull request is attached to the JIRA ticket.
  • The JIRA ticket has the Patch Available state.
  • The pull request body describes changes that have been made.
    The description explains WHAT and WHY was made instead of HOW.
  • The pull request title is treated as the final commit message.
    The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
  • A reviewer has been mentioned through the JIRA comments
    (see the Maintainers list)
  • The pull request has been checked by the Teamcity Bot and
    the green visa attached to the JIRA ticket (see tab PR Check at TC.Bot - Instance 1 or TC.Bot - Instance 2)

Notes

If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.

@chesnokoff chesnokoff changed the title IGNITE-28049 Fix testConvertAfterRestoreIfNeeded IGNITE-28049 Fix DurableBackgroundTasksProcessorSelfTest#testConvertAfterRestoreIfNeeded Mar 3, 2026

n.cluster().state(ACTIVE);

checkStateAndMetaStorage(n, t2, STARTED, true, false, true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes the line may fail because it does not have enough time to transform state: INIT -> PREPARED -> STARTED
Because of that
assertEquals(expState, taskState.state())
May fail with java.lang.AssertionError: expected:<STARTED> but was:<INIT> or java.lang.AssertionError: expected:<STARTED> but was:<PREPARE>

assertNull(taskState);
else {
assertEquals(expState, taskState.state());
assertTrue(waitForCondition(() -> expState.equals(taskState.state()), getTestTimeout()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

State is enum, == comparison can be applied.
getTestTimeout() looks unreasonable long. Maybe some constant, like 1-5 secs?
Test still locally fails with:

java.lang.AssertionError: 
Expected :3
Actual   :2

	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.failNotEquals(Assert.java:834)
	at org.junit.Assert.assertEquals(Assert.java:645)
	at org.junit.Assert.assertEquals(Assert.java:631)
	at org.apache.ignite.testframework.junits.JUnitAssertAware.assertEquals(JUnitAssertAware.java:95)
	at org.apache.ignite.internal.processors.localtask.DurableBackgroundTasksProcessorSelfTest.testConvertAfterRestoreIfNeeded(DurableBackgroundTasksProcessorSelfTest.java:349)

n = startGrid(0);

assertEquals(3, tasks(n).size());
int tasks = tasks(n).size();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alex-plekhanov Thanks for notion about new fail in this test, it was not reproduced by my local machine and on tc. It can be reproduced if we put doSleep(5_000) before the assertion. The reason is that checkpoint deletes completed task t0 (see DurableBackgroundTasksProcessor#onMarkCheckpointBegin where we make it.remove()). So there is race condition between the assertion and checkpoint

I suggest updating statement to assert and providing clear message

assertNull(taskState);
else {
assertEquals(expState, taskState.state());
assertTrue(waitForCondition(() -> expState == taskState.state(), 5));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 sec may not be enough, decreased to 5

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parameter is ms, so you need 5_000L for 5 seconds.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 6, 2026

@zstan zstan merged commit 10800f7 into apache:master Mar 6, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants