This is an automated email from the ASF dual-hosted git repository. CRZbulabula pushed a commit to branch fix-extend-region-npe-invalid-target in repository https://gitbox.apache.org/repos/asf/iotdb.git
commit c5dcd0da5d5af821d0aacd4191fe63f934da3558 Author: Yongzao <[email protected]> AuthorDate: Wed Jul 1 01:32:59 2026 +0800 Fix NullPointerException when extend/reconstruct region targets a non-DataNode id When the target node id of `extend region` (or `reconstruct region`) does not belong to any registered DataNode -- for example it is a ConfigNode id or simply does not exist -- NodeManager.getRegisteredDataNode returns an empty TDataNodeConfiguration whose location is null. checkExtendRegion and checkReconstructRegion then dereferenced targetDataNode.getDataNodeId() unconditionally, throwing a NullPointerException in the ConfigNode RPC handler. The client only saw a misleading "Fail to connect to any config node" message. regionOperationCommonCheck already reports "Cannot find Target DataNode" for a null target, so reorder both checks to follow the safe checkMigrateRegion pattern: run regionOperationCommonCheck first and short-circuit before any targetDataNode dereference. The client now receives a clear, correct error. Add regression ITs for both the extend and reconstruct paths. --- .../IoTDBRegionGroupExpandAndShrinkForIoTV1IT.java | 56 ++++++++++++++++++++++ .../commit/IoTDBRegionReconstructForIoTV1IT.java | 55 +++++++++++++++++++++ .../iotdb/confignode/manager/ProcedureManager.java | 56 +++++++++++++--------- 3 files changed, 145 insertions(+), 22 deletions(-) diff --git a/integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/commit/IoTDBRegionGroupExpandAndShrinkForIoTV1IT.java b/integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/commit/IoTDBRegionGroupExpandAndShrinkForIoTV1IT.java index 1e1b300fdf6..50753d06abc 100644 --- a/integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/commit/IoTDBRegionGroupExpandAndShrinkForIoTV1IT.java +++ b/integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/commit/IoTDBRegionGroupExpandAndShrinkForIoTV1IT.java @@ -36,6 +36,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.sql.Connection; +import java.sql.SQLException; import java.sql.Statement; import java.util.ArrayList; import java.util.List; @@ -120,6 +121,61 @@ public class IoTDBRegionGroupExpandAndShrinkForIoTV1IT } } + /** + * Regression test for TIMECHODB-0689: when the target id of "extend region" does not belong to + * any registered DataNode (e.g. it is a ConfigNode id or simply does not exist), the ConfigNode + * used to throw a NullPointerException in {@code checkExtendRegion} and the client only saw a + * misleading "Fail to connect to any config node" message. After the fix a clear, actionable + * error message must be returned instead. + */ + @Test + public void extendRegionToInvalidDataNodeTest() throws Exception { + EnvFactory.getEnv() + .getConfig() + .getCommonConfig() + .setDataRegionConsensusProtocolClass(ConsensusFactory.IOT_CONSENSUS) + .setSchemaRegionConsensusProtocolClass(ConsensusFactory.RATIS_CONSENSUS) + .setDataReplicationFactor(1) + .setSchemaReplicationFactor(1); + + EnvFactory.getEnv().initClusterEnvironment(1, 3); + + try (final Connection connection = makeItCloseQuietly(EnvFactory.getEnv().getConnection()); + final Statement statement = makeItCloseQuietly(connection.createStatement())) { + // prepare data so that at least one region exists + statement.execute(INSERTION1); + statement.execute(FLUSH_COMMAND); + + Map<Integer, Set<Integer>> regionMap = getAllRegionMap(statement); + Set<Integer> allDataNodeId = getAllDataNodes(statement); + Assert.assertFalse(regionMap.isEmpty()); + + int selectedRegion = regionMap.keySet().iterator().next(); + // an id that is guaranteed not to belong to any registered DataNode; a ConfigNode id triggers + // the exact same code path (getRegisteredDataNode returns an empty configuration whose + // location is null) + int invalidDataNodeId = 9999; + Assert.assertFalse(allDataNodeId.contains(invalidDataNodeId)); + + try { + statement.execute(String.format(EXPAND_FORMAT, selectedRegion, invalidDataNodeId)); + Assert.fail("extend region to a non-existent DataNode is expected to fail"); + } catch (SQLException e) { + String message = e.getMessage(); + LOGGER.info("extend region to invalid DataNode failed as expected: {}", message); + Assert.assertNotNull(message); + // the ConfigNode must not crash with an NPE any more ... + Assert.assertFalse( + "ConfigNode should not throw NullPointerException, but got: " + message, + message.contains("NullPointerException")); + // ... and the client should receive a clear, correct error message + Assert.assertTrue( + "Expected a 'Cannot find Target DataNode' style error but got: " + message, + message.contains("Cannot find Target DataNode")); + } + } + } + private void regionGroupExpand( Statement statement, SyncConfigNodeIServiceClient client, diff --git a/integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/commit/IoTDBRegionReconstructForIoTV1IT.java b/integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/commit/IoTDBRegionReconstructForIoTV1IT.java index ccde946ada5..6eca425ca4a 100644 --- a/integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/commit/IoTDBRegionReconstructForIoTV1IT.java +++ b/integration-test/src/test/java/org/apache/iotdb/confignode/it/regionmigration/pass/commit/IoTDBRegionReconstructForIoTV1IT.java @@ -45,6 +45,7 @@ import org.slf4j.LoggerFactory; import java.io.File; import java.sql.Connection; +import java.sql.SQLException; import java.sql.Statement; import java.util.Collections; import java.util.Map; @@ -220,4 +221,58 @@ public class IoTDBRegionReconstructForIoTV1IT extends IoTDBRegionOperationReliab } } } + + /** + * Regression test for TIMECHODB-0689 (reconstruct path): targeting "reconstruct region" at an id + * that is not a registered DataNode (a ConfigNode id or a non-existent id) used to trigger a + * NullPointerException in {@code checkReconstructRegion}. After the fix a clear, correct error + * message must be returned instead of crashing the ConfigNode RPC. + */ + @Test + public void reconstructRegionToInvalidDataNodeTest() throws Exception { + EnvFactory.getEnv() + .getConfig() + .getCommonConfig() + .setDataRegionConsensusProtocolClass(ConsensusFactory.IOT_CONSENSUS) + .setSchemaRegionConsensusProtocolClass(ConsensusFactory.RATIS_CONSENSUS) + .setDataReplicationFactor(1) + .setSchemaReplicationFactor(1); + + EnvFactory.getEnv().initClusterEnvironment(1, 3); + + try (Connection connection = makeItCloseQuietly(EnvFactory.getEnv().getConnection()); + Statement statement = makeItCloseQuietly(connection.createStatement())) { + // prepare data so that at least one region exists + statement.execute(INSERTION1); + statement.execute(FLUSH_COMMAND); + + Map<Integer, Set<Integer>> regionMap = getAllRegionMap(statement); + Set<Integer> allDataNodeId = getAllDataNodes(statement); + Assert.assertFalse(regionMap.isEmpty()); + + int selectedRegion = regionMap.keySet().iterator().next(); + // an id that is guaranteed not to belong to any registered DataNode; a ConfigNode id triggers + // the exact same code path (getRegisteredDataNode returns an empty configuration whose + // location is null) + int invalidDataNodeId = 9999; + Assert.assertFalse(allDataNodeId.contains(invalidDataNodeId)); + + try { + statement.execute(String.format(RECONSTRUCT_FORMAT, selectedRegion, invalidDataNodeId)); + Assert.fail("reconstruct region on a non-existent DataNode is expected to fail"); + } catch (SQLException e) { + String message = e.getMessage(); + LOGGER.info("reconstruct region on invalid DataNode failed as expected: {}", message); + Assert.assertNotNull(message); + // the ConfigNode must not crash with an NPE any more ... + Assert.assertFalse( + "ConfigNode should not throw NullPointerException, but got: " + message, + message.contains("NullPointerException")); + // ... and the client should receive a clear, correct error message + Assert.assertTrue( + "Expected a 'Cannot find Target DataNode' style error but got: " + message, + message.contains("Cannot find Target DataNode")); + } + } + } } diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java index 855e9f4a4ed..b42a1df5324 100644 --- a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java +++ b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java @@ -870,19 +870,24 @@ public class ProcedureManager { private TSStatus checkReconstructRegion( TReconstructRegionReq req, TConsensusGroupId regionId, - TDataNodeLocation targetDataNode, + @Nullable TDataNodeLocation targetDataNode, TDataNodeLocation coordinator) { - String failMessage = - regionOperationCommonCheck( - regionId, - targetDataNode, - Arrays.asList( - new Pair<>("Target DataNode", targetDataNode), - new Pair<>("Coordinator", coordinator)), - req.getModel()); - - ConfigNodeConfig conf = ConfigNodeDescriptor.getInstance().getConf(); - if (configManager + String failMessage; + // regionOperationCommonCheck reports "Cannot find Target DataNode" when targetDataNode is null + // (e.g. the requested id belongs to a ConfigNode or does not exist), so the following branches + // are only reached when targetDataNode is non-null. Keeping it as the first branch of the + // if-else chain avoids dereferencing a null targetDataNode. + if ((failMessage = + regionOperationCommonCheck( + regionId, + targetDataNode, + Arrays.asList( + new Pair<>("Target DataNode", targetDataNode), + new Pair<>("Coordinator", coordinator)), + req.getModel())) + != null) { + // need to do nothing more + } else if (configManager .getPartitionManager() .getAllReplicaSetsMap(regionId.getType()) .get(regionId) @@ -912,17 +917,24 @@ public class ProcedureManager { private TSStatus checkExtendRegion( TExtendRegionReq req, TConsensusGroupId regionId, - TDataNodeLocation targetDataNode, + @Nullable TDataNodeLocation targetDataNode, TDataNodeLocation coordinator) { - String failMessage = - regionOperationCommonCheck( - regionId, - targetDataNode, - Arrays.asList( - new Pair<>("Target DataNode", targetDataNode), - new Pair<>("Coordinator", coordinator)), - req.getModel()); - if (configManager + String failMessage; + // regionOperationCommonCheck reports "Cannot find Target DataNode" when targetDataNode is null + // (e.g. the requested id belongs to a ConfigNode or does not exist), so the following branches + // are only reached when targetDataNode is non-null. Keeping it as the first branch of the + // if-else chain avoids dereferencing a null targetDataNode. + if ((failMessage = + regionOperationCommonCheck( + regionId, + targetDataNode, + Arrays.asList( + new Pair<>("Target DataNode", targetDataNode), + new Pair<>("Coordinator", coordinator)), + req.getModel())) + != null) { + // need to do nothing more + } else if (configManager .getPartitionManager() .getAllReplicaSets(targetDataNode.getDataNodeId()) .stream()
