[CCIP-12469] fix(commit): make observations non-blocking on stuck ops - #2193
Merged
Conversation
- Replace the free WaitForAllNoErrOperations (which ended in an unconditional wg.Wait) with a stateful asynclib.Runner whose WaitForAll returns on timeout, so one operation that ignores its context can no longer hang the whole Observation / OCR round. - Add a per-operation in-flight guard: an op whose previous invocation is still running is skipped rather than re-spawned, bounding goroutines to the number of distinct ops instead of accumulating one stuck goroutine per round. - Return only operations that completed this round; a missing result degrades to its zero value (empty), and a late result from an abandoned goroutine is discarded rather than served, so consensus never acts on stale data.
|
makramkd
marked this pull request as ready for review
July 15, 2026 11:14
Collaborator
Author
|
Verified on staging - commit plugin no longer blocks on a misconfigured Sui plugin. |
KodeyThomas
approved these changes
Jul 15, 2026
RensR
approved these changes
Jul 15, 2026
FelixFan1992
pushed a commit
that referenced
this pull request
Jul 15, 2026
* fix(commit): make observations non-blocking on stuck ops - Replace the free WaitForAllNoErrOperations (which ended in an unconditional wg.Wait) with a stateful asynclib.Runner whose WaitForAll returns on timeout, so one operation that ignores its context can no longer hang the whole Observation / OCR round. - Add a per-operation in-flight guard: an op whose previous invocation is still running is skipped rather than re-spawned, bounding goroutines to the number of distinct ops instead of accumulating one stuck goroutine per round. - Return only operations that completed this round; a missing result degrades to its zero value (empty), and a late result from an abandoned goroutine is discarded rather than served, so consensus never acts on stale data. * lint
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
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.
Problem
The commit plugin's
chainfee.Observationwas hanging indefinitely in production. Root cause:asynclib.WaitForAllNoErrOperationsfans its operations out into goroutines and ends with an unconditionalwg.Wait(). Despite setting atimeouton the context, however, it was not being honored by the LOOPP code because other callers are calling with an unbounded context (CCIP launcher here: https://smartcontract-it.atlassian.net/browse/CCIP-12413 already fixed, but headreporter has the same issue, still not fixed https://smartcontract-it.atlassian.net/browse/CCIP-12468) which caused the plugin caller to wait on aLock()that would never get unlocked.Solution
Replace the free function with a stateful
asynclib.Runner:WaitForAllwaits for completions or the timeout, whichever comes first, and proceeds with what finished. A stuck operation can no longer hold the round hostage.Both commit observers (
chainfee,tokenprice) now run through a reusedRunner. This is defense-in-depth at the plugin boundary: the plugin can no longer be blocked by any downstream that ignores its context, independent of the chainlink-common connection fix and the launcher timeout fix in the companion PRs.Testing
WaitForAllreturn within the timeout; a stuck operation is not re-spawned on the next round.ApplyDefaultsAndValidate).