fix: delay resuming workflows after restart#41
Merged
jason-lynch merged 2 commits intomainfrom Jun 3, 2025
Merged
Conversation
jason-lynch
force-pushed
the
fix/PLAT-99/workflow-resume-delay
branch
from
9166097 to
53e1663
Compare
The `go-workflows` backend locks workflows and activities while they're being executed. In the event of a crash or restart, the lock would prevent the worker from resuming the workflow. This fix makes a few changes to resolve this issue: - Uses a stable ID for the worker ID - Adds a random "worker instance" ID - Adds the new IDs to the lock entities - Incorporates the new IDs into the lock checks If the worker ID matches the lock's worker ID but the worker instance ID does not match, we know that the lock belonged to an old instance of the worker and that the lock can be reassigned to the new instance. PLAT-99
jason-lynch
force-pushed
the
fix/PLAT-99/workflow-resume-delay
branch
from
53e1663 to
2710268
Compare
tsivaprasad
requested changes
Jun 3, 2025
| Create(&workflow_instance_lock.Value{ | ||
| WorkflowInstanceID: instanceID, | ||
| WorkflowExecutionID: executionID, | ||
| CreatedAt: time.Now(), |
Contributor
There was a problem hiding this comment.
We're calling time.Now() multiple times. I think we can optimize it by calling it once—now := time.Now()—and reuse. What do you think?
| @@ -252,15 +254,34 @@ func (b *Backend) GetWorkflowTask(ctx context.Context, queues []workflow.Queue) | |||
| instanceID := item.WorkflowInstance.InstanceID | |||
Contributor
There was a problem hiding this comment.
The GetWorkflowTask method looks a bit large and complex with 3 nested loops. It might be a good idea to refactor. What do you think?
Member
Author
There was a problem hiding this comment.
Yes, this package could absolutely be refactored. But, I would prefer to do that as its own ticket, because changes to this package need to be made very carefully. This ticket is just focused on fixing a bug.
tsivaprasad
approved these changes
Jun 3, 2025
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
2 participants
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.
The
go-workflowsbackend locks workflows and activities while they're being executed. In the event of a crash or restart, the lock would prevent the worker from resuming the workflow. This fix makes a few changes to resolve this issue:If the worker ID matches the lock's worker ID but the worker instance ID does not match, we know that the lock belonged to an old instance of the worker and that the lock can be reassigned to the new instance.
PLAT-99