[Usein Mambediiev (usemam)]

Fixes #148

Terminal targets (Completed/Failed/Terminated) are now purged via the existing PurgeInstanceStateByID before the insert, mirroring CreateInstance's overwrite path. Active instances are untouched.

Tests

Re-enabled the disabled RetryFailedSubOrchestration(true) case; added ReuseCompletedSubOrchestrationID and CannotRecreateRunningSubOrchestrationInstance.

[Usein Mambediiev (usemam)]

Chris Gillum (@cgillum) could please take a look? Thank you.

[andystaples]

Requesting changes for two P1 data-consistency issues in the new sub-orchestration instance-ID reuse path:

1. A concurrent checkpoint can purge another parent's newly active child execution.
2. A duplicate checkpoint can commit a purge and restart of a completed child even though checkpoint deduplication raises an error.

Both issues were reproduced against this PR's SQL in an isolated SQL Server 2022 database. For the second issue, the same replay against the merge-base SQL preserves the completed child's status and history. The inline comments include the failure sequences and requested changes.

[andystaples]

[P1] Keep the terminal-state check stable through purge and recreation

This SELECT does not retain locks under READ COMMITTED, and can read an older version under READ_COMMITTED_SNAPSHOT. PurgeInstanceStateByID then deletes solely by instance ID, without rechecking the execution or terminal status. Another parent checkpoint or CreateInstance call can therefore recreate the selected child before this checkpoint reaches the DELETE, and this checkpoint will purge the new Pending/Running execution.

I reproduced this with two connections running the actual PR SQL: A selected the Completed child and blocked entering purge; B recreated that child as Pending and committed; A resumed, deleted B's start event, and installed its own child. B's parent retained its committed child-scheduling history but was left waiting for a deleted execution.

Please serialize and revalidate the child execution/status through replacement, while preserving consistent lock ordering, and add a deterministic concurrent-reuse regression. CannotReuseRunningSubOrchestrationInstanceID only covers an instance that was already Running before this SELECT, so it does not exercise this race.

[Usein Mambediiev (usemam)]

Fixed in 705d14f - checkpoint reads all started sub-orchestration rows with UPDLOCK (status filtered afterwards), holding them until commit, so nothing can recreate the child between check and purge. CreateInstance uses UPDLOCK too and waits for the in-flight reuse. No HOLDLOCK, because its range locks blocked unrelated inserts with adjacent IDs. Regression test ConcurrentReuseOfCompletedSubOrchestrationInstanceID pauses the parent's checkpoint mid-replacement while a client creates the same ID, and asserts that the client gets OrchestrationAlreadyExistsException and the parent completes.

[andystaples]

[P1] Roll back the child purge when checkpoint deduplication fails

This destructive purge happens before the parent History insert used to detect duplicate/split-brain checkpoints. If a parent checkpoint commits, its child completes, and the same checkpoint is replayed after a lost response or by a stale worker, this code purges the completed child before detecting the duplicate parent history.

The later History insert raises error 2627, but _CheckpointOrchestration has neither XACT_ABORT ON nor a TRY/CATCH rollback. With the default SQL session settings, execution continues to COMMIT, and CompleteTaskOrchestrationWorkItemAsync subsequently treats the unique-key error as a harmless duplicate.

I reproduced the rejected replay leaving the child Pending, with its history deleted, a fresh ExecutionStarted event queued, and @@TRANCOUNT = 0. The identical replay against the merge-base SQL preserves the child's Completed status and history. The rollback gap already exists, but this new purge turns it into deletion and re-execution of an already-completed child.

Please ensure checkpoint failure rolls back the entire transaction, or reject duplicates before destructive work, and add a regression that replays a checkpoint after its child has completed. Otherwise completed sub-orchestrations and their activities can execute again.

[Usein Mambediiev (usemam)]

Copied the fix from #332 - so if it merges before this one, I'll rebase.
_CheckpointOrchestration now sets XACT_ABORT ON, so the duplicate-history PK violation rolls back the entire checkpoint, including the sub-orchestration purge. Regression test FaultTesting.DuplicateCheckpointDoesNotPurgeCompletedSubOrchestration forces a history conflict on the parent turn that reuses a completed child's ID, then asserts the child keeps its Completed status and output and executes exactly twice overall.

[Usein Mambediiev (usemam)]

Hi andystaples
Sorry to bother. Just wanted to follow up if you had a chance to take a look at my fixes for the concerns you voiced in your review of this PR?
Thank you!