lokeshj1703 commented on code in PR #3535:
URL: https://github.com/apache/ozone/pull/3535#discussion_r908222198
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java:
##########
@@ -1032,16 +1048,30 @@ private void validateState(boolean expectedRunning)
*/
@Override
public void stop() {
+ Thread balancingThread;
lock.lock();
try {
if (!isBalancerRunning()) {
LOG.warn("Cannot stop Container Balancer because it's not running");
return;
}
- stopBalancingThread();
+ balancingThread = currentBalancingThread;
+ currentBalancingThread = null;
} finally {
lock.unlock();
}
+
+ // wait for balancingThread to die
+ if (balancingThread != null &&
Review Comment:
Can we add a preconditions check here that lock is not held by the current
thread? There is an api for it.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java:
##########
@@ -839,20 +850,34 @@ private void resetState() {
@Override
public void notifyStatusChanged() {
if (!scmContext.isLeader() || scmContext.isInSafeMode()) {
- if (isBalancerRunning()) {
+ boolean shouldStop = false;
+
+ // lock here to ensure no change is made to the balancing thread while
+ // we're reading it
+ lock.lock();
+ try {
+ shouldStop = isBalancerRunning();
Review Comment:
I would suggest merging the two lock sections and afterwards stop can be
called.
--
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]
