Optimize Ping message #1903
Optimize Ping message #1903
Conversation
|
@Qiao-Jin What do you think? |
|
I think this looks fine |
|
But I think we shouldn't relay the unverified blocks to other nodes. |
|
I think Remove Ping message is ok.
We don't seem to relay unverified blocks. |
|
In this PR, we send ping messages in |
|
The Ping message tell the |
|
We can add the last sent height in session in order to avoid duplicates |
|
We can send ping messages after |
|
Please, could you check it again? |
| @@ -359,6 +357,8 @@ private VerifyResult OnNewBlock(Block block) | |||
| Self.Tell(unverifiedBlock, ActorRefs.NoSender); | |||
There was a problem hiding this comment.
This is not parallel, right?
In this sense, we were telling heights that were cached?
There was a problem hiding this comment.
previously was before persists, now after it
|
@Qiao-Jin Review. |
|
I think this PR is OK. However, I think maybe we can remove the |
|
@shargon, take a look at the last commit, since |
|
@vncoelho yes, i think that it's a good change! |
src/neo/Network/P2P/RemoteNode.cs
Outdated
| if (msg.Command == MessageCommand.Ping || msg.Command == MessageCommand.Pong) | ||
| { | ||
| PingPayload payload = (PingPayload)msg.Payload; | ||
| if (msg.Command == MessageCommand.Ping && LastHeightSent >= payload.LastBlockIndex) break; |
There was a problem hiding this comment.
What do you think @neo-project/core @Qiao-Jin:
- Avoid duplicate pings and pong set
LastHeightSentto avoid duplicate pings
There was a problem hiding this comment.
Maybe it could be something like this:
PingPayload payload = (PingPayload)msg.Payload;
aux = LastHeightSent;
if (LastHeightSent < payload.LastBlockIndex) LastHeightSent = payload.LastBlockIndex;
if (aux >= payload.LastBlockIndex) break;
|
I am not sure, @erikzhang reverted the pong, yes? Why not to use it to update the |
|
@vncoelho ping and pong use PingPayload, is not reverted, only optimized |
Related to #1844 (comment)