Disable ethash by replacing with ethash.NewFullFake #318
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
jpmsam
reviewed
Mar 29, 2018
eth/backend.go
Outdated
| // For Quorum, Raft run as a separate service, so | ||
| // the Ethereum service still needs a consensus engine, | ||
| // use the consensus with the lightest overhead | ||
| log.Warn("Ethash used in full fake mode") |
There was a problem hiding this comment.
Can you make it the default consensus instead of removing the switch statement completely?
There was a problem hiding this comment.
thanks @jpmsam for the review, makes sense to me, this way the other test/fake flavors of ethash can still be used. Just pushed another commit to address your comment
|
Thanks @jimthematrix , we'll have to do a bit more testing but it seems like this could save geth from allocating memory for ethash unnecessarily. |
…ult so other test/fake flavors can still be used
|
Thanks @jimthematrix , please complete the CLA and email it to quorum_cla@jpmorgan.com |
|
@jpmsam thanks for the information, just sent the signed CLA scan to the email address. |
jpmsam
approved these changes
Apr 19, 2018
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.
Given that the Raft consensus is implemented as a Service rather than a consensus engine, it essentially runs alongside ethhash. Most of the time the fact that ethash is "active" is not an issue, but in some circumstances this can have negative impact on the Geth node's availability. What we have observed is that when the memory allocated to the Geth process is low, generating and regenerating the ethash verification cache can take a long time (several minutes) and as this is happening the node is not able to commit blocks.
This PR is an attempt to disable ethash. I found using Geth's built-in "faked pow" works pretty well. But surely more testing could be done to ensure this doesn't impact how Raft behaves. Note that this change does not impact IBFT.