JacksonYao287 commented on a change in pull request #2578:
URL: https://github.com/apache/ozone/pull/2578#discussion_r700842452
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java
##########
@@ -722,8 +722,10 @@ public void stop() {
return;
}
balancerRunning = false;
- currentBalancingThread.interrupt();
- currentBalancingThread.join(1000);
+ if (Thread.currentThread().getId() != currentBalancingThread.getId()) {
+ currentBalancingThread.interrupt();
+ currentBalancingThread.join(100);
Review comment:
seems we should use `currentBalancingThread.join(0)` here to wait for
ever until the thread exit. also we should `thread.isInterrupted()` to check
interrupt status
[here](https://github.com/apache/ozone/blob/7cb235ea002aad235de4d36bd86807079b298cb6/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java#L197)
let us think about a scenario. a stop command is received for command line,
and thus `currentBalancingThread.interrupt()` is called according to current
logic. suppose we have too many datanodes and containers in the ozone cluster ,
then `doIteration()` will take much time. if currrentBalancingThread is running
in `doIteration()` now and does not block at `wait()`, then
`currentBalancingThread.interrupt()` will just set the thread's interrupt
status and return. then `currentBalancingThread.join(100)` is called, but
currentBalancingThread may still running in `doIteration()`. then
`currentBalancingThread = null` is called , now the thread becomes an orphan
running thread and may still running in the background. this thread may modify
some variables , such as `sourceToTargetMap` , `selectedContainers`, so if now
we start a new balance thread for command line, there may be some problmes.
so in my opinion, if stop is called from command line , we must make sure
the current balance thread exit.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
