Skip to content

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

Merged
ferglor merged 2 commits intodevelopfrom
feature/AUTO-10164-rework
Jun 4, 2024
Merged

Track when we enqueue logs for the same upkeep on the same block more than once#13176
ferglor merged 2 commits intodevelopfrom
feature/AUTO-10164-rework

Conversation

@ferglor
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

infiloop2
infiloop2 previously approved these changes Jun 3, 2024
@ferglor ferglor enabled auto-merge June 3, 2024 23:13
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
Copy Markdown
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
Copy Markdown
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
@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