Skip to content
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

Dequeue minimum commitment logs for all upkeeps as a priority #13653

Merged
merged 27 commits into from
Jul 5, 2024

Conversation

ferglor
Copy link
Collaborator

@ferglor ferglor commented Jun 21, 2024

In this PR, we're modifying the log provider to dequeue logs in two distinct phases:

  • Minimum commitment dequeue (while we have upkeeps who have not had the minimum number of logs dequeued)
  • Best effort dequeue (when all upkeeps have had minimum logs dequeued)

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.

@ferglor ferglor requested a review from a team as a code owner July 1, 2024 18:27
- Use a default log limit of 1
- Simplify dequeue interface
infiloop2
infiloop2 previously approved these changes Jul 4, 2024

if uid == nil {
return
if len(reorgBlocks) > 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be cleaner to do this check+eviction outside of this function (should just get stats)

@cl-sonarqube-production
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants