Skip to content

Fix stale IoTConsensus peers after restart during removal - #18892

Merged
CRZbulabula merged 2 commits into
apache:masterfrom
d-wang-commit:fix-stale-peers-after-restart
Oct 19, 2026
Merged

CRZbulabula merged 2 commits into
apache:masterfrom
d-wang-commit:fix-stale-peers-after-restart

Conversation

@d-wang-commit

@d-wang-commit d-wang-commit commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Description

Restart during peer removal

A surviving IoTConsensus V1 replica can restart after the removed peer's sync channel has been deleted but before ConfigNode commits the updated replica set. Its startup response still includes the removed peer, so it recreates that peer's dispatcher. Previously, completing the metadata update did not correct this recovered membership. Failed replication to the removed peer could then pin the minimum sync index and keep syncLag above zero.

Reconcile committed membership

Commit the replica-set removal before entering the new RESET_IOT_CONSENSUS_V1_PEER_LIST state. For IoTConsensus V1 DataRegions, send the final peer list to every retained replica through the existing resetPeerList RPC, including temporarily unavailable replicas. The existing DataNode handler waits for consensus startup before applying the list.

Metadata failures and unsuccessful V1 resets perform an initial attempt and up to three retries per state, sleeping five seconds between retries. Exhaustion marks the procedure as failed. The business state is persisted by the existing procedure framework and can resume after ConfigNode recovery; the in-memory cycle counter restarts after recovery. Existing state IDs are unchanged, and the new state is appended. No DataNode production code or RPC definitions are changed.

Ratis and IoTConsensus V2 crash ITs now exclude the V1-only state from their expected kill points. V1 retains coverage of the new state, and the assertion requiring all expected kill points to trigger is preserved.

Tests

  • UT added: RestartDuringRemovePeerTest covers metadata ordering, metadata failures for V1 and V2, missing or unsuccessful reset responses, exceptions, recovery, idempotent replay, retry exhaustion, interruption, and protocol gating. RemovePeerRestartConsensusTest covers stale membership, syncLag convergence, continued writes, and another restart using real IoTConsensus instances and loopback Thrift RPCs. StalePeerListRestartReproductionTest provides the stale-startup reproduction and an explicit reset control.
  • IT modified: cnCrashTest in IoTDBRegionMigrateConfigNodeCrashForRatisIT, IoTDBRegionMigrateConfigNodeCrashIoTV2BatchIT, and IoTDBRegionMigrateConfigNodeCrashIoTV2StreamIT excludes the unreachable V1-only kill point.
  • UT run: All 20 targeted tests passed across RestartDuringRemovePeerTest, RemovePeerRestartConsensusTest, StalePeerListRestartReproductionTest, RemoveRegionPeerProcedureTest, and STMProcedureTest. The CN-driven regression reduced syncLag from 8 to 0; further writes and another restart kept it at 0.
  • IT run: All three modified cnCrashTest scenarios passed in real ConfigNode/DataNode processes with the DailyIT profile. Every expected kill point triggered, including the existing removal stages; the V1-only reset point was correctly excluded for Ratis and both V2 modes.
  • Build checks and boundaries: The full English reactor verify build, Chinese-locale full-reactor package build (tests skipped), Spotless for the three updated IT files, and git diff --check passed. The consensus component tests use same-JVM restarts, an in-memory state machine, and a test internal-service facade; they do not launch full DataNode processes. The three crash ITs exercise full node processes but do not assert syncLag.

Side effects and risks

  • If a retained replica cannot acknowledge the reset within all attempts, finalization fails and stale membership may remain uncorrected. No background reconciliation is added for exhausted or previously completed procedures.
  • Retry sleeps occupy a procedure worker, adding up to 15 seconds of sleep per failing state in addition to RPC time.
  • ConfigNodes recovering the new state must contain this change; older binaries cannot interpret its appended state ID.
  • Existing DataNode dispatcher shutdown behavior is unchanged. The component regression verifies membership and syncLag accounting, not complete dispatcher-thread termination.

This PR has:

  • been self-reviewed.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.

