Fix minor issues to speed up block processing logic #1145
Merged
Conversation
This file contains hidden or 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
diamondhands0
force-pushed
the
diamondhands/INCREASE-LOGGING-ON-BLOCK-TIP-ERROR
branch
from
1fb8281 to
77c73ab
Compare
* Fix a bug whereby block production would not start until the first call to SubmitBlock, which resulted in erratic miner behavior. Specifically, all calls to get-block-template would fail until the first call to submit-block. Normally in the steady-state this is not an issue, but during a reboot it can become a "chicken and egg" where miners need get-block-template to generate a valid block, but get-block-template won't return until they actually produce a block (which they can't do without a template). * Reduce the default block size used by miners to 500kb, down from 2mb. This ends up giving better performance during periods of high congestion because smaller blocks are processed more quickly. With very large blocks, we were encountering an issue whereby a block would take a minute or two to fuly validate, causing erratic behavior as the block propagated through the network. * Add a rest period during initial transaction download that allows for other processes to grab the ChainLock if needed. This was the least obviously-needed of the fixes, but it doesn't hurt to leave it in, and could prevent deadlock-related issues during periods of high congestion. * Add more useful logging in various places.
diamondhands0
force-pushed
the
diamondhands/INCREASE-LOGGING-ON-BLOCK-TIP-ERROR
branch
from
77c73ab to
875d276
Compare
diamondhands0
commented
Mar 25, 2024
|
|
||
| for { | ||
| if atomic.LoadInt32(&desoBlockProducer.exit) >= 0 { | ||
| if atomic.LoadInt32(&desoBlockProducer.exit) > 0 { |
Member
Author
There was a problem hiding this comment.
This was causing a lot of the erratic behavior when nodes would restart. See main PR description for details.
diamondhands0
commented
Mar 25, 2024
| // spam blocks in the event someone tries to abuse the initially low min | ||
| // fee rates. | ||
| MinerMaxBlockSizeBytes: 2000000, | ||
| MinerMaxBlockSizeBytes: 500000, |
Member
Author
There was a problem hiding this comment.
Note that this does not reduce the maximum block size permitted by the network, only the default block size used by miners when generating block templates.
diamondhands0
commented
Mar 25, 2024
| // five seconds is the right amount. We added it during a mission-critical sprint | ||
| // to find and fix slow block production issue as one of several patches. It clearly doesn't | ||
| // hurt so we decided to leave it in for now. | ||
| if (ii+1)%1000 == 0 { |
Member
Author
There was a problem hiding this comment.
Early on, I suspected that the reason get-block-template wasn't working was because of a ChainLock starvation caused by downloading a lot of transactions. Ultimately, it wasn't that, but giving a breather for other processes to acquire the ChainLock seemed like a fine thing to keep, and doesn't slow down sync too much.
lazynina
approved these changes
Mar 25, 2024
| return | ||
| } | ||
|
|
||
| // Set the time to a nil value so we run on the first iteration of the loop. |
Member
There was a problem hiding this comment.
Let's update this comment since time is no longer nil.
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.
Fix a bug whereby block production would not start until the first call to SubmitBlock, which resulted in erratic miner behavior. Specifically, all calls to get-block-template would fail until the first call to submit-block. Normally in the steady-state this is not an issue, but during a reboot it can become a "chicken and egg" where miners need get-block-template to generate a valid block, but get-block-template won't return until they actually produce a block (which they can't do without a template).
Reduce the default block size used by miners to 500kb, down from 2mb. This ends up giving better performance during periods of high congestion because smaller blocks are processed more quickly. With very large blocks, we were encountering an issue whereby a block would take a minute or two to fuly validate, causing erratic behavior as the block propagated through the network.
Add a rest period during initial transaction download that allows for other processes to grab the ChainLock if needed. This was the least obviously-needed of the fixes, but it doesn't hurt to leave it in, and could prevent deadlock-related issues during periods of high congestion.
Add more useful logging in various places.