elsa-workflows / elsa-workflows/elsa-core
To avoid getting stale data from IWorkflowInstanceStore, developers should have a way to ensure they are getting up-to-date state
- Dominant language
- C#
- Stars
- 7.9k
- Forks
- 1.5k
- Avg merge
- 15h 22m
- Merged PRs (30d)
- 114
Description
Currently in Elsa 2 there are a number of workflow persistence behaviours enumerated by `Elsa.Models.WorkflowPersistenceBehavior`; [the default behaviour is to persist upon completion of a burst of workflow activity]. Generally this is a good default, because it will avoid persisting state which will go out of date very quickly.
[the default behaviour is to persist upon completion of a burst of workflow activity]: https://github.com/elsa-workflows/elsa-core/blob/173ded683fa369b0515bf303c9d4df9279f3da14/src/core/Elsa.Core/Builders/WorkflowBuilder.cs#L26
## Example use-case & the problem
Sometimes, when using `IWorkflowInstanceStore`, it is important that the developer can get the _real_ up-to-date state of a workflow instance, even one which is being processed at that very moment.
For example, I have a use-case in which I have a (custom) workflow activity that sends "notifications". Those actual notifications are sent from a service within my application (dependency-injected into my activity) and their actual nature could vary. They might be email, SMS, push notifications to a native app etc etc. Consequently the precise content of those notifications is not known by the activity. The activity only passes the workflow correlation ID to that service. It is up to the actual notification-creating/sending logic (behind that service) to extract the relevant data for the notification which is going to be sent.
```csharp
public interface ISendsWorkflowNotifications
{
Task SendWorkflowNotificationsAsync(string workflowCorrelationId, CancellationToken cancellationToken = default);
}
```
In this case, the service makes use of `IWorkflowInstanceStore` in order to load the workflow by its correlation ID and then inspect its state for the relevant data. However, if the workflow persistence behaviour is anything but `ActivityExecuted` then it is likely that the workflow instance store will provide a stale/outdated version of the workflow instance's state, because the in-memory workflow instance which is being executed in-memory has not yet been flushed.
In our case, it means the notifications can be sent with outdated data from the workflow, using values from its variables which have been overwritten by the workflow, but not yet persisted with their new values.
## Workaround: Set persistence behaviour to `ActivityExecuted`
I could immediately set the persistence behaviour to `ActivityExecuted`, in which case the persisted state will be updated after every activity execution. This is not great though, because it is workflow-wide. It will mean that the workflow is persisted at many times when it doesn't need to be. I really only need it to be persisted just-before I execute one of those "send notifications" activities.
## Proposal: A new activity that flushes the workflow instance
One way to solve this problem would be a new activity named something like `PersistWorkflowInstance`. This would immediately trigger persistence for the current workflow instance (regardless of the current `WorkflowPersistenceBehavior` setting) thus ensuring that its persisted state is up-to-date.
I would then place one of these activities in my workflow design, "just before" my notification activity.
## Alternative: Use 2nd-layer persistence to ensure single-object-instance
This idea might not fly, but an alternative which also addresses the same problem would be to use a 2nd layer of persistence as a form of registry of current in-memory workflow instances. Then, when a store is used to get one or more workflow instances, any which already exist in that registry (because they are "alive" at that moment in time) are actually returned from the registry rather than the primary persistence back-end.
This might not be a desirable solution though because:
* Thread synchronisation problems (accessing the same mutable object instance)
* Horizontal scaling problems (a simple in-process registry won't fly once you have more than one server processing workflows, you'd need something more fancy)
### Other places this technique is used
NHibernate does this with its "Identity Map" aka "First level cache". If you get the same persisted entity from the database twice, you will actually end up with two references to the same object, and not two copies of the object.
Contributor guide
Research direction
Start with IWorkflowInstanceStore, Elsa.Models.WorkflowPersistenceBehavior, and the persistence configuration in src/core/Elsa.Core/Builders/WorkflowBuilder.cs around line 26. Compare the proposed flush activity with the alternative in-memory registry, then determine how current state can be guaranteed for notification activities without forcing workflow-wide persistence. Done means an agreed design and implementation path for retrieving up-to-date workflow state.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100