[KAFKA-17114] DefaultStateUpdater::handleRuntimeException should update isRunning before calling `addToExceptionsAndFailedTasksThenClearUpdatingAndPausedTasks`

• Type: Test
• Status: Resolved
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 3.9.0
• Fix Version/s: 4.1.0
• Component/s: streams, unit tests
• Assignee: Unassigned
• Reporter: Ao Li
• Created: 10/Jul/24 19:43
• Updated: 5 hours ago
• Resolved: 6 hours ago

Description

I saw a flaky test in DefaultStateUpdaterTest::shouldThrowIfAddingStandbyAndActiveTaskWithSameId recently.

org.opentest4j.AssertionFailedError: expected: <false> but was: <true>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertFalse.failNotFalse(AssertFalse.java:63)
	at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:36)
	at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:31)
	at org.junit.jupiter.api.Assertions.assertFalse(Assertions.java:231)
	at org.apache.kafka.streams.processor.internals.DefaultStateUpdaterTest.shouldThrowIfAddingTasksWithSameId(DefaultStateUpdaterTest.java:294)
	at org.apache.kafka.streams.processor.internals.DefaultStateUpdaterTest.shouldThrowIfAddingStandbyAndActiveTaskWithSameId(DefaultStateUpdaterTest.java:285)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

To make the bug more reproducible, you may add `Thread.sleep(5)` after `addToExceptionsAndFailedTasksThenClearUpdatingAndPausedTasks(runtimeException);` in DefaultStateUpdater::handleRuntimeException

The test is flaky because `addToExceptionsAndFailedTasksThenClearUpdatingAndPausedTasks(runtimeException);` will unblock the `verifyFailedTasks(IllegalStateException.class, task1);` statement in DefaultStateUpdaterTest::shouldThrowIfAddingTasksWithSameId.

If `assertFalse(stateUpdater.isRunning());` is executed before `isRunning.set(false);` the test will fail