KAFKA-20522: Refine test coverage for consumer close-on-interrupt best-effort behavior.#22138
KAFKA-20522: Refine test coverage for consumer close-on-interrupt best-effort behavior.#22138chickenchickenlove wants to merge 3 commits intoapache:trunkfrom
Conversation
…t-effort behavior.
github-actions
Bot
added
triage
| } | ||
|
|
||
| @Test | ||
| public void testClassicConsumerCloseAttemptsLeaveGroupWhenInterrupted() { |
There was a problem hiding this comment.
Could we test it for the AsyncConsumer too please? We can check that it add the LeaveGroupOnCloseEvent (even though it cannot wait for it because it's Interrupted)
--update
actually, I noticed we already have testCloseLeavesGroupDespiteInterrupt for the Async, nice
| Thread.interrupted(); | ||
| } | ||
|
|
||
| //THEN |
There was a problem hiding this comment.
not sure this kind of comments add much value?? (given, when, then) What do you think?
There was a problem hiding this comment.
You are right!
I've addressed it.
|
@lianetm |
| import java.util.concurrent.ExecutionException | ||
|
|
||
| @Timeout(60) | ||
| class PlaintextConsumerTest extends AbstractConsumerTest { |
There was a problem hiding this comment.
We can safely delete this test class directly, since it is fully covered by testAsyncConsumerCloseRunsRevocationCallbackOnInterrupt and testClassicConsumerCloseRunsRevocationCallbackOnInterrupt. The only reason it was kept before was to preserve the original code that was causing the flakiness
There was a problem hiding this comment.
@chia7712
Thanks for the time to take a look into this!
Yes, you are right. I removed PlaintextConsumerTest.scala class.
|
@chia7712 |
3 participants
Description
In the previous,
PlaintextConsumerTest.testClassicConsumerCloseLeavesGroupOnInterruptwere flaky.The reason is that
LeaveGroupon consumer close has best-effort semantics, but those tests were written under the assumption that the consumer deterministically leaves the consumer group. In practice, however, the consumer may fail to leave the group within the bounded timeout, for example due to metadata expiration. (#21332)This PR splits the flaky test into two separate tests.
callsToRevokedis invoked wheninterrupt()is called. This behavior is still deterministic even under best-effort semantics.ApiKeys.LEAVE_GROUPrequest is recorded inMockClient.requests()wheninterrupt()is called.Previously, even if an
ApiKeys.LEAVE_GROUPrequest was created, request processing could be delayed due to metadata expiration. As a result, the exact point at which the consumer leaves the group was non-deterministic. Therefore, taking the best-effort semantics into account, the unit test only verifies that theApiKeys.LEAVE_GROUPrequest is recorded.Related