-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
fix: Synchronization lock handling in Step/DAG Template level #5081
Conversation
Signed-off-by: Saravanan Balasubramanian <sarabala1979@gmail.com>
woc.controller.syncManager.Release(woc.wf, node.ID, tmpl.Synchronization) | ||
} | ||
} else { | ||
if !sgNode.Fulfilled() { |
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.
This is a bug. Code is releasing parent lock if child completes
@@ -1785,6 +1785,12 @@ func (woc *wfOperationCtx) executeTemplate(ctx context.Context, nodeName string, | |||
} | |||
} | |||
|
|||
if node.Fulfilled() { |
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.
This is the right place to check the node status and release the lock.
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.
Good work. I love how thorough your tests are.
Can I make an ask?
I see this logged a lot:
controller | time="2021-02-11T08:36:08.398Z" level=info msg="Check the workflow existence"
Could we either:
- Make this DEBUG?
- Add some fields to it to give it a bit more context? Currently I need to look at the source code to understand it. Most users will not do that.
Signed-off-by: Saravanan Balasubramanian <sarabala1979@gmail.com>
Checklist: