Proper Locking During Block Persistence and Increase Performance while Syncing the Chain #237
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
…cepting a new block.
erikzhang
reviewed
May 16, 2018
neo/Network/LocalNode.cs
Outdated
|
|
||
| // Need to ensure any outstanding calls to Blockchain_PersistCompleted are not in progress. | ||
| // TODO: could add locking instead of using an arbitrarily long sleep here. | ||
| Thread.Sleep(3000); |
There was a problem hiding this comment.
Do you have any plans to improve this place before this PR is merged?
There was a problem hiding this comment.
Yes, I should be able to adjust this to obtain the new PersistLock now. I’ll adjust and test it and update the pull request.
There was a problem hiding this comment.
Should be good now.
…new_tx_event is disposed.
Closed
erikzhang
approved these changes
May 18, 2018
lock9
pushed a commit
to simplitech/neo
that referenced
this pull request
Aug 3, 2018
…e Syncing the Chain (neo-project#237)
Thacryba
pushed a commit
to simplitech/neo
that referenced
this pull request
Feb 17, 2020
gui 2.3.5 need .net Framework 4.7.1
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.
This change insures consistency of the mem_pool upon accepting and persisting a new block and it improves performance of block persistence. Performance is especially improved when syncing the chain in the case that there are a high number number of future transactions hanging around in the mem_pool.
Syncing from a recent chain.acc.zip file has been almost required in the past since syncing could get very slow without this fix if the chain file is rather old. This is because contention on the mem_pool lock from the AddTransactionLoop would block the block persistence thread when LocalNode.Blockchain_PersistCompleted is invoked.
This change will cancel transaction verification each time a new block is received and persisted. I tested syncing the chain, and observed significant performance improvement using these changes. Testing was performed on 2 cores of an Intel(R) Xeon(R) CPU E5-2676 v3 @ 2.40GHz using a disk provisioned at 300 minimum IOPS.
This pull request was originally here (#218), but I'm submitting again freshly from a new branch: