-
Notifications
You must be signed in to change notification settings - Fork 1.9k
(fix): Workflow Registry Syncer no longer errors on orphaned pending create events #19117
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
Conversation
|
I see you added a changeset file but it does not contain a tag. Please edit the text include at least one of the following tags:
|
|
|
||
| if len(pendingEvents) != 0 { | ||
| return nil, fmt.Errorf("invariant violation: some pending events were not handled in the reconcile loop: keys=%+v, len=%d", maps.Keys(pendingEvents), len(pendingEvents)) | ||
| w.lggr.Debugf("some pending events were not handled in the reconcile loop, cleaning up: keys=%+v, len=%d", maps.Keys(pendingEvents), len(pendingEvents)) |
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.
@justinkaseman Hmmm... the intention was for the code above to handle all cases, including cleaning up any pending events that are no longer relevant. (See for instance this clause which illustrates what I mean: https://github.com/smartcontractkit/chainlink/pull/19117/files#diff-66829056a38220ab64391f36184d372d6ec0e3c03ef7ae393f103ecd378b1d92L268)
Do you have an example of a scenario that wasn't handled by the code above?
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.
Yes, the one laid out in the PR description and test.
That's good context. I do like that intention. Let me think on if there is a way to handle it instead of removing the error.
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.
Okay I changed it to specifically only handle the "pending create event is left over" case, by checking if the create event is no longer in wf metadata. If it's not there, then it's safe to throw it away, because it's no longer a workflow that needs to be created.
|




Description
In the situation where a workflow fails to be created by the Workflow Registry Syncer, it is put into a pending state that will exponentially back off.
While something is in the pending state, if it then gets deleted from the Workflow Registry, it will no longer show up during the Workflow Metadata query - which leaves an orphaned pending event.
There is no mechanism to clean these up. Instead the current behavior is to error out as an invariant. This scenario is not an invariant.
Note
This change and the change to handle delete events first will be back ported to V1 WF Registry Syncer after this PR is merged.