From 2fc7eb4db762cc658030406577d42313092d33a7 Mon Sep 17 00:00:00 2001 From: Di Wang Date: Wed, 14 Oct 2026 16:43:31 +0800 Subject: [PATCH] Refactor node status recovery from DiskFull Move disk recovery checks into CommonConfig and share status updates. Keep recovery limited to ReadOnly/DiskFull and block it once shutdown starts. Add regression tests for shutdown and later status changes. --- .../impl/DataNodeInternalRPCServiceImpl.java | 8 +---- ...ataNodeInternalRPCServiceImplDiskTest.java | 19 +++++++++++ .../iotdb/commons/conf/CommonConfig.java | 33 ++++++++++--------- .../iotdb/commons/conf/CommonConfigTest.java | 17 ++++++++++ 4 files changed, 55 insertions(+), 22 deletions(-) diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/DataNodeInternalRPCServiceImpl.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/DataNodeInternalRPCServiceImpl.java index adce7c280fcd6..10e8d6f882d61 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/impl/DataNodeInternalRPCServiceImpl.java +++ b/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(); } } } diff --git a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/protocol/thrift/impl/DataNodeInternalRPCServiceImplDiskTest.java b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/protocol/thrift/impl/DataNodeInternalRPCServiceImplDiskTest.java index e397baabb1672..a8d1505a8d0e2 100644 --- a/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/protocol/thrift/impl/DataNodeInternalRPCServiceImplDiskTest.java +++ b/iotdb-core/datanode/src/test/java/org/apache/iotdb/db/protocol/thrift/impl/DataNodeInternalRPCServiceImplDiskTest.java @@ -40,6 +40,7 @@ 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; @@ -47,6 +48,8 @@ public class DataNodeInternalRPCServiceImplDiskTest { @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()); + } } diff --git a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/conf/CommonConfig.java b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/conf/CommonConfig.java index d307e8bfa365f..1b5f486ba0508 100644 --- a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/conf/CommonConfig.java +++ b/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; } diff --git a/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/conf/CommonConfigTest.java b/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/conf/CommonConfigTest.java index 86d959c6d552d..45ef34cc67db4 100644 --- a/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/conf/CommonConfigTest.java +++ b/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();