kaisun2000 commented on a change in pull request #1227:
URL: https://github.com/apache/helix/pull/1227#discussion_r467242429
##########
File path: helix-core/src/test/java/org/apache/helix/common/ZkTestBase.java
##########
@@ -719,6 +720,12 @@ public void cleanupLiveInstanceOwners() {
clientMap.clear();
}
_liveInstanceOwners.clear();
+
+ boolean status = TestHelper.afterClassCheck(name);
+ // Assert here does not work.
+ if (!status) {
+ System.out.println("---------- Test Class " + name + " thread leakage
detected! ---------------");
Review comment:
This is something I spent a lot of time yesterday Can't make it work
this way. See comment 725 "Assert does not work".
On the hand, after testing /running this diff many time. I found this may
not be a good idea to fail it outright. The reasons are:
- closing Zk Session, Cancel() timer thread or executors by our code and
till the thread there are shutdown are async. There are some time delta before
these threads tearing down from cancel() time. And it is out of our control.
For example, closing Zk session via Zookeeper client library. But the
sendThread and eventThread associated with this session is not teared down
before Zookeeper return from close() and hand the execution to our code.
- Some threads created from stream() etc java functional framework, such as
forkjoinwork, or pool- can happen. We don't have control of their life time.
Thus, sometimes some of these to-be-closed threads would appear here. So
relying on human examination here may not be a bad idea for now. Let me know
what is your take?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]