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

[neo3] Add Ping expiration #1829

Merged
merged 6 commits into from
Aug 12, 2020
Merged

Conversation

shargon
Copy link
Member

@shargon shargon commented Aug 6, 2020

Related to #1806
Related to #1812

@erikzhang
Copy link
Member

Send Ping every minute?

@shargon
Copy link
Member Author

shargon commented Aug 10, 2020

Send Ping every minute?

We can update the values also when we receive a block

@roman-khimov
Copy link
Contributor

Just as a side note, the way we ping in neo-go is we're setting a node-wide timer for PingInterval (which by default is set twice SecondsPerBlock), when it fires we're checking if there were any new blocks added to the chain since previous timer expiration. If the chain has progressed, we're not doing anything. If not, then we're sending pings to all of our neighbors.

The rationale for this behavior is:

  • pinging to check the connection doesn't improve things much as dead connection is usually quickly detected by broadcasts that happen quite frequently
  • if we're receiving new blocks we shouldn't care much about the other node's height
  • if we're not receiving blocks for some time, that's when we should worry a little and try to check with our peers if we're not missing anything
  • per-peer timers don't make much sense with this usage pattern as the primary driver for ping requests here is the chain state
  • pings are being sent to all peers for the same reason, we either need to confirm that everyone is at the same height with us and it's just the block being late, or we need to find some peer that's more up to date

erikzhang
erikzhang previously approved these changes Aug 11, 2020
@shargon
Copy link
Member Author

shargon commented Aug 11, 2020

@erikzhang please take a look again, we can reduce the ping intervals when we receive a block.

@shargon shargon requested a review from erikzhang August 11, 2020 09:28
@shargon shargon merged commit ca13358 into neo-project:master Aug 12, 2020
@shargon shargon deleted the ping-pong-expiration branch August 12, 2020 08:39
cloud8little pushed a commit to cloud8little/neo that referenced this pull request Jan 24, 2021
* PingPong expiration

* ms

* Change to TimeProvider.Current.UtcNow

* Optimize
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.

4 participants