fix: Fix extra GetBlock requests for validators
#27
Conversation
| @@ -139,6 +158,31 @@ impl PeerStates { | |||
| } | |||
| } | |||
|
|
|||
| /// Cancels pending block retrieval for blocks that appear in the storage using other means | |||
| /// (e.g., thanks to the consensus algorithm). This works at best-effort basis; it's not guaranteed | |||
| /// that this method will timely cancel all block retrievals. | |||
There was a problem hiding this comment.
why do we need to compromise here?
There was a problem hiding this comment.
That's the most general approach I've come up with that doesn't make many assumptions as to what could update blocks externally and how these updates would look (e.g., whether it's consensus or some other actor). I guess with the knowledge of consensus we could do marginally better by cancelling not only the latest block produced by consensus, but all the preceding blocks as well; is this what you had in mind? If you have any suggestions, I'm happy to listen.
There was a problem hiding this comment.
I mean you are using watch to signal the received block from external source, which naturally will lead to NOT cancelling some blocks fetching tasks if multiple blocks are received from an external source at the same time. Also AFAICT there is a race condition between spawning a fetching routine for a block and receiving this block from an external source.
|
@brunoffranca WDYM by non-existent blocks? |
|
I misremembered how sync_blocks works. I thought it would always try to fetch block N+1 if we had block N. But you just ask peers for their head number. My question doesn't apply then. |
slowli
deleted the
aov-bft-370-fix-extra-getblock-requests-for-validators
branch
What ❔
If the
SyncBlocksactor runs on a validator node, the node previously produced unnecessaryGetBlockrequests to peers. This is becauseSyncBlockslogic assumed that new blocks are only added within the actor. This PR lifts this assumption; it filtersGetBlockrequests and cancels pending requests reacting to new blocks being put in the storage.Why ❔
Extra
GetBlockrequests waste traffic and processing time of peers.