mirror of
https://github.com/hanzoai/tasks.git
synced 2026-10-04 02:04:03 +00:00
Dan DavisonandGitHub
2aa673b629
## What changed? Skip reapplying an `UpdateAdmitted` event if an update with that ID already exists. ## Why? In a split-brain scenario it is possible for both clusters to accept an update with the same ID. Prior to this PR, conflict resolution would create an `UpdateAdmitted` event via a reapply, despite the fact that the update ID conflicts with the other update. This state would not be correct: the update registry can only hold one of them and it would not be possible to interact with both updates (i.e. the worker will not be able to accept/reject/complete both of them). ### Summary: - We have a function that takes a slice of events and decides whether to "reapply" them. - It is called during `WorkflowReset` (might need to reapply a signal/update in the portion of history being lost) and on conflict resolution (might need to reapply a signal/update accepted in another cluster during a split brain situation) - In the conflict resolution case, there's a possibility that the incoming accepted request might have an ID that conflicts with one already in-flight - But that's not possible in the reset case (the workflow state was consistent before reset) - So we need to check for this in the conflict resolution case at least. That requires having access to a registry (representing the state of the receiving branch) - The PR passes the registry down through the stack to the reapply function. - Reset doesn't need the check, so we use a nil registry argument in that case. ## How did you test it? Test in PR simulates a split-brain situation and performs a reset to generate the `UpdateAdmitted` event: `TestHistoryReplicationSignalsAndUpdatesTestSuite/TestConflictResolutionReappliesUpdatesSameIds ` ## Potential risks It could break workflow reset and history event conflict resolution if it is not correct. ## Is hotfix candidate? Yes, it should be included in 1.24.