Skip to content
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.

Sign up for GitHub

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

Fix offset calculation bugs in ObjectFetcher #3239

Merged
merged 2 commits into from
Nov 19, 2024
Merged

Fix offset calculation bugs in ObjectFetcher #3239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a47e2f
Add PieceIndex::source_position()
teor2345 Nov 19, 2024
428e221
Fix offset calculation bugs in ObjectFetcher
teor2345 Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions crates/subspace-core-primitives/src/pieces.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,14 @@ impl PieceIndex {
(self.0 % ArchivedHistorySegment::NUM_PIECES as u64) as u32
}

/// Position of a source piece in the source pieces for a segment.
/// Panics if the piece is not a source piece.
#[inline]
pub const fn source_position(&self) -> u32 {
assert!(self.is_source());
self.position() / (Self::source_ratio() as u32)
}

/// Is this piece index a source piece?
#[inline]
pub const fn is_source(&self) -> bool {
Expand Down
10 changes: 7 additions & 3 deletions shared/subspace-data-retrieval/src/object_fetcher.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ impl ObjectFetcher {
//
// The last 2 bytes might contain padding if a piece is the last piece in the segment.
let before_last_two_bytes = piece_offset as usize <= RawRecord::SIZE - 1 - 2;
let piece_position_in_segment = piece_index.position();
let piece_position_in_segment = piece_index.source_position();
let data_shards = RecordedHistorySegment::NUM_RAW_RECORDS as u32;
let last_data_piece_in_segment = piece_position_in_segment >= data_shards - 1;

Expand All @@ -261,7 +261,11 @@ impl ObjectFetcher {
//
// The last 2 bytes might contain padding if a piece is the last piece in the segment.
let bytes_available_in_segment =
(data_shards - piece_position_in_segment) * RawRecord::SIZE as u32 - piece_offset - 2;
(data_shards - piece_position_in_segment) * RawRecord::SIZE as u32 - piece_offset;
let Some(bytes_available_in_segment) = bytes_available_in_segment.checked_sub(2) else {
// We need to reconstruct the full segment and discard padding before reading the length.
return Ok(None);
};

// Data from pieces that were already read, starting with piece at index `piece_index`
let mut read_records_data = Vec::<u8>::with_capacity(RawRecord::SIZE * 2);
Expand Down Expand Up @@ -374,7 +378,7 @@ impl ObjectFetcher {
piece_offset: u32,
) -> Result<Vec<u8>, Error> {
let mut segment_index = piece_index.segment_index();
let piece_position_in_segment = piece_index.position();
let piece_position_in_segment = piece_index.source_position();
// Used to access the data after it is converted to raw bytes
let offset_in_segment =
piece_position_in_segment as usize * RawRecord::SIZE + piece_offset as usize;
Expand Down
Loading