-
Notifications
You must be signed in to change notification settings - Fork 15
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
Worker should stop when operation is canceled #94
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're moving the timeout logic to SegmentAndReplacePods
, why not also do all the cancellation stuff there?
I think the sub-functions could only just receive a single context, and when it cancels it just stop doing its work, they don't really need to know the reason. And SegmentAndReplacePods
could return the reason why it stopped. What do you think?
I think it makes sense. Will make the change. |
When an operation is canceled the worker should know and stop the operation and the api should issue a rollback. Right now this is not happening because
OperationManager.WasCanceled
checks a cached state and never hits redis and the worker never starts the loop to check for operation updates.I changed to
OperationManager
to remove the loop and make it soWasCanceled
always check on redis. This works for most use cases inside maestro. To prevent having multiple goroutines hitting redis at the same time I moved the loop to inside the worker and separated fromOperationManager
.I also added tests to
SegmentAndReplacePods
that were missing and added a test case to the watcher for when an operation is canceled.