Persist effect snapshot atomically with terminal status on succeed/fail#189
Open
stidsborg wants to merge 5 commits into
Open
Persist effect snapshot atomically with terminal status on succeed/fail#189stidsborg wants to merge 5 commits into
stidsborg wants to merge 5 commits into
Conversation
Make `effects` a required parameter on IFunctionStore.SucceedFunction and FailFunction and always pass the flow's full current effect snapshot. Each store now writes the effects column in the same UPDATE as the status change, so the terminal status and effect state land together in one atomic write rather than via a separate preceding flush. - PostgreSQL: add effects-column write branch to the succeed/fail SQL - SqlServer/MariaDB: source the effects bytes from the parameter, not the session - InMemory: persist the snapshot alongside the status update - Add Effect.GetAllStoredEffects() exposing the current effect snapshot - Thread the snapshot through PersistResult and PersistFailure (success and throw paths)
…edMessages Flaky in the SqlServer and MariaDB store suites; to be resolved or removed by upcoming functionality changes.
In SucceedFunction/FailFunction pack the effects parameter with BinaryPacker directly rather than routing through SnapshotStorageSession, and always write the column. The empty-effects guard added no value now that the caller always passes the real snapshot; element order is irrelevant since reads re-key by EffectId. SnapshotStorageSession keeps its serializer for create/postpone/suspend.
The method returns a filtered snapshot (pending deletes and null entries excluded), not all entries, so the name now reflects that.
The store now always passes a non-null effects pack, so the generators' `byte[]? effects = null` default and the `if (effects == null)` no-effects SQL branch were dead. Make the parameter required and keep only the with-effects statement across PostgreSQL, SqlServer and MariaDB.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Makes
effectsa required parameter onIFunctionStore.SucceedFunction/FailFunction, and has the completion path always pass the flow's full current effect snapshot. Each store now writes the effects column in the same UPDATE as the terminal status change, so status and effect state are persisted together in one atomic write instead of relying on a separate preceding flush.This is groundwork: it lets any effect set during the completion tail (e.g. a future "already-notified-parent" idempotency marker) become durable atomically with
Succeeded/Failed, which is the prerequisite for making completion idempotent and eventually retiring the bespokeTryInlinePendingMessagespath.Changes
effectsis now non-nullable onSucceedFunction/FailFunction.sessionsnapshot (unifying behaviour with PG).Effect.GetAllStoredEffects()(current snapshot, pending-deletes excluded) and threads it throughPersistResultandPersistFailureon both the success and throw paths.Notes
workflow.Effect.Flush()is intentionally kept, so ordering,AfterFlush(message clearing), and at-most-once effect timing are unchanged; the new write additionally guarantees the terminal snapshot is atomic with the status. The redundant delegate-effect write on succeed/fail can be streamlined in a follow-up.[Ignore]s the known-flakyControlPanelsExistingMessagesContainsPreviouslyAddedMessagesin the SqlServer/MariaDB suites.Testing
🤖 Generated with Claude Code