Key changed/added classes (or packages if there are too many classes) in this PR
  • RemoveRegionPeerProcedure: commit metadata, execute V1 reconciliation, and bound finalization retries.
  • RegionMaintainHandler: return metadata status and verify reset responses from every retained V1 replica.
  • RemoveRegionPeerState: append the V1 reconciliation state.
  • ProcedureMessages (en/zh): add reconciliation and retry diagnostics.
  • RestartDuringRemovePeerTest, RemovePeerRestartConsensusTest, StalePeerListRestartReproductionTest: cover restart, replication, and finalization behavior.
  • Ratis and IoTConsensus V2 ConfigNode crash ITs: filter the V1-only kill point.

@d-wang-commit
d-wang-commit marked this pull request as draft October 15, 2026 09:46
@d-wang-commit
d-wang-commit marked this pull request as ready for review October 16, 2026 11:11
Commit removal metadata before reconciling IoTConsensus V1 peer lists.
Retry metadata updates and V1 resets up to three times before failing.
Cover restart recovery and filter V1-only crash points in Ratis/V2 ITs.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Retry exhaustion can abort enclosing DataNode-removal cleanup and leave removal targets registered in Removing.

1 open issue
What changed in this PR

Repairs stale IoTConsensus V1 membership after a replica restarts during peer removal.

Changes:

  • Commits metadata before reconciling retained replicas, with bounded finalization retries.
  • Adds restart, recovery, and replication regression tests.
  • Updates localized diagnostics and protocol-specific crash-test expectations.
File Description
iotdb-core/​consensus/​src/​test/​java/​org/​apache/​iotdb/​consensus/​iot/​StalePeerListRestartReproductionTest.java Reproduces stale membership and verifies explicit repair.
iotdb-core/​confignode/​src/​test/​java/​org/​apache/​iotdb/​confignode/​procedure/​impl/​region/​RestartDuringRemovePeerTest.java Tests ordering, retries, recovery, and protocol gating.
iotdb-core/​confignode/​src/​test/​java/​org/​apache/​iotdb/​confignode/​procedure/​impl/​region/​RemovePeerRestartConsensusTest.java Verifies reconciliation with real consensus instances.
iotdb-core/​confignode/​src/​main/​java/​org/​apache/​iotdb/​confignode/​procedure/​state/​RemoveRegionPeerState.java Appends the V1 reconciliation state.
iotdb-core/​confignode/​src/​main/​java/​org/​apache/​iotdb/​confignode/​procedure/​impl/​region/​RemoveRegionPeerProcedure.java Adds reconciliation and bounded finalization retries.
iotdb-core/​confignode/​src/​main/​java/​org/​apache/​iotdb/​confignode/​procedure/​env/​RegionMaintainHandler.java Checks metadata results and retained-replica acknowledgments.
iotdb-core/​confignode/​src/​main/​i18n/​zh/​org/​apache/​iotdb/​confignode/​i18n/​ProcedureMessages.java Adds Chinese diagnostics.
iotdb-core/​confignode/​src/​main/​i18n/​en/​org/​apache/​iotdb/​confignode/​i18n/​ProcedureMessages.java Adds English diagnostics.
integration-test/​src/​test/​java/​org/​apache/​iotdb/​confignode/​it/​regionmigration/​pass/​daily/​ratis/​IoTDBRegionMigrateConfigNodeCrashForRatisIT.java Excludes the V1-only crash point.
integration-test/​src/​test/​java/​org/​apache/​iotdb/​confignode/​it/​regionmigration/​pass/​daily/​iotv2/​stream/​IoTDBRegionMigrateConfigNodeCrashIoTV2StreamIT.java Excludes the V1-only crash point.
integration-test/​src/​test/​java/​org/​apache/​iotdb/​confignode/​it/​regionmigration/​pass/​daily/​iotv2/​batch/​IoTDBRegionMigrateConfigNodeCrashIoTV2BatchIT.java Excludes the V1-only crash point.

Review effort: Balanced

Finalize drained nodes or restore previous statuses during rollback.
Retry failed cleanup without losing the original reconciliation failure.
Cover retry exhaustion and delayed cleanup with ProcedureExecutor tests.
@CRZbulabula
CRZbulabula merged commit 9bb1f4d into apache:master Oct 19, 2026
38 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants