Skip to content
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

Improve Relay after Persisting blocks (relay last 2 instead of first 1) #458

Merged
merged 2 commits into from Nov 9, 2018

Conversation

jsolman
Copy link
Contributor

@jsolman jsolman commented Nov 9, 2018

Had a discussion with @vncoelho about the code that happens on OnNewBlock and during the discussion I noticed something that looked sub-optimal related to how the code determines which blocks to relay upon persist.

The code was only relaying the first block persisted in the case that there are multiple blocks also being persisted from the block cache. Since the idea is to keep nodes on the chain aware of the the most recent blocks, it should be relaying the last block rather than the first one. The code change I've made here will relay the last 2 blocks when multiple were persisted at once by taking some from the block cache.

@vncoelho indicated he would be able to do some testing with this, and I'll give it some testing also and update notes here after.

I believe this change will help nodes to obtain the latest blocks more quickly.

@jsolman jsolman changed the title Upon peristing multiple blocks, relay last 2 instead of first 1. Improve Relay after Persisting blocks (relay last 2 instead of first 1) Nov 9, 2018
@erikzhang erikzhang merged commit bbb0c72 into master Nov 9, 2018
@erikzhang erikzhang deleted the ImproveRelayOnPersist branch November 9, 2018 08:27
@jsolman
Copy link
Contributor Author

jsolman commented Nov 9, 2018

I did some testing on this already and it looks good so far.

@vncoelho
Copy link
Member

vncoelho commented Nov 9, 2018

Thanks, @jsolman! You are genius.
It is great to talk to you in person.
I already tested yesterday as well.
We merged it inside the new Consensus proposal, thus we will be often testing this during the next days.
Let's go for the next step 🗡️ : TaskManager request blocks

Thacryba pushed a commit to simplitech/neo that referenced this pull request Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants