Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove CancellationToken from writing persistence operations #1079

Closed

Conversation

mamidenn
Copy link
Contributor

@mamidenn mamidenn commented Aug 8, 2022

Describe the change
Removed the CancellationToken parameter from all writing persistence operations. This prevents cancelling workflows or stopping the host from cancelling writing persistence operations and thus losing data.

Fixes #953
Fixes #1032

Describe your implementation or design
Removed the CancellationToken parameter from all writing persistence operations.

Tests
Yes. I reproduced the issue by adding a StopScenario for persistence providers. After the change, this scenario passes.

Breaking change
Yes. Removed CancellationToken parameter from multiple public methods.

@mamidenn mamidenn force-pushed the fix-race-condition-on-stop branch 3 times, most recently from 67af11f to 78b5828 Compare August 8, 2022 10:30
If the host is stopped as soon as WorkflowCompleted LifeCycleEvent is
raised, some persistence providers cannot persist the 'Completed' state
in time.
Writing persistence operations can no longer be cancelled to prevent
data loss when cancelling a workflow.

Fixes danielgerlag#953, danielgerlag#1032
@mamidenn mamidenn force-pushed the fix-race-condition-on-stop branch from 78b5828 to c92d06b Compare August 8, 2022 10:40
@mamidenn
Copy link
Contributor Author

mamidenn commented Aug 8, 2022

Here is an alternative, less invasive approach:
master...mamidenn:workflow-core:fix-race-condition-on-stop-min

@danielgerlag
Copy link
Owner

Here is an alternative, less invasive approach: master...mamidenn:workflow-core:fix-race-condition-on-stop-min

Yes, I like this approach better ^

@mamidenn
Copy link
Contributor Author

Yes, I like this approach better ^

Closing this in favor of #1081 then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants