Dequeue minimum commitment logs for all upkeeps as a priority #13653
Conversation
This file contains 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
ferglor
force-pushed
the
feature/dequeue-upkeep-first
branch
from
f69ecac to
9aeb99c
Compare
ferglor
commented
Jul 4, 2024
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider/provider.go
Outdated
Show resolved
Hide resolved
- Use a default log limit of 1 - Simplify dequeue interface
ferglor
force-pushed
the
feature/dequeue-upkeep-first
branch
from
51e1173 to
f9a2037
Compare
infiloop2
reviewed
Jul 4, 2024
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider/buffer_v1.go
Outdated
Show resolved
Hide resolved
infiloop2
reviewed
Jul 4, 2024
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider/buffer_v1.go
Outdated
Show resolved
Hide resolved
infiloop2
reviewed
Jul 4, 2024
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider/factory.go
Outdated
Show resolved
Hide resolved
infiloop2
previously approved these changes
Jul 4, 2024
amirylm
reviewed
Jul 5, 2024
|
|
||
| if uid == nil { | ||
| return | ||
| if len(reorgBlocks) > 0 { |
There was a problem hiding this comment.
Would be cleaner to do this check+eviction outside of this function (should just get stats)
|
infiloop2
approved these changes
Jul 5, 2024
amirylm
approved these changes
Jul 5, 2024
This was referenced Jul 31, 2024
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.
In this PR, we're modifying the log provider to dequeue logs in two distinct phases:
Minimum commitment dequeue
In this phase the log buffer will only dequeue logs for upkeeps who have not had the minimum number of logs dequeued yet, for the current block window.
Best effort dequeue
In this phase the buffer will dequeue as many logs as possible, based on available capacity
Removal of upkeep selector
Earlier versions of this code used an upkeep selector to evenly distribute the number of logs dequeued across upkeeps across a series of dequeue calls. This logic was only applicable to the best effort dequeue, since minimum dequeue focuses on dequeuing logs for upkeeps who have had not had the minimum commitment met for a specific block window. The additional complexity and managed state needed to coordinate the dequeue iterations with the upkeep selector was too complex for what it offered us; in best effort dequeue, we now dequeue logs as and where they are available.