Refactor node status recovery from DiskFull

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/DataNodeInternalRPCServiceImpl.java

@@ -2623,13 +2623,7 @@ void sampleDiskLoad(TLoadSample loadSample) {
             commonConfig.getDiskSpaceWarningThreshold());
         commonConfig.setNodeStatusWithReason(NodeStatus.ReadOnly, NodeStatus.DISK_FULL);
       } else {
-        synchronized (commonConfig) {
-          // A concurrent Manual/Stopping/error transition must not be cleared by disk recovery.
-          if (NodeStatus.ReadOnly.equals(commonConfig.getNodeStatus())
-              && NodeStatus.DISK_FULL.equals(commonConfig.getStatusReason())) {
-            commonConfig.setNodeStatus(NodeStatus.Running);
-          }
-        }
+        commonConfig.recoverFromDiskFull();
       }
     }
   }

iotdb-core/datanode/src/test/java/org/apache/iotdb/db/protocol/thrift/impl/DataNodeInternalRPCServiceImplDiskTest.java

@@ -40,13 +40,16 @@ public class DataNodeInternalRPCServiceImplDiskTest {
 
   private final CommonConfig commonConfig = CommonDescriptor.getInstance().getConfig();
   private final IoTDBConfig dataNodeConfig = IoTDBDescriptor.getInstance().getConfig();
+  private boolean originalStopping;
   private NodeStatus originalStatus;
   private String originalStatusReason;
   private double originalDiskSpaceWarningThreshold;
   private int originalDataNodeId;
 
   @Before
   public void setUp() {
+    originalStopping = commonConfig.isStopping();
+    commonConfig.setStopping(false);
     originalStatus = commonConfig.getNodeStatus();
     originalStatusReason = commonConfig.getStatusReason();
     originalDiskSpaceWarningThreshold = commonConfig.getDiskSpaceWarningThreshold();
@@ -66,6 +69,7 @@ public void tearDown() {
     commonConfig.setNodeStatusWithReason(originalStatus, originalStatusReason);
     commonConfig.setDiskSpaceWarningThreshold(originalDiskSpaceWarningThreshold);
     dataNodeConfig.setDataNodeId(originalDataNodeId);
+    commonConfig.setStopping(originalStopping);
   }
 
   @Test
@@ -136,4 +140,19 @@ public void testRecoveryRequiresExactReadOnlyWithDiskFullReason() {
     Assert.assertEquals(NodeStatus.ReadOnly, commonConfig.getNodeStatus());
     Assert.assertEquals(NodeStatus.STOPPING, commonConfig.getStatusReason());
   }
+
+  @Test
+  public void testStoppingFlagPreventsDiskRecovery() {
+    SystemMetrics systemMetrics = mock(SystemMetrics.class);
+    when(systemMetrics.getSystemDiskAvailableSpace()).thenReturn(20L);
+    when(systemMetrics.getSystemDiskTotalSpace()).thenReturn(100L);
+    DataNodeInternalRPCServiceImpl service =
+        new DataNodeInternalRPCServiceImpl(mock(DataNodeContext.class), systemMetrics);
+    commonConfig.setStopping(true);
+
+    service.sampleDiskLoad(new TLoadSample());
+
+    Assert.assertTrue(commonConfig.isReadOnly());
+    Assert.assertEquals(NodeStatus.DISK_FULL, commonConfig.getStatusReason());
+  }
 }

iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/conf/CommonConfig.java

@@ -868,24 +868,27 @@ public synchronized void setNodeStatusWithReason(NodeStatus newStatus, String ne
     } else if (status == NodeStatus.ReadOnly) {
       newReason = NodeStatus.transitionReadOnlyReason(statusReason, newReason);
     }
-    if (status == newStatus) {
-      this.statusReason = newReason;
-      return;
-    }
+    updateNodeStatus(newStatus, newReason);
+  }
 
-    logger.info(ConfigMessages.SET_SYSTEM_MODE, status, newStatus);
+  /** Applies the chosen state; the caller holds this monitor and checks transition rules. */
+  private void updateNodeStatus(NodeStatus newStatus, String newReason) {
+    if (status != newStatus) {
+      logger.info(ConfigMessages.SET_SYSTEM_MODE, status, newStatus);
+      if (newStatus == NodeStatus.ReadOnly) {
+        logger.warn(ConfigMessages.STATUS_CHANGE_TO_READ_ONLY);
+      } else if (newStatus == NodeStatus.Removing) {
+        logger.info(ConfigMessages.STATUS_CHANGE_TO_REMOVING);
+      }
+    }
     this.status = newStatus;
     this.statusReason = newReason;
+  }
 
-    switch (newStatus) {
-      case ReadOnly:
-        logger.warn(ConfigMessages.STATUS_CHANGE_TO_READ_ONLY);
-        break;
-      case Removing:
-        logger.info(ConfigMessages.STATUS_CHANGE_TO_REMOVING);
-        break;
-      default:
-        break;
+  /** Clears DiskFull after disk space recovers, unless shutdown has started. */
+  public synchronized void recoverFromDiskFull() {
+    if (!isStopping && status == NodeStatus.ReadOnly && NodeStatus.DISK_FULL.equals(statusReason)) {
+      updateNodeStatus(NodeStatus.Running, null);
     }
   }
 
@@ -901,7 +904,7 @@ public boolean isStopping() {
     return isStopping;
   }
 
-  public void setStopping(boolean stopping) {
+  public synchronized void setStopping(boolean stopping) {
     isStopping = stopping;
   }
 

iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/conf/CommonConfigTest.java

@@ -61,6 +61,23 @@ public void testSameNodeStatusDoesNotClearStatusReason() {
     Assert.assertEquals(NodeStatus.DISK_FULL, config.getStatusReason());
   }
 
+  @Test
+  public void testDiskRecoveryDoesNotUndoLaterStatusChange() {
+    for (NodeStatus status : NodeStatus.values()) {
+      if (status == NodeStatus.ReadOnly) {
+        continue;
+      }
+      CommonConfig config = new CommonConfig();
+      config.setNodeStatusWithReason(NodeStatus.ReadOnly, NodeStatus.DISK_FULL);
+      config.setNodeStatus(status);
+
+      config.recoverFromDiskFull();
+
+      Assert.assertEquals(status, config.getNodeStatus());
+      Assert.assertNull(config.getStatusReason());
+    }
+  }
+
   @Test
   public void testSetReadOnlyWithReasonPriority() {
     CommonConfig config = new CommonConfig();