MINOR: Deflake streams admin api describe streams group with short timeout #20320
Conversation
github-actions
bot
added
triage
There was a problem hiding this comment.
Thanks for the PR @aliehsaeedii - overall LGTM - I'll kick off the build a few times to see if the flakiness is fixed, and either merge or discuss if we need to keep the test.
There was a problem hiding this comment.
What was the original purpose of the test? -- It's unclear to me, why creating a second KS instance help to stabilize the test. Can you elaborate?
If I read the test correctly, the idea is to get a timeout exception for a describe call -- what is the conditions when we would get a timeout exception? Group does not exist? Or something else?
tools/src/test/java/org/apache/kafka/tools/streams/DescribeStreamsGroupTest.java
Outdated
Show resolved
Hide resolved
| List<String> args = List.of("--bootstrap-server", bootstrapServers, "--describe", "--members", "--verbose", "--group", APP_ID, "--timeout", "1"); | ||
| Throwable e = assertThrows(ExecutionException.class, () -> getStreamsGroupService(args.toArray(new String[0])).describeGroups()); | ||
| assertEquals(TimeoutException.class, e.getCause().getClass()); | ||
|
|
||
| streams2.close(); | ||
| streams2.cleanUp(); |
There was a problem hiding this comment.
If we use try-with-resource, we can remove close() and put cleanUp() into finally-block.
There was a problem hiding this comment.
The finally block does not know the variable defined in the try clause
I assumed in real-world scenarios, 1ms is too little for describing a group, but the flakiness proved that 1ms could be enough sometimes. I thought the group is existing, and we had a couple of other |
With a
1mstimeout, the streams group description occasionallysucceeds. To check if this resolves the flakiness, I created the streams
group within the test itself. If this approach doesn't stabilize the
test, we should consider removing it altogether, since the original
intent behind the test may no longer be valid.
Reviewers: Bill Bejeck[email protected]