-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Log provider edge cases - combined changes #13527
Conversation
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider/log.go
Outdated
Show resolved
Hide resolved
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider/buffer_v1.go
Show resolved
Hide resolved
@@ -105,7 +106,11 @@ func (b *logBuffer) Enqueue(uid *big.Int, logs ...logpoller.Log) (int, int) { | |||
b.setUpkeepQueue(uid, buf) | |||
} | |||
|
|||
latestLogBlock, uniqueBlocks := blockStatistics(logs...) | |||
latestLogBlock, uniqueBlocks, reorgBlocks := b.blockStatistics(logs...) |
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.
now that this function accesses and changes buffer state, it seems we need to acquire a 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.
discussed offline, we'll need to combine lock for blockStatistics and evictReorgdLogs (maybe combine those)
also would be good to think through concurrency of the whole function
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider/buffer_v1.go
Outdated
Show resolved
Hide resolved
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider/buffer_v1.go
Show resolved
Hide resolved
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider/buffer_v1.go
Outdated
Show resolved
Hide resolved
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider/buffer_v1.go
Show resolved
Hide resolved
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider/buffer_v1.go
Outdated
Show resolved
Hide resolved
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider/buffer_v1.go
Show resolved
Hide resolved
e72f895
to
35f9597
Compare
35f9597
to
918e748
Compare
0f0bc07
to
52fd633
Compare
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider/buffer_v1.go
Show resolved
Hide resolved
96ffbca
to
6586579
Compare
6586579
to
aba10a7
Compare
aba10a7
to
6966c69
Compare
6966c69
to
dad9f04
Compare
eca29ee
to
58c2620
Compare
fabeb63
to
6ca54f5
Compare
6ca54f5
to
2af72ce
Compare
…e operation is atomic
Track enqueued logs Add tests
6c0f550
to
65eb2f4
Compare
Quality Gate passedIssues Measures |
This PR is the complete changeset introduced across the following three PRs:
Additional changes have been made on top of these three base PRs, but to summarise:
Testing
Unit testing
Multiple unit tests have been added around the provider, buffer, and dequeue coordinator. The provider tests cover multiple scenarios regarding block progression, how dequeues are distributed across upkeeps and within block windows, as well as reorgs and changes in the number of upkeeps.
Happy path tests (AUTO-9176)