ETCM-1023: Block fetching and -building flow #1108
Conversation
…o plug it into RegularSync
jvdp
changed the title
src/main/scala/io/iohk/ethereum/blockchain/sync/regular/FetcherService.scala
Outdated
Show resolved
Hide resolved
src/main/scala/io/iohk/ethereum/blockchain/sync/regular/BranchBuffer.scala
Show resolved
Hide resolved
src/main/scala/io/iohk/ethereum/blockchain/sync/regular/RegularSync.scala
Show resolved
Hide resolved
| * @param byParent in-memory buffer of blocks retrievable by parentHash. Only one block per parent is kept, last received wins. | ||
| * @param branchFound branch found from given best branch, as a list of blocks. They are removed from the buffer. | ||
| */ | ||
| case class BranchBuffer(byParent: Map[Hash, Block] = Map.empty, branchFound: Queue[Block] = Queue.empty) { |
There was a problem hiding this comment.
why do we need a branch buffer if we immediately store everything?
There was a problem hiding this comment.
We don't immediately store everything; only rooted blocks / branches, with the reasoning that these blocks have passed the proof of work threshold and so should be relatively safe to store without DoS concerns.
(With the caveat that we should also not store super old blocks; we have been discussing a "settled block" concept that we can also use for this.)
There was a problem hiding this comment.
got it. how much work do you think it takes to turn this into a structure that supports parallel branches? and can the current one be used in any way as is?
There was a problem hiding this comment.
The current one is able to sync Sagano fully, but then might get stuck on a dead branch.
I think I'm close to one that will work for parallel branches for my current ticket (1059.)
| * @param bodiesRequest information for requesting bodies corresponding to newly outstanding headers | ||
| * @param result blocks produced by matching received headers with received bodies | ||
| */ | ||
| case class FetchState( |
There was a problem hiding this comment.
so in the end we still can't get rid of the state? this was one of the goals we defined. We still can do it by using sync processing
There was a problem hiding this comment.
So this isn't a state in the imperative 'shared mutable state' sense; it's kept by the scan operator. This is a more functional approach.
| PeerEventBusActor | ||
| .messageSource( | ||
| peerEventBus, | ||
| PeerEventBusActor.SubscriptionClassifier |
There was a problem hiding this comment.
if you're replacing the PeerRequestHandler here then PeerDisconnectedClassifier is missing
There was a problem hiding this comment.
It's not replacing PeerRequestHandler yet. What's missing is a piece of Flow that manages the sending of headers requests and handling those disconnection and best block messages (and more?)
There was a problem hiding this comment.
but it's gonna replace it in future? just pointing out what should be there if we proceed with this
There was a problem hiding this comment.
I sorta misspoke. It's largely replacing PeerRequestHandler but by replacing BlockFetcher. FastSync (until rewritten) still uses PeerRequestHandler so it's not replacing it in that sense.
jvdp
force-pushed
the
feature/ETCM-1023-fetched-blocks-stream
branch
from
30136c1 to
4890acd
Compare
…fetched-blocks-stream
|
So after some discussion we've decided to abandon the |
nrdxp
force-pushed
the
develop
branch
6 times, most recently
from
22e35b2 to
63187a8
Compare
Implements a flow that construct blocks from messages from the
PeerEventBus, and hooks it intoRegularSyncvia a temporary block buffer data structure and flow (as proof of concept.)[update 23/8]: Just tested this on Sagano and it regular-syncs up to the best block and then remains stuck there. So it's close.