Add logs to identify when assumptions of log queuing behaviour are violated #12846
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
infiloop2
reviewed
May 2, 2024
|
|
||
| // clean up enqueued block counts | ||
| for block := range b.enqueuedBlocks { | ||
| if block < blockThreshold-reorgBuffer { |
There was a problem hiding this comment.
not fully sure why we need reorgBuffer here?
i think the other functions in buffer simply work on blockThreshold-lookback to blockThreshold range, reorg handling is done at the provider level
infiloop2
reviewed
May 2, 2024
Comment on lines
+139
to
+140
| blockNumbers[uid.String()] = upkeepBlockInstances + 1 | ||
| b.lggr.Debugw("enqueuing logs again for a previously seen block for this upkeep", "blockNumber", blockNumber, "numberOfEnqueues", b.enqueuedBlocks[blockNumber], "upkeepID", uid.String()) |
There was a problem hiding this comment.
i think this logic will change with reorg handling?
There was a problem hiding this comment.
Yes; based on what I have in the spike for reorgs, we should have a set of block numbers available to us to perform this check against 👍
amirylm
reviewed
May 3, 2024
| @@ -50,6 +51,96 @@ func TestLogEventBufferV1_SyncFilters(t *testing.T) { | |||
| require.Equal(t, 1, buf.NumOfUpkeeps()) | |||
| } | |||
|
|
|||
| type readableLogger struct { | |||
There was a problem hiding this comment.
Just for the purposes of catching and validating log messages within the tests
|
amirylm
approved these changes
May 7, 2024
FelixFan1992
approved these changes
May 9, 2024
akuzni2
pushed a commit
that referenced
this pull request
May 11, 2024
…olated (#12846) * Add logs to identify when assumptions of log queuing behaviour are violated * Add tests * go import * Add changeset * Update enqueuing assumption * Update tests * Extract block tracking into a separate function * Clean up outdated enqueued blocks * Clean up imports * Ignore reord buffer in cleanup * Cleanup test name
github-merge-queue bot
pushed a commit
that referenced
this pull request
May 11, 2024
* Fix metric description on mercury_transmit_queue_load * Add changeset * Add changesets tag * Decouple gas tests (#12972) * Add first version of evm utils * Remove unused context util * Add WSServer tests * Add NewLegacyTransaction test * Update NewTestChainScopedConfig to apply correct defaults * Decouple gas package tests from core * Move testutils * Update paths * Fix import error * Add changeset * Add logs to identify when assumptions of log queuing behaviour are violated (#12846) * Add logs to identify when assumptions of log queuing behaviour are violated * Add tests * go import * Add changeset * Update enqueuing assumption * Update tests * Extract block tracking into a separate function * Clean up outdated enqueued blocks * Clean up imports * Ignore reord buffer in cleanup * Cleanup test name * tools/docker: bump postgres to v14 (#13156) * changed spammy error log to debug (#13153) * [KS-186] Add custom OCR3 Capability Provider (#13137) * [KS-193] Pass MercuryTriggerService to Mercury Transmitter (#13118) 1. Add EnableTriggerCapability flag to Relay config 2. Create MercuryTriggerService lazily, on the first call to NewMercuryProvider() 3. Make it available in the Transmitter (no-op for now) * update changeset to include db_update tag (#13170) * fix go-etheruem compatibility pipeline trigger (#13162) * Revert "Add logs to identify when assumptions of log queuing behaviour are violated" (#13173) * Revert "Add logs to identify when assumptions of log queuing behaviour are vi…" This reverts commit 6a342ae. * Add changeset * chore: update solana-build-contracts for node20 (#13175) * chore: update solana-build-contracts for node20 * chore: bump chainlink-solana version * Decouple utils tests from core (#12993) * Add first version of evm utils * Remove unused context util * Add WSServer tests * Add NewLegacyTransaction test * Update NewTestChainScopedConfig to apply correct defaults * Move testutils * Decouple utils tests from core * Add changeset --------- Co-authored-by: Dimitris Grigoriou <dimitris.grigoriou@smartcontract.com> Co-authored-by: ferglor <fergal.gribben@smartcontract.com> Co-authored-by: Jordan Krage <jmank88@gmail.com> Co-authored-by: Patrick <patrick.huie@smartcontract.com> Co-authored-by: Cedric <cedric.cordenier@smartcontract.com> Co-authored-by: Bolek <1416262+bolekk@users.noreply.github.com> Co-authored-by: Chunkai Yang <matYang@users.noreply.github.com> Co-authored-by: Bartek Tofel <bartek.tofel@smartcontract.com> Co-authored-by: Erik Burton <erikburt@users.noreply.github.com>
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.
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.