Skip to content

Commit 22ea4bd

Browse files
authored
A0-4616: Don't request blocks you started iimporting (#1950) (#1951)
# Description By optimistically assuming that a block that we started importing will get imported, we treat it as already imported for purposes of sending requests. This should limit both the amount of received blocks, as well as the amount of blocks in the import queue, both of which were slowing sync down. ## Type of change - Bug fix (non-breaking change which fixes an issue) - New feature (non-breaking change which adds functionality) # Checklist: <!-- delete when not applicable to your PR --> - I have added tests - I have made corresponding changes to the existing documentation - I have created new documentation
1 parent 6dbe2b0 commit 22ea4bd

File tree

5 files changed

+303
-108
lines changed

5 files changed

+303
-108
lines changed

finality-aleph/src/sync/forest/mod.rs

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ enum VertexHandle<'a, I: PeerId, J: Justification> {
4545
pub enum Interest<H: Header, I: PeerId> {
4646
/// We are not interested in requesting this branch.
4747
Uninterested,
48+
/// We might be interested in requesting this later.
49+
MaybeLater,
4850
/// We would like to have this branch.
4951
Required {
5052
header: H,
@@ -555,43 +557,44 @@ where
555557

556558
/// Prepare additional info required to create a request for the branch.
557559
/// Returns `None` if we're not interested in the branch.
558-
/// Can be forced to fake interest, but only for blocks we have headers for.
559-
fn prepare_request_info(
560-
&self,
561-
id: &BlockId,
562-
force: bool,
563-
) -> Option<(J::Header, HashSet<I>, BranchKnowledge)> {
560+
/// Can be forced to fake interest, but only for blocks we have headers for, that are not
561+
/// importing.
562+
fn prepare_request_info(&self, id: &BlockId, force: bool) -> Interest<J::Header, I> {
563+
use Interest::*;
564564
use VertexHandle::Candidate;
565565
match self.get(id) {
566566
Candidate(vertex) => {
567+
// if we are currently importing this vertex don't send a request, but don't drop
568+
// the task in case there is an error
569+
if vertex.vertex.importing() {
570+
return MaybeLater;
571+
}
567572
// request only requestable blocks, unless forced
568573
if !(force || vertex.vertex.requestable()) {
569-
return None;
574+
return Uninterested;
570575
}
571576
let header = match vertex.vertex.header() {
572577
Some(header) => header,
573-
None => return None,
578+
None => return Uninterested,
574579
};
575580
let know_most = vertex.vertex.know_most();
576581
// should always return Some, as the branch of a Candidate always exists
577582
self.branch_knowledge(id.clone())
578-
.map(|branch_knowledge| (header, know_most, branch_knowledge))
583+
.map(|branch_knowledge| Required {
584+
header,
585+
know_most,
586+
branch_knowledge,
587+
})
588+
.unwrap_or(Uninterested)
579589
}
580590
// request only Candidates
581-
_ => None,
591+
_ => Uninterested,
582592
}
583593
}
584594

585595
/// How much interest we have for requesting the block.
586596
pub fn request_interest(&self, id: &BlockId) -> Interest<J::Header, I> {
587-
match self.prepare_request_info(id, false) {
588-
Some((header, know_most, branch_knowledge)) => Interest::Required {
589-
header,
590-
know_most,
591-
branch_knowledge,
592-
},
593-
None => Interest::Uninterested,
594-
}
597+
self.prepare_request_info(id, false)
595598
}
596599

597600
/// Whether we would like to eventually import this block.
@@ -605,6 +608,13 @@ where
605608
}
606609
}
607610

611+
pub fn start_import(&mut self, id: &BlockId) {
612+
use VertexHandleMut::Candidate;
613+
if let Candidate(mut vertex) = self.get_mut(id) {
614+
vertex.get_mut().vertex.start_import();
615+
}
616+
}
617+
608618
fn know_most(&self, id: &BlockId) -> HashSet<I> {
609619
match self.get(id) {
610620
VertexHandle::Candidate(vertex) => vertex.vertex.know_most(),
@@ -628,8 +638,11 @@ where
628638
use VertexHandle::*;
629639
if self.behind_finalization() > 0 {
630640
// This should always happen, but if it doesn't falling back to other forms of extension requests is acceptable.
631-
if let Some((_, know_most, branch_knowledge)) =
632-
self.prepare_request_info(&self.highest_justified.id(), true)
641+
if let Interest::Required {
642+
know_most,
643+
branch_knowledge,
644+
..
645+
} = self.prepare_request_info(&self.highest_justified.id(), true)
633646
{
634647
return HighestJustified {
635648
header: self.highest_justified.clone(),

0 commit comments

Comments
 (0)