kaisun2000 commented on a change in pull request #1227:
URL: https://github.com/apache/helix/pull/1227#discussion_r494728983
##########
File path:
helix-core/src/main/java/org/apache/helix/messaging/handling/HelixTaskExecutor.java
##########
@@ -623,6 +623,13 @@ void reset() {
}
}
+ // some thing like STATE_TRANSITION.Key specific pool are not shut down.
+ for (String msgType : _executorMap.keySet()) {
+ ExecutorService pool = _executorMap.remove(msgType);
Review comment:
removed. Instead noted the potential improvement for now.
##########
File path: helix-core/src/main/java/org/apache/helix/task/TaskDriver.java
##########
@@ -969,17 +969,29 @@ public TaskState pollForJobState(String workflowName,
String jobName, long timeo
Set<TaskState> allowedStates = new HashSet<>(Arrays.asList(states));
// Wait for state
long st = System.currentTimeMillis();
+ long ct = 0;
do {
Thread.sleep(timeToSleep);
ctx = getWorkflowContext(workflowName);
+ ct = System.currentTimeMillis();
} while ((ctx == null || ctx.getJobState(jobName) == null
|| !allowedStates.contains(ctx.getJobState(jobName)))
- && System.currentTimeMillis() < st + timeout);
+ && ct < st + timeout);
if (ctx == null || !allowedStates.contains(ctx.getJobState(jobName))) {
Review comment:
revert back.
##########
File path:
helix-core/src/main/java/org/apache/helix/task/TaskStateModelFactory.java
##########
@@ -97,7 +97,7 @@ public void shutdown() {
}
@VisibleForTesting
- void shutdownNow() {
+ public void shutdownNow() {
Review comment:
helix-core/src/test/java/org/apache/helix/integration/multizk/TestMultiZkHelixJavaApis.java
actually leaks hundreds of threads.
We need to call this method to avoid the thread leak
```
public void afterClass() throws Exception {
String testClassName = getClass().getSimpleName();
System.out.println("AfterClass: " + testClassName + " of
TestMultiZkHelixJavaApis called.");
try {
// Kill all mock controllers and participants
MOCK_CONTROLLERS.values().forEach(ClusterControllerManager::syncStop);
MOCK_PARTICIPANTS.forEach(mockParticipantManager -> {
mockParticipantManager.syncStop();
StateMachineEngine stateMachine =
mockParticipantManager.getStateMachineEngine();
if (stateMachine != null) {
StateModelFactory stateModelFactory =
stateMachine.getStateModelFactory("Task");
if (stateModelFactory != null && stateModelFactory instanceof
TaskStateModelFactory) {
((TaskStateModelFactory) stateModelFactory).shutdownNow();
}
}
});
```
##########
File path:
helix-core/src/main/java/org/apache/helix/tools/ClusterStateVerifier.java
##########
@@ -181,6 +181,12 @@ private BestPossAndExtViewZkVerifier(HelixZkClient
zkClient, String clusterName,
this.resources = resources;
}
+ public void close() {
Review comment:
There are still many places using this one. Change to use new verifier
seems to worth a separate diff.
##########
File path: helix-core/src/test/java/org/apache/helix/TestHelper.java
##########
@@ -794,7 +797,12 @@ public static boolean verify(Verifier verifier, long
timeout) throws Exception {
long start = System.currentTimeMillis();
do {
boolean result = verifier.verify();
- if (result || (System.currentTimeMillis() - start) > timeout) {
+ long now = System.currentTimeMillis();
+ if (result || (now - start) > timeout) {
Review comment:
changed.
##########
File path: helix-core/src/test/java/org/apache/helix/TestListenerCallback.java
##########
@@ -119,6 +119,8 @@ public void beforeClass() throws Exception {
@AfterClass
public void afterClass() throws Exception {
+ String testClassName = this.getShortClassName();
+ System.out.println("AfterClass: " + testClassName + " of
TestListenerCallback called.");
Review comment:
It does not really make test output much longer. The reason is that
sometime, the error message is emitted from afterClass(), sometimes, from last
testing method. It is hard to distinguish.
With this log, it would be clear for people to examine
----------------------------------------------------------------
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]
