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

Track when we enqueue logs for the same upkeep on the same block more than once #13176

Merged
merged 2 commits into from
Jun 4, 2024

Conversation

ferglor
Copy link
Collaborator

@ferglor ferglor commented May 10, 2024

https://smartcontract-it.atlassian.net/browse/AUTO-10164

WIth this PR, we're updating some comments within the buffer implementation, to explicitly declare our assumptions on how the buffer will be used.

Further to this, we're also adding logs to identify scenarios when these assumptions are violated. To achieve this, we use a map to track the number of times we enqueue upkeep logs for a particular block number.

Testing

Running the load test, we don't see the logs appear for enqueuing upkeep logs for the same block number more than once.

if lastBlockSeen := b.lastBlockSeen.Load(); lastBlockSeen < latestLogBlock {
b.lastBlockSeen.Store(latestLogBlock)
} else if latestLogBlock < lastBlockSeen {
b.lggr.Debugw("enqueuing logs from a block older than latest seen block", "logBlock", latestLogBlock, "lastBlockSeen", lastBlockSeen)
Copy link
Contributor

@infiloop2 infiloop2 May 28, 2024

Choose a reason for hiding this comment

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

nit. enqueuing logs with a latest block older older than latest seen block

infiloop2
infiloop2 previously approved these changes May 28, 2024
@cl-sonarqube-production
Copy link

infiloop2
infiloop2 previously approved these changes Jun 3, 2024
@ferglor ferglor enabled auto-merge June 3, 2024 23:13
@ferglor ferglor force-pushed the feature/AUTO-10164-rework branch from 5980d0d to 449c76f Compare June 3, 2024 23:14
amirylm
amirylm previously approved these changes Jun 4, 2024
@ferglor ferglor dismissed stale reviews from amirylm and infiloop2 via ee685d5 June 4, 2024 12:55
Comment on lines +147 to +149
if uid == nil {
return
}
Copy link
Contributor

Choose a reason for hiding this comment

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

curious did we run into this issue? do we have codepaths that call this with nil upkeepID?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In unit testing, passing a nil uid led to "nil" strings in the map, which I didn't want

@ferglor ferglor added this pull request to the merge queue Jun 4, 2024
Merged via the queue into develop with commit d2cd916 Jun 4, 2024
108 checks passed
@ferglor ferglor deleted the feature/AUTO-10164-rework branch June 4, 2024 15:33
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