feat(download): optimistic downloader #881
Conversation
rkrasiuk
added
C-enhancement
rkrasiuk
requested review from
rakita,
joshieDo,
onbjerg and
gakonst
as code owners
| #[derive(Clone, Debug)] | ||
| pub(crate) enum DownloadOrigin<T> { | ||
| /// The remote download origin. | ||
| Remote(MessageSender<T>), |
There was a problem hiding this comment.
There are two types of downloads: requested externally and self initiated. this variant contains the sender for the external response channel
There was a problem hiding this comment.
requested externally and self initiated. this variant contains the sender for the external response channel
do we plan/need to call this from multiple places? if so, why?
I thought this downloader is just for the bodies stage
| pub enum DownloaderError { | ||
| /// The downloader is at capacity and cannot process new requests. | ||
| #[error("Downloader is at maximum capacity.")] | ||
| Full, |
There was a problem hiding this comment.
Note here that this should not be an error that reaches the stages as it doesn't matter if the downloader is full, the stages only empty the buffer. Not sure when this error is thrown?
| } | ||
| } | ||
|
|
||
| impl<B, C> BodiesDownload<B, C> { |
There was a problem hiding this comment.
I think technically these futures are up to the client that the downloader has a handle to, right? We don't need retry logic anymore really since we should retry forever, and we should not backoff because we can't know where the request is routed to
| pub(crate) struct DownloadBuffer { | ||
| // TODO: merge two buffers? | ||
| /// Internal header buffer | ||
| headers: LruCache<H256, SealedHeader>, |
There was a problem hiding this comment.
What's the benefit of LruCache vs a Vec with a fixed capacity?
There was a problem hiding this comment.
maybe a LruCache vs a HashMap with a fixed capacity would be a better comparison. it's LRU for now, but I'm still thinking about whether it introduces additional complexity. the benefit is that the headers buffer is used to lookup downloaded headers for both headers and bodies requests
| client: Arc<N>, | ||
| consensus: Arc<C>, | ||
| config: DownloaderConfig, | ||
| request_rx: UnboundedReceiverStream<DownloaderMessage>, |
There was a problem hiding this comment.
why does this take a receiver?
how will this be used inside the stage?
There was a problem hiding this comment.
similar to a NetworkHandle, i'll create a DownloaderHandle which would be able to forward requests to the downloader
Closes #764
Implementation of an optimistic downloader service