Skip to content

Persist parent and child procedures atomically before scheduling - #19007

Open
d-wang-commit wants to merge 1 commit into
apache:masterfrom
d-wang-commit:fix-procedure-child-persistence
Open

d-wang-commit wants to merge 1 commit into
apache:masterfrom
d-wang-commit:fix-procedure-child-persistence

Conversation

@d-wang-commit

Copy link
Copy Markdown
Contributor

Description

Persist the parent's progress together with newly submitted children

When a Procedure returns new children, ProcedureExecutor.initializeChildren advances the parent's execution state to WAITING. The existing persistence path writes only the children before scheduling them. A later lock release can persist the parent, but a crash before that write leaves an old RUNNABLE parent alongside its already persisted children. Procedures that hold their lock across child execution, such as RemoveDataNodesProcedure, cannot rely on that later lock-release write either.

On recovery, the executor can schedule both the stale parent and its unfinished children. Re-executing the parent's submission step creates duplicate children with different procedure IDs. For region migration, competing add-peer procedures can advance and roll back the same replica independently.

Persist the parent's next state and all new children before scheduling any child. ConfigProcedureStore writes this batch as one UpdateProceduresPlan consensus entry, checks its returned status, and lets the executor retry failures with the existing backoff helper. An interrupted persistence retry must not fall through to scheduling children.

Preserve recovery and snapshot consistency

Each procedure in the batch is length-delimited and uses the existing single-procedure encoding, so optional trailing fields cannot consume the next record. Apply the batch under the same ProcedureInfo monitor used by recovery reads and snapshot creation, preventing either operation from observing a partially applied batch. Existing single-procedure entries and snapshot file formats remain readable.

Tests

  • Added ProcedureChildPersistenceTest: restart after one or several children are persisted, retry a failed batch before executing children, and stop during a failing retry without starting children. It exercises the real executor and serializers with a recording store and a controlled stop at the persistence boundary.
  • Negative control on the existing upstream executor: both restart tests failed because the saved parent remained RUNNABLE instead of WAITING. The test parent uses the existing default lock behavior; no production fault hook or downstream lock override is needed.
  • Added UpdateProceduresPlanTest: batch serialization and snapshot recovery, preservation of the old single-entry reader, one consensus write per batch, and rejection-status propagation.
  • Passed 142 UTs across ProcedureChildPersistenceTest, UpdateProceduresPlanTest, TestProcedureExecutor, STMProcedureTest, StateMachineProcedureTest, ProcedureInfoTest, ConfigPhysicalPlanSerDeTest, and ConfigPhysicalPlanTypeTest.
  • IT: none added or modified in this PR. Passed IoTDBRegionMigrateConfigNodeCrashForRatisIT#cnCrashDuringCreatePeerTest and IoTDBRegionMigrateConfigNodeCrashIoTV1IT#cnCrashDuringCreatePeerTest, using the unmodified upstream IT assertions on this independent branch.
  • The UT/IT run used mvn verify -pl integration-test -am -PDailyIT,with-integration-tests with the above selections and no-specified-test flags for upstream reactor modules. Spotless passed.
  • Full reactor mvn package -am -DskipTests and mvn package -am -DskipTests -Pwith-zh-locale both passed.

Side effects and risks

This introduces ConfigNode plan type 502. New readers retain support for old entries, but old ConfigNodes cannot decode the new batch type; mixed-version rolling upgrade and downgrade are not established by these tests. A batch entry grows with the parent and child records, and snapshot creation now excludes batch updates for its duration. Large child batches and slow snapshot disks were not stress-tested.

The change prevents duplicate child submission caused by mismatched persisted parent/child states. It does not make external DataNode actions exactly-once, repair existing inconsistent procedure records, or replace individual procedures' retry and rollback logic. Persistent store failures keep the children unscheduled until the batch succeeds or the executor is interrupted.


This PR has:

  • been self-reviewed.
    • concurrent read and write
  • added comments explaining the persistence ordering and record boundaries.
  • added unit tests covering the new persistence and recovery paths.
  • been tested in a test IoTDB cluster.

Key changed/added classes (or packages if there are too many classes) in this PR
  • ProcedureExecutor: persist the parent and new children before scheduling, retry store failures, and stop after interruption.
  • ConfigProcedureStore / IProcedureStore: use and document atomic batch persistence.
  • UpdateProceduresPlan / ConfigPhysicalPlan / ConfigPhysicalPlanType / ConfigPlanExecutor: encode, decode, and apply the batch consensus entry.
  • ProcedureInfo: exclude recovery reads and snapshots from partially applied batches.
  • ProcedureChildPersistenceTest / UpdateProceduresPlanTest: exercise recovery, failures, serialization, and consensus submission.

Commit parent progress and new children in one ConfigNode consensus entry.
Retry failed writes before scheduling and preserve recovery/snapshot consistency.
Add regression coverage for restart, retry, interruption, and serialization.
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.

1 participant