Allow customizing BackupPollerDelayBlock or disabling Backup LogPoller entirely for tests
#11850
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
reductionista
requested review from
connorwstein,
samsondav and
prashantkumar1982
as code owners
|
I see that you haven't updated any CHANGELOG files. Would it make sense to do so? |
reductionista
force-pushed
the
BCF-2835-disable-backup-lp
branch
from
8f9195f to
d37454f
Compare
reductionista
requested review from
bolekk,
justinkaseman,
KuphJr and
a team
as code owners
samsondav
previously approved these changes
Jan 23, 2024
reductionista
force-pushed
the
BCF-2835-disable-backup-lp
branch
from
fee6632 to
1c8abc7
Compare
reductionista
force-pushed
the
BCF-2835-disable-backup-lp
branch
from
62ccf21 to
4c463e8
Compare
reductionista
force-pushed
the
BCF-2835-disable-backup-lp
branch
from
4c463e8 to
7fd561e
Compare
reductionista
requested review from
a team,
sdrug and
martin-cll
as code owners
reductionista
force-pushed
the
BCF-2835-disable-backup-lp
branch
from
d2149cc to
041cf6f
Compare
Tofel
reviewed
Feb 26, 2024
core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider/integration_test.go
Outdated
Show resolved
Hide resolved
Tofel
reviewed
Feb 26, 2024
reductionista
force-pushed
the
BCF-2835-disable-backup-lp
branch
from
5f9d673 to
eb4ee40
Compare
reductionista
force-pushed
the
BCF-2835-disable-backup-lp
branch
from
eb4ee40 to
e168be1
Compare
This failing chainreader test had FinalityDepth set to 0, so LogPoller was starting after the block where the logs were emitted. It was only passing due to Backup LogPoller. With Backup LogPoller disabled, we have to increase the finality depth so that LogPoller will start a few blocks back (at the first unfinalized block). The failing logpoller test was looking for an extra log entry emitted by BackupLogPoller. Now we can run it with BackupLogPoller disabled (leading to a more robust test) and stop looking for that
reductionista
force-pushed
the
BCF-2835-disable-backup-lp
branch
from
b1b2aa8 to
78913bd
Compare
|
Tofel
reviewed
Feb 28, 2024
Tofel
approved these changes
Feb 28, 2024
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.
Previously,
backupPollerBlockDelaywas defined as a const in LogPoller and hard-coded to 100. Now, any value can be passed to the LogPoller constructor including 0 to disable Backup LogPoller entirely.Also, the number of params passed to
NewLogPoller()has been gradually getting out of control, so this is a good opportunity to move them into alogpoller.Optsstruct. This simplifies many of the LogPoller test setups, allowing for the omission of default parameters and less duplication of codeThe main motivation for this: some fairly sophisticated scale tests have been added for LogPoller under
integration-tests, but presently they can give false negatives due to Backup Poller. (ie, most failures of regular LogPoller would go unnoticed because Backup LogPoller will automatically fix things as an emergency fallback. These tests don't look at timing issues, only correctness, so they cannot distinguish between a temporary failure for a short period and no failure). The only way we can obtain reliable results for these tests presently is to temporarily comment out the part that calls BackupPoller in the code. This adds a way to disable it in a test, so we can run them in an automated way and trust the results.