From ba8562b6ab5d36d601f5d33e8b4f983d69b3bd39 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 7 May 2024 14:28:21 +0300 Subject: [PATCH 1/3] bump to 4MB Signed-off-by: Andrei Sandu --- .../network/availability-recovery/src/lib.rs | 16 ++++++++++------ .../network/availability-recovery/src/tests.rs | 4 ++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/polkadot/node/network/availability-recovery/src/lib.rs b/polkadot/node/network/availability-recovery/src/lib.rs index 94b9d9546cde..7f7435dc49b3 100644 --- a/polkadot/node/network/availability-recovery/src/lib.rs +++ b/polkadot/node/network/availability-recovery/src/lib.rs @@ -78,7 +78,7 @@ const LRU_SIZE: u32 = 16; const COST_INVALID_REQUEST: Rep = Rep::CostMajor("Peer sent unparsable request"); /// PoV size limit in bytes for which prefer fetching from backers. -const SMALL_POV_LIMIT: usize = 128 * 1024; +pub(crate) const FETCH_CHUNKS_THRESHOLD: usize = 4 * 1024 * 1024; #[derive(Clone, PartialEq)] /// The strategy we use to recover the PoV. @@ -448,7 +448,7 @@ async fn handle_recover( if let Some(backing_validators) = session_info.validator_groups.get(backing_group) { let mut small_pov_size = true; - if let RecoveryStrategyKind::BackersFirstIfSizeLower(small_pov_limit) = + if let RecoveryStrategyKind::BackersFirstIfSizeLower(fetch_chunks_threshold) = recovery_strategy_kind { // Get our own chunk size to get an estimate of the PoV size. @@ -457,13 +457,13 @@ async fn handle_recover( if let Ok(Some(chunk_size)) = chunk_size { let pov_size_estimate = chunk_size.saturating_mul(session_info.validators.len()) / 3; - small_pov_size = pov_size_estimate < small_pov_limit; + small_pov_size = pov_size_estimate < fetch_chunks_threshold; gum::trace!( target: LOG_TARGET, ?candidate_hash, pov_size_estimate, - small_pov_limit, + fetch_chunks_threshold, enabled = small_pov_size, "Prefer fetch from backing group", ); @@ -551,7 +551,9 @@ impl AvailabilityRecoverySubsystem { metrics: Metrics, ) -> Self { Self { - recovery_strategy_kind: RecoveryStrategyKind::BackersFirstIfSizeLower(SMALL_POV_LIMIT), + recovery_strategy_kind: RecoveryStrategyKind::BackersFirstIfSizeLower( + FETCH_CHUNKS_THRESHOLD, + ), bypass_availability_store: true, post_recovery_check: PostRecoveryCheck::PovHash, req_receiver, @@ -595,7 +597,9 @@ impl AvailabilityRecoverySubsystem { metrics: Metrics, ) -> Self { Self { - recovery_strategy_kind: RecoveryStrategyKind::BackersFirstIfSizeLower(SMALL_POV_LIMIT), + recovery_strategy_kind: RecoveryStrategyKind::BackersFirstIfSizeLower( + FETCH_CHUNKS_THRESHOLD, + ), bypass_availability_store: false, post_recovery_check: PostRecoveryCheck::Reencode, req_receiver, diff --git a/polkadot/node/network/availability-recovery/src/tests.rs b/polkadot/node/network/availability-recovery/src/tests.rs index 909f6a25f46b..cdac09f04797 100644 --- a/polkadot/node/network/availability-recovery/src/tests.rs +++ b/polkadot/node/network/availability-recovery/src/tests.rs @@ -942,7 +942,7 @@ fn recovers_from_only_chunks_if_pov_large() { AllMessages::AvailabilityStore( AvailabilityStoreMessage::QueryChunkSize(_, tx) ) => { - let _ = tx.send(Some(1000000)); + let _ = tx.send(Some(crate::FETCH_CHUNKS_THRESHOLD + 1)); } ); @@ -987,7 +987,7 @@ fn recovers_from_only_chunks_if_pov_large() { AllMessages::AvailabilityStore( AvailabilityStoreMessage::QueryChunkSize(_, tx) ) => { - let _ = tx.send(Some(1000000)); + let _ = tx.send(Some(crate::FETCH_CHUNKS_THRESHOLD + 1)); } ); From 8f76ff39c83e38d00e92494f8c606f8abeccf08b Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 24 May 2024 13:06:33 +0300 Subject: [PATCH 2/3] Use 1MB on polkadot, 4MB on Kusama and all testnets Signed-off-by: Andrei Sandu --- .../node/network/availability-recovery/src/lib.rs | 12 ++++++++---- polkadot/node/service/src/lib.rs | 7 +++++++ polkadot/node/service/src/overseer.rs | 7 +++++++ 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/polkadot/node/network/availability-recovery/src/lib.rs b/polkadot/node/network/availability-recovery/src/lib.rs index 7f7435dc49b3..b836870cd8af 100644 --- a/polkadot/node/network/availability-recovery/src/lib.rs +++ b/polkadot/node/network/availability-recovery/src/lib.rs @@ -77,8 +77,10 @@ const LRU_SIZE: u32 = 16; const COST_INVALID_REQUEST: Rep = Rep::CostMajor("Peer sent unparsable request"); -/// PoV size limit in bytes for which prefer fetching from backers. -pub(crate) const FETCH_CHUNKS_THRESHOLD: usize = 4 * 1024 * 1024; +/// PoV size limit in bytes for which prefer fetching from backers. (conservative, Polkadot for now) +pub(crate) const CONSERVATIVE_FETCH_CHUNKS_THRESHOLD: usize = 1 * 1024 * 1024; +/// PoV size limit in bytes for which prefer fetching from backers. (Kusama and all testnets) +pub const FETCH_CHUNKS_THRESHOLD: usize = 4 * 1024 * 1024; #[derive(Clone, PartialEq)] /// The strategy we use to recover the PoV. @@ -547,12 +549,13 @@ impl AvailabilityRecoverySubsystem { /// which never requests the `AvailabilityStoreSubsystem` subsystem and only checks the POV hash /// instead of reencoding the available data. pub fn for_collator( + fetch_chunks_threshold: Option, req_receiver: IncomingRequestReceiver, metrics: Metrics, ) -> Self { Self { recovery_strategy_kind: RecoveryStrategyKind::BackersFirstIfSizeLower( - FETCH_CHUNKS_THRESHOLD, + fetch_chunks_threshold.unwrap_or(CONSERVATIVE_FETCH_CHUNKS_THRESHOLD), ), bypass_availability_store: true, post_recovery_check: PostRecoveryCheck::PovHash, @@ -593,12 +596,13 @@ impl AvailabilityRecoverySubsystem { /// Create a new instance of `AvailabilityRecoverySubsystem` which requests chunks if PoV is /// above a threshold. pub fn with_chunks_if_pov_large( + fetch_chunks_threshold: Option, req_receiver: IncomingRequestReceiver, metrics: Metrics, ) -> Self { Self { recovery_strategy_kind: RecoveryStrategyKind::BackersFirstIfSizeLower( - FETCH_CHUNKS_THRESHOLD, + fetch_chunks_threshold.unwrap_or(CONSERVATIVE_FETCH_CHUNKS_THRESHOLD), ), bypass_availability_store: false, post_recovery_check: PostRecoveryCheck::Reencode, diff --git a/polkadot/node/service/src/lib.rs b/polkadot/node/service/src/lib.rs index 665533e9bc70..6d365b93ac7c 100644 --- a/polkadot/node/service/src/lib.rs +++ b/polkadot/node/service/src/lib.rs @@ -750,6 +750,7 @@ pub fn new_full< prepare_workers_hard_max_num, }: NewFullParams, ) -> Result { + use polkadot_availability_recovery::FETCH_CHUNKS_THRESHOLD; use polkadot_node_network_protocol::request_response::IncomingRequest; use sc_network_sync::WarpSyncParams; @@ -988,6 +989,11 @@ pub fn new_full< stagnant_check_interval: Default::default(), stagnant_check_mode: chain_selection_subsystem::StagnantCheckMode::PruneOnly, }; + + // Kusama + testnets get a higher threshold, we are conservative on Polkadot for now. + let fetch_chunks_threshold = + if config.chain_spec.is_polkadot() { None } else { Some(FETCH_CHUNKS_THRESHOLD) }; + Some(ExtendedOverseerGenArgs { keystore, parachains_db, @@ -1001,6 +1007,7 @@ pub fn new_full< dispute_req_receiver, dispute_coordinator_config, chain_selection_config, + fetch_chunks_threshold, }) }; diff --git a/polkadot/node/service/src/overseer.rs b/polkadot/node/service/src/overseer.rs index 4b7777a09671..175a77e1c5f6 100644 --- a/polkadot/node/service/src/overseer.rs +++ b/polkadot/node/service/src/overseer.rs @@ -133,6 +133,10 @@ pub struct ExtendedOverseerGenArgs { pub dispute_coordinator_config: DisputeCoordinatorConfig, /// Configuration for the chain selection subsystem. pub chain_selection_config: ChainSelectionConfig, + /// Optional availability recovery fetch chunks threshold. If PoV size size is lower + /// than the value put in here we always try to recovery availability from backers. + /// The presence of this parameter here is needed to have different values per chain. + pub fetch_chunks_threshold: Option, } /// Obtain a prepared validator `Overseer`, that is initialized with all default values. @@ -166,6 +170,7 @@ pub fn validator_overseer_builder( dispute_req_receiver, dispute_coordinator_config, chain_selection_config, + fetch_chunks_threshold, }: ExtendedOverseerGenArgs, ) -> Result< InitializedOverseerBuilder< @@ -240,6 +245,7 @@ where Metrics::register(registry)?, )) .availability_recovery(AvailabilityRecoverySubsystem::with_chunks_if_pov_large( + fetch_chunks_threshold, available_data_req_receiver, Metrics::register(registry)?, )) @@ -421,6 +427,7 @@ where )) .availability_distribution(DummySubsystem) .availability_recovery(AvailabilityRecoverySubsystem::for_collator( + None, available_data_req_receiver, Metrics::register(registry)?, )) From 2524164d720a0be8a695e7333707ccd78bdd7b9b Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 24 May 2024 13:18:21 +0300 Subject: [PATCH 3/3] clippy Signed-off-by: Andrei Sandu --- polkadot/node/network/availability-recovery/src/tests.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/polkadot/node/network/availability-recovery/src/tests.rs b/polkadot/node/network/availability-recovery/src/tests.rs index cdac09f04797..6049a5a5c3a2 100644 --- a/polkadot/node/network/availability-recovery/src/tests.rs +++ b/polkadot/node/network/availability-recovery/src/tests.rs @@ -906,6 +906,7 @@ fn recovers_from_only_chunks_if_pov_large() { let test_state = TestState::default(); let req_protocol_names = ReqProtocolNames::new(&GENESIS_HASH, None); let subsystem = AvailabilityRecoverySubsystem::with_chunks_if_pov_large( + Some(FETCH_CHUNKS_THRESHOLD), request_receiver(&req_protocol_names), Metrics::new_dummy(), ); @@ -1015,6 +1016,7 @@ fn fast_path_backing_group_recovers_if_pov_small() { let test_state = TestState::default(); let req_protocol_names = ReqProtocolNames::new(&GENESIS_HASH, None); let subsystem = AvailabilityRecoverySubsystem::with_chunks_if_pov_large( + Some(FETCH_CHUNKS_THRESHOLD), request_receiver(&req_protocol_names), Metrics::new_dummy(), );