chungen0126 commented on code in PR #8488:
URL: https://github.com/apache/ozone/pull/8488#discussion_r2101343579
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java:
##########
@@ -133,6 +135,10 @@ public class DatanodeStateMachine implements Closeable {
private final DatanodeQueueMetrics queueMetrics;
private final ReconfigurationHandler reconfigurationHandler;
+ //global variable to track if diskBalancer was running before pause
+ private Boolean shouldRun = false;
Review Comment:
I think we don't need shouldRun, since paused can represent whether
DiskBalancer is suspended (i.e. It's running when the DN enters DECOMMISSIONING
or MAINTENANCE). So just checking paused should be enough.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java:
##########
@@ -704,6 +711,44 @@ private Thread getCommandHandlerThread(Runnable
processCommandQueue) {
return handlerThread;
}
+ /**
+ * Stops the DiskBalancerService if it is running.
+ */
+ public void stopDiskBalancer() {
+ OzoneContainer ozoneContainer = getContainer();
+ if (ozoneContainer != null) {
+ DiskBalancerService diskBalancerService =
ozoneContainer.getDiskBalancerService();
+ if (diskBalancerService != null) {
+ shouldRun = diskBalancerService.getDiskBalancerInfo().isShouldRun();
+ paused.set(true); // Mark as paused
+ diskBalancerService.setShouldRun(false);
+ }
+ }
+ }
+
+ /**
+ * Resume the DiskBalancerService if it was running previously.
+ */
+ public void resumeDiskBalancer() {
+ OzoneContainer ozoneContainer = getContainer();
+ if (ozoneContainer != null) {
+ DiskBalancerService diskBalancerService =
ozoneContainer.getDiskBalancerService();
+ if (diskBalancerService != null && paused.get()) {
Review Comment:
Maybe replace get() + set(false) with compareAndSet(true, false). It’s
cleaner and atomic.
For example:
```
if (diskBalancerService != null && paused.compareAndSet(true, false)) {
diskBalancerService.setShouldRun(true);
}
```
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java:
##########
@@ -704,6 +711,44 @@ private Thread getCommandHandlerThread(Runnable
processCommandQueue) {
return handlerThread;
}
+ /**
+ * Stops the DiskBalancerService if it is running.
+ */
+ public void stopDiskBalancer() {
+ OzoneContainer ozoneContainer = getContainer();
+ if (ozoneContainer != null) {
+ DiskBalancerService diskBalancerService =
ozoneContainer.getDiskBalancerService();
+ if (diskBalancerService != null) {
+ shouldRun = diskBalancerService.getDiskBalancerInfo().isShouldRun();
+ paused.set(true); // Mark as paused
+ diskBalancerService.setShouldRun(false);
+ }
+ }
+ }
+
+ /**
+ * Resume the DiskBalancerService if it was running previously.
+ */
+ public void resumeDiskBalancer() {
+ OzoneContainer ozoneContainer = getContainer();
+ if (ozoneContainer != null) {
+ DiskBalancerService diskBalancerService =
ozoneContainer.getDiskBalancerService();
+ if (diskBalancerService != null && paused.get()) {
+ // Reset paused state
+ paused.set(false);
+ diskBalancerService.setShouldRun(true);
+ }
+ }
+ }
+
+ /**
+ * Checks if DiskBalancerService should run based
+ * on its previous state and paused flag.
+ */
+ public boolean shouldRunDiskBalancer() {
+ return shouldRun && paused.get();
Review Comment:
Based on the above, shouldRun can be removed here. Just use paused.get()
directly.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java:
##########
@@ -704,6 +711,44 @@ private Thread getCommandHandlerThread(Runnable
processCommandQueue) {
return handlerThread;
}
+ /**
+ * Stops the DiskBalancerService if it is running.
+ */
+ public void stopDiskBalancer() {
+ OzoneContainer ozoneContainer = getContainer();
+ if (ozoneContainer != null) {
+ DiskBalancerService diskBalancerService =
ozoneContainer.getDiskBalancerService();
+ if (diskBalancerService != null) {
+ shouldRun = diskBalancerService.getDiskBalancerInfo().isShouldRun();
+ paused.set(true); // Mark as paused
Review Comment:
Based on above, we could combine `shouldRun` and `paused` here by just
setting `paused` to `diskBalancerService.getDiskBalancerInfo().isShouldRun()`.
It makes the code simpler and still keeps track of whether DiskBalancer was
running before pausing.
--
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]
