Skip to content

Commit

Permalink
Fix clearing queued blocks in the sync module (paritytech#11763)
Browse files Browse the repository at this point in the history
  • Loading branch information
arkpar authored and ark0f committed Feb 27, 2023
1 parent 5fe9e3d commit db10a2b
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 30 deletions.
21 changes: 8 additions & 13 deletions client/network/sync/src/blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

use crate::message;
use libp2p::PeerId;
use log::{debug, trace};
use log::trace;
use sp_runtime::traits::{Block as BlockT, NumberFor, One};
use std::{
cmp,
Expand Down Expand Up @@ -212,18 +212,13 @@ impl<B: BlockT> BlockCollection<B> {
}

pub fn clear_queued(&mut self, from_hash: &B::Hash) {
match self.queued_blocks.remove(from_hash) {
None => {
debug!(target: "sync", "Can't clear unknown queued blocks from {:?}", from_hash);
},
Some((from, to)) => {
let mut block_num = from;
while block_num < to {
self.blocks.remove(&block_num);
block_num += One::one();
}
trace!(target: "sync", "Cleared blocks from {:?} to {:?}", from, to);
},
if let Some((from, to)) = self.queued_blocks.remove(from_hash) {
let mut block_num = from;
while block_num < to {
self.blocks.remove(&block_num);
block_num += One::one();
}
trace!(target: "sync", "Cleared blocks from {:?} to {:?}", from, to);
}
}

Expand Down
43 changes: 26 additions & 17 deletions client/network/sync/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1520,9 +1520,7 @@ where
let mut has_error = false;
for (_, hash) in &results {
self.queue_blocks.remove(hash);
}
if let Some(from_hash) = results.first().map(|(_, h)| h) {
self.blocks.clear_queued(from_hash);
self.blocks.clear_queued(hash);
}
for (result, hash) in results {
if has_error {
Expand Down Expand Up @@ -3299,23 +3297,34 @@ mod test {
);
}

let mut notify_imported: Vec<_> = resp_blocks
.iter()
.rev()
.map(|b| {
(
Ok(BlockImportStatus::ImportedUnknown(
b.header().number().clone(),
Default::default(),
Some(peer_id1.clone()),
)),
b.hash(),
)
})
.collect();

// The import queue may send notifications in batches of varying size. So we simulate
// this here by splitting the batch into 2 notifications.
let second_batch = notify_imported.split_off(notify_imported.len() / 2);
let _ = sync.on_blocks_processed(
MAX_BLOCKS_TO_REQUEST as usize,
MAX_BLOCKS_TO_REQUEST as usize,
resp_blocks
.iter()
.rev()
.map(|b| {
(
Ok(BlockImportStatus::ImportedUnknown(
b.header().number().clone(),
Default::default(),
Some(peer_id1.clone()),
)),
b.hash(),
)
})
.collect(),
notify_imported,
);

let _ = sync.on_blocks_processed(
MAX_BLOCKS_TO_REQUEST as usize,
MAX_BLOCKS_TO_REQUEST as usize,
second_batch,
);

resp_blocks
Expand Down

0 comments on commit db10a2b

Please sign in to comment.