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

Log provider edge cases - combined changes #13527

Closed
wants to merge 10 commits into from

Conversation

ferglor
Copy link
Collaborator

@ferglor ferglor commented Jun 12, 2024

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:

  • We are now calculating the number of iterations needed to dequeue logs for all upkeeps in the buffer, and we dequeue a different set of upkeeps in every iteration
  • We're also using a new dequeue coordinator to determine which block window should be dequeued from. The dequeue coordinator operates as follows:
    • Dequeue the windows for which we have not yet dequeued the minimum number of guaranteed logs for, going from the oldest window to the newest until all windows have had the minimum number of guaranteed logs dequeued
    • Dequeue the remaining logs from all windows as best effort, going from the oldest window to the newest, until all windows are completely dequeued
  • When a reorg has been identified, the dequeue coordinator gets reset for the affected block windows

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)

  • AUTO-8216 - 0.3% missed events vs 0.1% missed events in develop
  • AUTO-8219 - 84% missed events vs 95% missed events in develop
  • AUTO-8220 - 92% missed events vs 95% missed events in develop

@@ -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...)
Copy link
Contributor

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

Copy link
Contributor

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

amirylm
amirylm previously approved these changes Jun 20, 2024
@cl-sonarqube-production
Copy link

@ferglor ferglor closed this Jul 2, 2024
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