Skip to content

fix(durable): dispatch effects from Start, Tick and RunService - #235

Draft
reka18 wants to merge 1 commit into
fix/durable-canceled-servicefrom
fix/durable-effect-dispatch
Draft

reka18 wants to merge 1 commit into
fix/durable-canceled-servicefrom
fix/durable-effect-dispatch

Conversation

@reka18

@reka18 reka18 commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

What this change does

Domain effects now reach the handler from Start, Tick and RunService, not only Fire. Stacked on #234.

  • Every step path goes through one commitStep: append, dispatch, then checkpoint.
  • A failed dispatch fails the Handle before its checkpoint. Recover retries the effect, and fails with ErrEffectDispatch while the handler still does. This changes behavior: a Fire after a failed dispatch used to be accepted.
  • Start encodes the baseline, applies the initial entry's effects, then writes the baseline, and applies none if the entry failed. Those effects are at-least-once: a handler failure, a failed baseline write or overlapping Start calls re-apply them. Nothing can re-derive them after the baseline.
  • New durable.EffectInstance(ctx) gives a handler the instance, because effect ids repeat across instances (durable: defects found in the platform-redesign review #214 question 5). The proposed answer was to add the instance to the effect id. This takes the issue's other option and passes the instance to the handler, so the dispatched set needs no migration.
  • New state.ServiceRunner.WithResultTransform, an additive change to the frozen state module. durable uses it so a service result reaches Assign in its decoded form live as well as on replay. Without it, a retried effect could differ from the first attempt under the same id.
  • A KindedEffect that can't marshal now fails Tick and RunService, as it already failed Fire, even with no handler wired.

Please check these:

  • Departs from the proposed answer to durable: defects found in the platform-redesign review #214 question 4 ("Recover now, redispatch later"): the Handle stops, and Recover fails while the handler is down. A handler that always refuses an effect wedges the instance. This needs a ruling.
  • Question 3: a handler on a Store that isn't a DispatchStore still silently applies nothing, now on four paths instead of one. The proposed answer is to refuse the configuration.
  • Not fixed: effects from a parent transition an actor drives, including during Fire, aren't dispatched. ActorSystem drops those results, so this needs a state change. durable: defects found in the platform-redesign review #214 lists it.
  • Decoding live changes what a service's Assign sees: a struct arrives as a map, an integer as a float64 that loses precision above 2^53, and an error loses its type. The journal records the decoded value.
  • A handler's error now matches through errors.Is.
  • The first Recover of an older journal applies effects from uncompacted Tick and service steps, which were never dispatched.
  • A failed MarkDispatched fails the Handle, and Recover applies that effect again.

Why

Effects from entry, timer and service transitions never reached the handler, and a failed effect could be compacted away and never retried.

Relates to: #214

Evidence

On GOTOOLCHAIN=go1.27.3, in durable unless the row says otherwise:

Command Result
go test . in state Pass. It has no direct test of WithResultTransform; durable exercises it
go test ./... Pass, including TestRunner_EffectDispatch_Start_AppliesEntryEffect, _TimerTransition_AppliesEffect, _ServiceSettle_AppliesEffect and _FailedThenCheckpointed_AppliedOnce

Checklist

  • Commits are signed off (git commit -s) per the DCO
  • Conventional commit messages (type: subject)
  • mage check passes locally (lint + race tests + govulncheck): not run as a whole
  • Tests added/updated for the change
  • Public API changes documented (godoc) and, if applicable, noted for the changelog

@reka18 reka18 added not-ready On every new PR until all review dimensions are confirmed wait:correctness ACR pending: correctness wait:regression-safety ACR pending: regression-safety wait:concurrency-discipline ACR pending: concurrency-discipline wait:replay-determinism ACR pending: replay-determinism wait:api-compatibility ACR pending: api-compatibility wait:missed-context ACR pending: missed-context wait:human-readability ACR pending: human-readability labels Nov 5, 2026
@reka18
reka18 force-pushed the fix/durable-effect-dispatch branch from ebb5863 to ef9c0a8 Compare November 5, 2026 16:38
@reka18
reka18 force-pushed the fix/durable-effect-dispatch branch from ef9c0a8 to d46adf2 Compare November 5, 2026 16:40
@reka18
reka18 force-pushed the fix/durable-effect-dispatch branch from d46adf2 to cc4d8cc Compare November 5, 2026 16:53
@reka18
reka18 force-pushed the fix/durable-effect-dispatch branch from cc4d8cc to d657891 Compare November 5, 2026 17:09
@reka18
reka18 force-pushed the fix/durable-effect-dispatch branch from d657891 to 3e41947 Compare November 5, 2026 17:14
@reka18
reka18 force-pushed the fix/durable-effect-dispatch branch from 3e41947 to 3edd2ee Compare November 5, 2026 17:22
@reka18
reka18 force-pushed the fix/durable-effect-dispatch branch from 3edd2ee to aae543d Compare November 5, 2026 17:33
@reka18
reka18 added this pull request to stack #238 November 5, 2026 17:34
@reka18
reka18 force-pushed the fix/durable-effect-dispatch branch from aae543d to 50bbc98 Compare November 5, 2026 17:42
@reka18
reka18 force-pushed the fix/durable-effect-dispatch branch from 50bbc98 to a14c4ed Compare November 5, 2026 18:04
@reka18
reka18 force-pushed the fix/durable-effect-dispatch branch from a14c4ed to 90bcbfb Compare November 5, 2026 18:20
@reka18
reka18 force-pushed the fix/durable-effect-dispatch branch 3 times, most recently from c1ea314 to e8c7bf8 Compare November 5, 2026 20:10
@reka18
reka18 force-pushed the fix/durable-effect-dispatch branch from e8c7bf8 to 3db1a7b Compare November 5, 2026 20:23
@reka18
reka18 removed this pull request from stack #238 November 5, 2026 20:26
@reka18
reka18 added this pull request to stack #239 November 5, 2026 20:26
@reka18
reka18 force-pushed the fix/durable-effect-dispatch branch from 3db1a7b to 7427624 Compare November 5, 2026 21:02
@reka18 reka18 added ready All ACR dimensions confirmed - clear to merge confirmed:correctness ACR clean: correctness confirmed:regression-safety ACR clean: regression-safety confirmed:concurrency-discipline ACR clean: concurrency-discipline confirmed:replay-determinism ACR clean: replay-determinism confirmed:api-compatibility ACR clean: api-compatibility confirmed:missed-context ACR clean: missed-context confirmed:human-readability ACR clean: human-readability and removed not-ready On every new PR until all review dimensions are confirmed wait:correctness ACR pending: correctness wait:regression-safety ACR pending: regression-safety wait:concurrency-discipline ACR pending: concurrency-discipline wait:replay-determinism ACR pending: replay-determinism wait:api-compatibility ACR pending: api-compatibility wait:missed-context ACR pending: missed-context wait:human-readability ACR pending: human-readability labels Nov 5, 2026
@reka18

reka18 commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor Author

Review results. One reviewer per dimension, each reading the others' findings. All seven dimensions have zero blockers on the latest round.

Fixed along the way: EffectInstance replaced advice to dedupe on an id that repeats across instances. The docs now state when an effect can apply twice, and that a service result reaches Assign JSON-decoded, with an integer losing precision above 2^53. examples/fooddelivery accepts the decoded amount. The four effect-dispatch tests moved here from #213.

For a maintainer: questions 3 and 4 on #214 still need a ruling. The description says where this PR departs from the proposed answers.

Only Fire dispatched domain effects. Every step path now commits through one
helper that appends, dispatches, then checkpoints. A failed dispatch fails the
Handle before its checkpoint, so a later checkpoint can't compact the effect
away, and Recover retries it. Start applies the initial entry's effects before
the baseline, so a handler failure leaves no instance.

EffectInstance gives a handler its instance id, and state.ServiceRunner gains
WithResultTransform so a service's result reaches the machine in the form a replay
supplies.

Effects from a parent transition an actor drives are still not dispatched.

Relates to #214.

Signed-off-by: Ray Karnes <ray.karnes@stablekernel.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

confirmed:api-compatibility ACR clean: api-compatibility confirmed:concurrency-discipline ACR clean: concurrency-discipline confirmed:correctness ACR clean: correctness confirmed:human-readability ACR clean: human-readability confirmed:missed-context ACR clean: missed-context confirmed:regression-safety ACR clean: regression-safety confirmed:replay-determinism ACR clean: replay-determinism ready All ACR dimensions confirmed - clear to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant