KAFKA-8878: Fix flaky test AssignedStreamsTasksTest#shouldCloseCleanl…

[cpettitt-confluent]

…yWithSuspendedTaskAndEOS

The previous approach to testing KAFKA-8412 was to look at the logs and
determine if an error occurred during close. There was no direct way to
detect than an exception occurred because the exception was eaten in
AssignedTasks.close. In the PR for that ticket (#7207) it was
acknowledged that this was a brittle way to test for the exception. We
now see occasional failures because an unrelated ERROR level log entry
is made while closing the task.

This change eliminates the brittle log checking by rethrowing any time
an exception occurs in close, even when a subsequent unclean close
succeeds. This has the potential benefit of uncovering other supressed
exceptions down the road.

I've verified that even with us rethrowing on closeUnclean that all
tests pass.

More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[cpettitt-confluent]

We may want to also backport this to 2.3, 2.2, and 2.1, since they all got the original fix in #7207 and may suffer from flaky test results as well.

[cpettitt-confluent]

It's worth noting that raising the exception from AssignedTasks.close as in this patch does not impact close for non assigned suspended tasks (e.g. in AssignedTasks.closeNonAssignedSuspendedTasks).

[mjsax]

LGTM.

[cpettitt-confluent]

The one test failure on JDK 8 / Scala 2.11 is:

org.apache.kafka.connect.integration.ConnectWorkerIntegrationTest.testAddAndRemoveWorker

It's a flaky test : https://issues.apache.org/jira/browse/KAFKA-8690.

[cpettitt-confluent]

@mjsax do I need to do anything else on my side to get this merged?

[bbejeck]

@cpettitt-confluent thanks for the patch. LGTM.

[bbejeck]

Java 8/2.11 failed test results already cleared out.

retest this please.

[bbejeck]

waiting for green build to merge

[cpettitt-confluent]

Thanks for the review @bbejeck. If it helps at all, I captured the test failure in a comment above. It was due to this flaky test: https://issues.apache.org/jira/browse/KAFKA-8690. Given the high false positive rate, I've been copying errors into comments. Not sure if there is a better way to be managing these :).

[bbejeck]

on one PR build both Java 11 builds passed and Java 8 failed #7302 (comment)

this PR build Java 8 passed so all builds passed at one point, merging this.

[bbejeck]

Merged #7302 into trunk.

[cpettitt-confluent]

Thanks Bill!