CRZbulabula opened a new pull request, #18075:
URL: https://github.com/apache/iotdb/pull/18075
## Problem
Running `extend region <regionId> to <nodeId>` (or `reconstruct region
<regionId> on <nodeId>`) where `<nodeId>` does **not** belong to any registered
DataNode — e.g. it is a **ConfigNode id** or simply a **non-existent id** like
`9999` — crashes the ConfigNode RPC handler with a `NullPointerException`, and
the client only sees a misleading error:
```
Msg: org.apache.iotdb.jdbc.IoTDBSQLException: 305: Error in calling method
extendRegion,
because: Fail to connect to any config node. Please check status of
ConfigNodes or logs of connected DataNode
```
ConfigNode log:
```
ERROR o.a.t.ProcessFunction:47 - Internal error processing extendRegion
java.lang.NullPointerException: Cannot invoke
"org.apache.iotdb.common.rpc.thrift.TDataNodeLocation.getDataNodeId()"
because "targetDataNode" is null
at
org.apache.iotdb.confignode.manager.ProcedureManager.checkExtendRegion(ProcedureManager.java:...)
at
org.apache.iotdb.confignode.manager.ProcedureManager.extendOneRegion(ProcedureManager.java:...)
...
```
## Root cause
`NodeManager.getRegisteredDataNode(id)` is backed by
`registeredDataNodes.getOrDefault(id, new TDataNodeConfiguration())`, so for an
unregistered id it returns an **empty** `TDataNodeConfiguration` whose
`location` is `null`. In `extendOneRegion` / `reconstructRegion`:
```java
final TDataNodeLocation targetDataNode =
configManager.getNodeManager().getRegisteredDataNode(req.getDataNodeId()).getLocation();
// -> null
```
`targetDataNode` becomes `null`. `checkExtendRegion` and
`checkReconstructRegion` then dereference `targetDataNode.getDataNodeId()`
**unconditionally**, throwing the NPE. Because the exception propagates
uncaught out of the Thrift handler, the client reports the generic "Fail to
connect to any config node" instead of the real reason.
`regionOperationCommonCheck` already detects this case and produces the
message `"Cannot find Target DataNode"`, but the buggy methods dereferenced the
null target *before* honoring that result.
For comparison, `checkMigrateRegion` is already safe — it chains
`regionOperationCommonCheck` as the first branch of an `if/else-if` and
short-circuits before any dereference — and `checkRemoveRegion` is safe via
`@Nullable` + explicit null guards.
## Fix
Reorder `checkExtendRegion` and `checkReconstructRegion` to follow the safe
`checkMigrateRegion` pattern: run `regionOperationCommonCheck` as the first
branch of the `if/else-if` chain so that a `null` target short-circuits with
`"Cannot find Target DataNode"` and the subsequent branches are only reached
when `targetDataNode` is non-null. The parameters are annotated `@Nullable`; a
dead `conf` local in `checkReconstructRegion` is removed.
Result: no more NPE, and the client receives a clear, correct error message.
## Tests
Added two regression integration tests asserting the operation fails with a
clear message (contains `Cannot find Target DataNode`) and **never** surfaces a
`NullPointerException`:
-
`IoTDBRegionGroupExpandAndShrinkForIoTV1IT#extendRegionToInvalidDataNodeTest`
- `IoTDBRegionReconstructForIoTV1IT#reconstructRegionToInvalidDataNodeTest`
--
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]
