Skip to content

Commit cancel, resume and delete with their checkpoint, and refuse a self-target - #656

Draft
devhawk wants to merge 5 commits into
mainfrom
transactional-internal-steps
Draft

devhawk wants to merge 5 commits into
mainfrom
transactional-internal-steps

Conversation

@devhawk

@devhawk devhawk commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator

A DBOS operation called from a workflow committed its effect and its step checkpoint in separate transactions. A crash between the two made recovery run the operation again. This PR adds one helper that commits both together, and moves cancel, resume and delete onto it. It is the first of three stacked PRs that move every DBOS step onto the helper.

The helper

SystemDatabase.runTransactionalStep(caller, stepName, work) runs in one transaction at READ COMMITTED. The whole transaction is retried on connection failures, 40001 and 40P01.

  • From the workflow body: it checks whether the step is already recorded and replays it if so. Otherwise it runs the work and records the output as the step. A retry after a lost commit acknowledgement finds its own checkpoint and replays it.
  • From inside a step: it records nothing, but the work commits only while the caller still owns the workflow.
  • From a client or the Conductor: it runs as a plain transaction.

If the work fails, the transaction rolls back and the failure is recorded as the step's error in a fresh transaction, so a replay is refused the same way. This includes a conflict that outlasted its retries. Some failures are not recorded:

  • a lost ownership;
  • a checkpoint that disagrees with the call;
  • a failed commit, whose outcome is unknown;
  • a failure cut short by an interrupt.

Cancel, resume and delete

  • Tree cancel and delete: both now walk the tree level by level inside that transaction. A child whose insert holds its parent's lock is found once the insert commits, and a child inserted afterwards finds its parent gone or cancelled and rolls back.
  • Self-targets: a workflow can no longer cancel, resume or delete itself, or cancel or delete a tree it belongs to. The call throws IllegalArgumentException, which the workflow can catch. Before, its own step's ownership check would roll the call back and leave the workflow waiting on an outcome nothing writes.

Behaviour changes

  • DBOS.cancelWorkflow(DBOS.workflowId()) used to end the workflow CANCELLED; it now throws IllegalArgumentException. A workflow that cancelled itself under an earlier release, and is recovered after upgrading, hits the refusal at that step.
  • DBOS.send from inside a startWorkflow lambda now throws, as other DBOS steps there already did.
  • Two workflows cancelling each other can now deadlock. The database aborts one with 40P01, and its retry runs as if the other had committed first.

Notes

  • dbos-enterprise-java#21, already merged, moved an enterprise test off the WorkflowDAO.deleteWorkflows signature this changes.
  • CockroachDB: not run against CockroachDB. Where a cluster does not allow READ COMMITTED, the transaction runs SERIALIZABLE, and a child inserted during a tree walk surfaces as a 40001, which the helper retries.

Testing

New tests: InternalStepTransactionTest, SelfTargetTest and TransactionalStepTest. Together with OwnershipFenceTest they pass, 41 of 41. spotlessCheck, compileTestJava and javadoc pass.

🤖 Generated with Claude Code

…self-target

A DBOS operation called from a workflow committed its effect and the
checkpoint recording it in separate transactions, so a crash between the
two let a replay run the operation again. SystemDatabase gains
runTransactionalStep: from the workflow body, one transaction checks the
step (replaying a recorded output or error), runs the work, and records
it under the ownership check. If the work fails, the transaction rolls
back and the failure is recorded in a fresh one; a failure the retries
would have kept trying, a lost ownership, or a mismatched checkpoint is
not recorded. From inside a step nothing is recorded, but the work
commits only while the caller owns the workflow. Outside a workflow it
is a plain transaction.

cancelWorkflows, resumeWorkflows and deleteWorkflows now run through it,
in place of runDbosFunctionAsStep. A cancel with children is one
transaction that cancels each level before reading its children, so a
child whose insert holds its parent's lock is read once that insert
commits, and one inserted later is refused by the ownership check on its
parent. SendCaller becomes StepCaller, the caller every such operation
takes.

In one transaction, a workflow that cancels, resumes or deletes itself
would fail its own checkpoint's ownership check and wait forever. Such a
call is now refused with IllegalArgumentException, from the workflow
body or a step: the caller among the targets before any write, and a
cancel or delete whose tree contains the caller ("workflow X cannot
cancel the tree of R, which contains it"), rolling back any level
already cancelled. A workflow cancelling its own child's tree is
unaffected. Step names and recorded output shapes are unchanged.
Every DBOS cancel, resume and delete overload now says, under @throws IllegalArgumentException, that a call from inside a target, or one of its steps, or with children included from inside a target's descendants, is refused and changes nothing.
…in tests

From review:

- runTransactionalStep sets READ COMMITTED instead of inheriting the
  pool's level. A tree walk depends on it: each level's children read
  must see a child committed while the level's write waited on its
  parent, which a REPEATABLE READ snapshot would hide.
- deleteWorkflows walks the tree as cancelWorkflows does, deleting each
  level before reading its children, so a child started mid-delete is
  either found once its insert commits or refused because its parent is
  gone; it read the whole tree first, and a child committed between the
  read and the delete was left behind. Both share walkTree, which carries
  each workflow's root down the walk, so a refusal names the tree without
  reading rows the walk may already have changed; the ancestry reads go.
- A failure is recorded only when the exception that escaped is the
  work's own, or the retries' wrapping of it.
- Replaying a step with neither output nor error recorded throws, as the
  old path did. The cancel, resume and delete log lines run inside the
  work, so a replay or a refused call does not log them.
- sendBulkAs takes its caller from stepCaller().

Tests: the commit triggers fire only on the test's own workflow, and each
test asserts its trigger fired; a child whose insert holds its parent is
cancelled with the tree, and one inserted after the cancel is never
created (a new debug trigger holds the insert open); a fork replays a
recorded refusal instead of deciding it again; isRetryableFailure.
…docs

A cancel, resume or delete whose work still fails once its retries run
out, a serialization failure or deadlock included, now records that
failure as the step's error. Before, the step ID was used up with no row
behind it, so a workflow that caught the failure and carried on could
take the other branch on replay, when the call might succeed. Only a
failure an interrupt cut short is left unrecorded, along with a lost
ownership, a mismatched checkpoint and a failed commit, none of which
come from the work.

The cancel, resume and delete log lines now follow the DAO call, so a
refused self-target is not logged. The checkOwner javadoc no longer
claims fenced writes cannot deadlock: two workflows cancelling each
other can, and the retry resolves it. The runTransactionalStep javadoc
now says a lost commit acknowledgement is replayed only for a call from
the workflow body; from a step or a client the work runs again.

Tests cover a recorded conflict and its replay, a stale caller's cancel
rolling back from the body and from a step, and a child inserted during
or after a delete of its parent's tree.
@devhawk
devhawk added this pull request to stack #659 October 30, 2026 17:34
@devhawk
devhawk marked this pull request as draft October 30, 2026 17:38
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