gharris1727 commented on code in PR #12290:
URL: https://github.com/apache/kafka/pull/12290#discussion_r1425879028
##########
connect/runtime/src/test/java/org/apache/kafka/connect/integration/BlockingConnectorTest.java:
##########
@@ -139,7 +141,8 @@ public void setup() throws Exception {
public void close() {
// stop all Connect, Kafka and Zk threads.
connect.stop();
- Block.resetBlockLatch();
+ // unblock everything so that we don't leak threads after each test run
+ Block.reset();
Review Comment:
> It's easier to handle in a single method rather than copy over to 11 test
cases
Oh I see, I thought it would be sufficient to add one test case that called
stop() to verify that one type of blocked thread still allows shutdown to
complete, rather than verifying it for all of the different ways of blocking
threads. That would have less coverage that the current tests on trunk.
> I've pushed a tweak that adds logic to wait for the blocked threads to
complete in Block::reset.
I think this is probably the better solution. The leak tester can separately
verify that resources were closed properly now that the test ensures the
threads stop. :+1:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
