-
Notifications
You must be signed in to change notification settings - Fork 805
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
[Merged by Bors] - Merge Engines and Engine struct in one in the execution_layer
crate
#3284
Closed
divagant-martian
wants to merge
1
commit into
sigp:unstable
from
divagant-martian:finish-fallback-removal-v2
Closed
[Merged by Bors] - Merge Engines and Engine struct in one in the execution_layer
crate
#3284
divagant-martian
wants to merge
1
commit into
sigp:unstable
from
divagant-martian:finish-fallback-removal-v2
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
cbb855d
to
b52743c
Compare
048c7d6
to
85ea052
Compare
paulhauner
approved these changes
Jul 10, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Jul 11, 2022
…#3284) ## Issue Addressed Part of #3118, continuation of #3257 and #3283 ## Proposed Changes - Merge the [`Engines`](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L161-L165) struct and [`Engine` ](https://github.com/sigp/lighthouse/blob/9c429d0764ed91cf56efb8a47a35a556b54a86a4/beacon_node/execution_layer/src/engines.rs#L62-L67) - Remove unnecessary generics ## Additional Info There is more cleanup to do that will come in subsequent PRs
execution_layer
crateexecution_layer
crate
bors bot
pushed a commit
that referenced
this pull request
Jul 13, 2022
## Issue Addressed Resolves #3176 ## Proposed Changes Continues from PRs by @divagant-martian to gradually remove EL redundancy (see #3284, #3257). This PR achieves: - Removes the `broadcast` and `first_success` methods. The functional impact is that every request to the EE will always be tried immediately, regardless of the cached `EngineState` (this resolves #3176). Previously we would check the engine state before issuing requests, this doesn't make sense in a single-EE world; there's only one EE so we might as well try it for every request. - Runs the upcheck/watchdog routine once per slot rather than thrice. When we had multiple EEs frequent polling was useful to try and detect when the primary EE had come back online and we could switch to it. That's not as relevant now. - Always creates logs in the `Engines::upcheck` function. Previously we would mute some logs since they could get really noisy when one EE was down but others were functioning fine. Now we only have one EE and are upcheck-ing it less, it makes sense to always produce logs. This PR purposefully does not achieve: - Updating all occurances of "engines" to "engine". I'm trying to keep the diff small and manageable. We can come back for this. ## Additional Info NA
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
low-hanging-fruit
Easy to resolve, get it before someone else does!
ready-for-merge
This PR is ready to merge.
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.
Issue Addressed
Part of #3118, continuation of #3257 and #3283
Proposed Changes
Engines
struct andEngine
Additional Info
There is more cleanup to do that will come in subsequent PRs