-
Notifications
You must be signed in to change notification settings - Fork 243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fetch multiple pieces during object reconstruction #3158
base: main
Are you sure you want to change the base?
Changes from all commits
74a30fe
01750d2
170588b
f488e3a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,10 +2,11 @@ | |
|
||
use async_trait::async_trait; | ||
use futures::stream::StreamExt; | ||
use futures::Stream; | ||
use std::fmt; | ||
use std::ops::{Deref, DerefMut}; | ||
use subspace_core_primitives::pieces::{Piece, PieceIndex}; | ||
use subspace_data_retrieval::piece_getter::{BoxError, ObjectPieceGetter}; | ||
use subspace_data_retrieval::piece_getter::ObjectPieceGetter; | ||
use subspace_networking::utils::piece_provider::{PieceProvider, PieceValidator}; | ||
use subspace_networking::Node; | ||
|
||
|
@@ -51,7 +52,7 @@ impl<PV> ObjectPieceGetter for DsnPieceGetter<PV> | |
where | ||
PV: PieceValidator, | ||
{ | ||
async fn get_piece(&self, piece_index: PieceIndex) -> Result<Option<Piece>, BoxError> { | ||
async fn get_piece(&self, piece_index: PieceIndex) -> anyhow::Result<Option<Piece>> { | ||
if let Some((got_piece_index, maybe_piece)) = | ||
self.get_from_cache([piece_index]).await.next().await | ||
{ | ||
|
@@ -64,6 +65,22 @@ where | |
|
||
Ok(None) | ||
} | ||
|
||
async fn get_pieces<'a, PieceIndices>( | ||
&'a self, | ||
piece_indices: PieceIndices, | ||
) -> anyhow::Result< | ||
Box<dyn Stream<Item = (PieceIndex, anyhow::Result<Option<Piece>>)> + Send + Unpin + 'a>, | ||
> | ||
where | ||
PieceIndices: IntoIterator<Item = PieceIndex, IntoIter: Send> + Send + 'a, | ||
{ | ||
Ok(Box::new( | ||
self.get_from_cache(piece_indices) | ||
.await | ||
.map(|(index, maybe_piece)| (index, Ok(maybe_piece))), | ||
Comment on lines
+79
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably worth having a TODO that this is supposed to eventually reach out to archival storage too and do reconstruction if necessary. Cache is what should be used basically all the time, but cold storage fallback is still necessary for completeness. |
||
)) | ||
} | ||
} | ||
|
||
impl<PV> DsnPieceGetter<PV> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,9 +16,8 @@ | |
//! Fetching pieces of the archived history of Subspace Network. | ||
|
||
use crate::object_fetcher::Error; | ||
use crate::piece_getter::{BoxError, ObjectPieceGetter}; | ||
use futures::stream::FuturesOrdered; | ||
use futures::TryStreamExt; | ||
use crate::piece_getter::ObjectPieceGetter; | ||
use futures::{StreamExt, TryStreamExt}; | ||
use subspace_core_primitives::pieces::{Piece, PieceIndex}; | ||
use tracing::{debug, trace}; | ||
|
||
|
@@ -31,7 +30,7 @@ use tracing::{debug, trace}; | |
pub async fn download_pieces<PG>( | ||
piece_indexes: &[PieceIndex], | ||
piece_getter: &PG, | ||
) -> Result<Vec<Piece>, BoxError> | ||
) -> anyhow::Result<Vec<Piece>> | ||
where | ||
PG: ObjectPieceGetter, | ||
{ | ||
|
@@ -42,40 +41,18 @@ where | |
); | ||
|
||
// TODO: | ||
// - consider using a semaphore to limit the number of concurrent requests, like | ||
// download_segment_pieces() | ||
// - if we're close to the number of pieces in a segment, use segment downloading and piece | ||
// reconstruction instead | ||
// Currently most objects are limited to 4 pieces, so this isn't needed yet. | ||
let received_pieces = piece_indexes | ||
.iter() | ||
.map(|piece_index| async move { | ||
match piece_getter.get_piece(*piece_index).await { | ||
Ok(Some(piece)) => { | ||
trace!(?piece_index, "Piece request succeeded",); | ||
Ok(piece) | ||
} | ||
Ok(None) => { | ||
trace!(?piece_index, "Piece not found"); | ||
Err(Error::PieceNotFound { | ||
piece_index: *piece_index, | ||
} | ||
.into()) | ||
} | ||
Err(error) => { | ||
trace!( | ||
%error, | ||
?piece_index, | ||
"Piece request caused an error", | ||
); | ||
Err(error) | ||
} | ||
} | ||
}) | ||
.collect::<FuturesOrdered<_>>(); | ||
let received_pieces = piece_getter | ||
.get_pieces(piece_indexes.iter().copied()) | ||
.await?; | ||
|
||
// We want exact pieces, so any errors are fatal. | ||
let received_pieces: Vec<Piece> = received_pieces.try_collect().await?; | ||
let received_pieces: Vec<Piece> = received_pieces | ||
.map(|(piece_index, maybe_piece)| maybe_piece?.ok_or(Error::PieceNotFound { piece_index })) | ||
.try_collect() | ||
.await?; | ||
Comment on lines
+52
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change is not equivalent to before. The order of returned pieces is arbitrary (hence |
||
|
||
trace!( | ||
count = piece_indexes.len(), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: remove these TODOs, we might not need them