CRZbulabula opened a new pull request, #17935: URL: https://github.com/apache/iotdb/pull/17935
## Problem During region migration (AddPeer), if the target peer failed to load the transferred snapshot, the failure was silently swallowed. The target's IoTConsensus RPC handler returned `SUCCESS` regardless, so the coordinator activated the new peer and `AddRegionPeerProcedure` / `RegionMigrateProcedure` were both marked successful. The control plane reported the migration complete while the destination replica actually held no data — and once the source replica was dropped, the data was silently lost (queries returned `count=0` / `max_time=null`). Observed (from the report): the destination DataNode logged `Exception occurs when loading snapshot ... Cannot find .../sequence/root.test/1 or .../unsequence/...` and `Fail to load snapshot`, yet immediately afterwards set the peer `active status to true`, and the ConfigNode logged `[AddRegion] success` and `[MigrateRegion] success`. ## Root cause The coordinator side is already correct: `IoTConsensusServerImpl.triggerSnapshotLoad` (the RPC sender) checks the response status and throws `ConsensusGroupModifyPeerException` on failure, which causes `addRemotePeer` to fail, the AddPeer task to be marked `FAIL`, and the procedure to roll back **without** deleting the source replica. The only broken link was that a snapshot-load failure was never reportable in the first place: - `IStateMachine.loadSnapshot` returned `void`. - `DataRegionStateMachine.loadSnapshot` caught the exception (or `null` region) and just logged it. - `IoTConsensusServerImpl.loadSnapshot` ignored the result (it carried a long-standing `// TODO: throw exception if the snapshot load failed`). - `IoTConsensusRPCServiceProcessor.triggerSnapshotLoad` therefore returned `SUCCESS` unconditionally. ## Fix Make snapshot-load failure reportable by changing `IStateMachine.loadSnapshot` to return `boolean` (`true` on success): - `DataRegionStateMachine` / `SchemaRegionStateMachine` / `ConfigRegionStateMachine` return `false` when loading fails (`SchemaRegionStateMachine` now guards its body so a failure is reported rather than thrown). - `IoTConsensusServerImpl.loadSnapshot` returns `false` if loading any receive folder fails (removing the TODO). - `IoTConsensusRPCServiceProcessor.triggerSnapshotLoad` returns a non-`SUCCESS` status when `loadSnapshot` fails, so the coordinator's existing error path fires and AddPeer fails instead of falsely succeeding. - `SimpleConsensusServerImpl` forwards the boolean; the Ratis `ApplicationStateMachineProxy` logs a load failure (its behavior is otherwise unchanged). Test state machines are updated accordingly. ## Test `AddPeerSnapshotLoadFailureTest` builds a real two-node IoTConsensus group and forces the target peer's `loadSnapshot` to fail. It verifies that `addRemotePeer`: - actually reaches the snapshot-load step (so the failure under test is the right one, not an earlier step), - throws `ConsensusException` instead of silently succeeding, - does not leave the target peer active with an incompletely-loaded snapshot. The test fails against the old code and passes with the fix. Existing IoTConsensus tests (`ReplicateTest`, `StabilityTest`) still pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
