fix(capabilities): refresh local node in engine on set of new DON #20059
Conversation
MStreet3
force-pushed
the
street_donSubscriber
branch
from
7a9f9bd to
ffbdd38
Compare
MStreet3
force-pushed
the
street_donSubscriber
branch
3 times, most recently
from
3d5c171 to
df3f43c
Compare
| platform.WorkflowRegistryAddress, cfg.WorkflowRegistryAddress, | ||
| platform.WorkflowRegistryChainSelector, cfg.WorkflowRegistryChainSelector, | ||
| platform.EngineVersion, platform.ValueWorkflowVersionV2, | ||
| platform.DonVersion, strconv.FormatUint(uint64(localNode.WorkflowDON.ConfigVersion), 10), |
There was a problem hiding this comment.
@patrickhuie19 please advise.. realizing that dynamic label update each time we get a new workflow DON is not yet handled in this PR. we could split to something different, seems like an atomic pointer on the beholder logger would be useful for a refactor.
There was a problem hiding this comment.
the labeler isn't static, so if we wire an accessor or a pointer to the engine labeler around to the right places, you could update the label. I'd prefer an accessor, or potentially even an update label functional hook provided by the eng to keep access locked down.
There was a problem hiding this comment.
| if h.OnRateLimited == nil { | ||
| h.OnRateLimited = func(executionID string) {} | ||
| } | ||
| if h.OnNodeSynced == nil { |
There was a problem hiding this comment.
what does this do?
how is it not-nil IRL?
There was a problem hiding this comment.
I use it to verify the state in tests, in practice there's no other use case for it. But we could put a metric counter in there in theory.
This reverts commit f52cf5a.
MStreet3
force-pushed
the
street_donSubscriber
branch
from
634d141 to
88dd221
Compare
|
…0059) * refactor(chainlink): app spacing * refactor(capabilities): adds subscribe method to notifier * refactor: passes don notifier to engine * fix: pass args to tests * refactor: uses atomic pointer for node state * deps: changeset * refactor: use local limit for local node timeout * refactor(workflows): adds tests for local node sync * refactor: adds additional test * refactor(v2/engine): simplifies node sync functions * chore: lint * refactor(capabilities): removes manual mutexes and resolve deadlock in test * fix(cre/utils): wires in mock subscriber to standalone engine * refactor(workflows/v2): load local node only once * fix(workflows/v1): pass don subscriber to v2 engine * fix: lint * Revert "fix: lint" This reverts commit f52cf5a. * refactor: moves struct * fix: removes unused * fix(capabilities): resolves race on channel read and close
5 participants
the
localNode, which carries the state of a workflow DON, is only set once when creating a workflow engine. this can lead to DON level issues if the state of the DON config differs between nodes. specifically, capability requests will differ, which will result in remote executable servers not reaching a quorum of requests.this fix injects a subscriber loop that listens for DON updates. a new node state is read from the cap registry on each update. node state is made into an atomic pointer.
Requires
Supports