netsync: Track peer for requested blocks. #3444
Conversation
This reworks the net sync manager block announcement handling to keep track of the best known block announced by each peer as determined by having the most cumulative proof of work. The primary motivation is to provide a relatively efficient mechanism to discover which blocks are available to download from each peer to eventually support downloading multiple blocks in parallel. It also doubles to increase robustness of best height reporting of each peer once the initial headers sync process is complete since the values are only updated when the header has more cumulative proof of work versus simply having a larger height since, although exceedingly rare in practice, it is possible for a chain with fewer blocks to have more cumulative work.
This modifies the map that tracks requests for blocks to keep track of which peer the block was requested from. This is currently not used since blocks are only downloaded from a single sync peer, but it helps pave the way to handling downloading blocks from multiple peers in parallel.
davecgh
force-pushed
the
netsync_track_requested_from_peer
branch
from
c860387 to
d2e603b
Compare
| continue | ||
| } | ||
|
|
||
| // Stop requesting when the request would exceed the max size of the | ||
| // map used to track requests. | ||
| if len(m.requestedBlocks)+1 > maxRequestedBlocks { |
There was a problem hiding this comment.
Just highlighting that this is a change in behavior. Previously limitAdd would enforce limits onto m.requestedBlocks and peer.requestedBlocks separately, but with the new code peer.requestedBlocks is not being limited.
There was a problem hiding this comment.
Indeed. I probably should've called this out in the description, but I changed it because the two maps really need to stay in sync. Otherwise, peers can be hit with misbehavior if expected entries are not in one map or the other.
It's currently not an issue because it's impossible to hit the limiting cases of limitAdd since there is a single sync peer and there are never more outstanding requests than maxInFlightBlocks = 16.
|
Seperately from this PR, e.g. Opened #3445 to look at this further. |
Discussed in the other PR. |
This is rebased on #3443.
This modifies the map that tracks requests for blocks to keep track of which peer the block was requested from. This is currently not used since blocks are only downloaded from a single sync peer, but it helps pave the way to handling downloading blocks from multiple peers in parallel.
This is work towards #1145.