From 258f90ef01031fe3f7c42d92405e346caaf21cd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 31 May 2022 10:51:22 +0200 Subject: [PATCH 1/3] Sync: Improve major sync detection When we still have a full import queue, we should still report that we are in major sync mode, otherwise validators may will already start producing blocks. --- client/network/sync/src/lib.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index bc0ed46c3f068..24d1324f96c48 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -578,12 +578,19 @@ where /// Returns the current sync status. pub fn status(&self) -> Status { - let best_seen = self.peers.values().map(|p| p.best_number).max(); + let best_seen = if self.peers.is_empty() { + None + } else { + Some( + self.peers.values().fold(NumberFor::::from(0u32), |c, p| c + p.best_number) / + self.peers.len().saturated_into::().into(), + ) + }; let sync_state = if let Some(n) = best_seen { // A chain is classified as downloading if the provided best block is - // more than `MAJOR_SYNC_BLOCKS` behind the best queued block. - if n > self.best_queued_number && n - self.best_queued_number > MAJOR_SYNC_BLOCKS.into() - { + // more than `MAJOR_SYNC_BLOCKS` behind the best block. + let best_block = self.client.info().best_number; + if n > best_block && n - best_block > MAJOR_SYNC_BLOCKS.into() { SyncState::Downloading } else { SyncState::Idle From 64af1af82a8ecf75e93fc0bdb102f6d33d884b35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 31 May 2022 14:44:25 +0200 Subject: [PATCH 2/3] Use median --- client/network/sync/src/lib.rs | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index 24d1324f96c48..2c4279c0ab1df 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -576,16 +576,27 @@ where .map(|p| PeerInfo { best_hash: p.best_hash, best_number: p.best_number }) } - /// Returns the current sync status. - pub fn status(&self) -> Status { - let best_seen = if self.peers.is_empty() { + /// Returns the best seen block. + fn best_seen(&self) -> Option> { + let mut best_seens = self.peers.values().map(|p| p.best_number).collect::>(); + best_seens.sort_unstable(); + + if best_seens.is_empty() { None } else { - Some( - self.peers.values().fold(NumberFor::::from(0u32), |c, p| c + p.best_number) / - self.peers.len().saturated_into::().into(), - ) - }; + let len = best_seens.len(); + + if len % 2 == 0 { + Some((best_seens[len / 2] + best_seens[len / 2 - 1]) / 2u32.into()) + } else { + Some(best_seens[len / 2]) + } + } + } + + /// Returns the current sync status. + pub fn status(&self) -> Status { + let best_seen = self.best_seen(); let sync_state = if let Some(n) = best_seen { // A chain is classified as downloading if the provided best block is // more than `MAJOR_SYNC_BLOCKS` behind the best block. From 6007c27fe498b837fc9417a96ff32063756f161d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 31 May 2022 15:23:06 +0200 Subject: [PATCH 3/3] Review comments --- client/network/sync/src/lib.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/client/network/sync/src/lib.rs b/client/network/sync/src/lib.rs index 2c4279c0ab1df..b027d77827374 100644 --- a/client/network/sync/src/lib.rs +++ b/client/network/sync/src/lib.rs @@ -579,18 +579,14 @@ where /// Returns the best seen block. fn best_seen(&self) -> Option> { let mut best_seens = self.peers.values().map(|p| p.best_number).collect::>(); - best_seens.sort_unstable(); if best_seens.is_empty() { None } else { - let len = best_seens.len(); + let middle = best_seens.len() / 2; - if len % 2 == 0 { - Some((best_seens[len / 2] + best_seens[len / 2 - 1]) / 2u32.into()) - } else { - Some(best_seens[len / 2]) - } + // Not the "perfect median" when we have an even number of peers. + Some(*best_seens.select_nth_unstable(middle).1) } }