From 416aaeb7631eddc6358ab0dc0983e2dfd02670ef Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Thu, 5 May 2022 10:21:50 +0200 Subject: [PATCH 1/8] allow runtime adjustment of signal channel size Closes #5436 --- .../proc-macro/src/impl_builder.rs | 61 ++++++++++++++++--- 1 file changed, 54 insertions(+), 7 deletions(-) diff --git a/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs b/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs index a1c9450c4178..ff0b8a313228 100644 --- a/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs +++ b/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs @@ -359,13 +359,12 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { pub fn handle(&self) -> #handle { self.handle.clone() } - } - impl ::std::default::Default for #connector { - fn default() -> Self { + /// Create a new connector with non-default signal channel capacity. + fn with_signal_capacity(signal_capacity: usize) -> Self { let (events_tx, events_rx) = #support_crate ::metered::channel::< #event - >(SIGNAL_CHANNEL_CAPACITY); + >(signal_capacity); Self { handle: events_tx, @@ -373,6 +372,12 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { } } } + + impl ::std::default::Default for #connector { + fn default() -> Self { + Self::with_capacity(SIGNAL_CHANNEL_CAPACITY) + } + } }); ts.extend(quote!{ @@ -385,6 +390,11 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { #baggage_name: #baggage_passthrough_state_generics, )* spawner: InitStateSpawner, + // user provided runtime overrides, + // if `None`, the `overlord(message_capacity=123,..)` is used + // or the default value. + channel_capacity: Option, + signal_capacity: Option, } }); @@ -406,6 +416,9 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { #field_name: Missing::<#field_type>::default(), )* spawner: Missing::::default(), + + channel_capacity: None, + signal_capacity: None, } } } @@ -419,18 +432,48 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { #spawner_where_clause { /// The `spawner` to use for spawning tasks. - pub fn spawner(self, spawner: S) -> #builder, #( #subsystem_passthrough_state_generics, )* #( #baggage_passthrough_state_generics, )*> + pub fn spawner(self, spawner: S) -> #builder< + Init, + #( #subsystem_passthrough_state_generics, )* + #( #baggage_passthrough_state_generics, )* + > { #builder { #( #field_name: self. #field_name, )* spawner: Init::::Value(spawner), + + channel_capacity: self.channel_capacity, + signal_capacity: self.signal_capacity, } } } }); + // message and signal channel capacity + ts.extend(quote!{ + impl + #builder + where + #spawner_where_clause + { + /// Set the interconnecting signal channel capacity. + pub fn signal_channel_capacity(self, capacity: usize) -> Self + { + self.signal_capacity = Some(capacity); + self + } + + /// Set the interconnecting message channel capacities. + pub fn message_channel_capacity(self, capacity: usize) -> Self + { + self.channel_capacity = Some(capacity); + self + } + } + }); + ts.extend(quote! { /// Type used to represent a builder where all fields are initialized and the overseer could be constructed. pub type #initialized_builder<#initialized_builder_generics> = #builder, #( Init<#field_type>, )*>; @@ -446,7 +489,9 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { /// Complete the construction and create the overseer type. pub fn build(self) -> ::std::result::Result<(#overseer_name, #handle), #error_ty> { - let connector = #connector ::default(); + let connector = #connector ::with_signal_capacity( + self.signal_capacity.unwrap_or(SIGNAL_CHANNEL_CAPACITY) + ); self.build_with_connector(connector) } @@ -470,7 +515,9 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { = #support_crate ::metered::channel::< MessagePacket< #consumes > - >(CHANNEL_CAPACITY); + >( + self.channel_capacity.unwrap_or(CHANNEL_CAPACITY) + ); )* #( From d0bf0d90d5e507891456e6783c33b8d17bdecbcf Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Thu, 5 May 2022 10:52:10 +0200 Subject: [PATCH 2/8] expose override of overseer channel size --- cli/src/cli.rs | 6 ++++++ cli/src/command.rs | 1 + node/overseer/overseer-gen/proc-macro/src/impl_builder.rs | 2 +- node/service/src/lib.rs | 1 + parachain/test-parachains/adder/collator/src/main.rs | 1 + parachain/test-parachains/undying/collator/src/main.rs | 1 + 6 files changed, 11 insertions(+), 1 deletion(-) diff --git a/cli/src/cli.rs b/cli/src/cli.rs index cc449bd844ae..804c4a9f81c6 100644 --- a/cli/src/cli.rs +++ b/cli/src/cli.rs @@ -138,6 +138,12 @@ pub struct RunCmd { /// telemetry, if telemetry is enabled. #[clap(long)] pub no_hardware_benchmarks: bool, + + /// Overseer message capacity override. + /// + /// **Dangerous!** Do not touch unless explicitly adviced to. + #[clap(long)] + pub overseer_channel_capacity_override: Option, } #[allow(missing_docs)] diff --git a/cli/src/command.rs b/cli/src/command.rs index 8869d9a4ead4..562fec638c66 100644 --- a/cli/src/command.rs +++ b/cli/src/command.rs @@ -331,6 +331,7 @@ where jaeger_agent, None, false, + cli.run.overseer_channel_capacity_override, overseer_gen, hwbench, ) diff --git a/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs b/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs index ff0b8a313228..a233dc48939b 100644 --- a/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs +++ b/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs @@ -452,7 +452,7 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { }); // message and signal channel capacity - ts.extend(quote!{ + ts.extend(quote! { impl #builder where diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index 43897cb8c7d8..9780b2def2d7 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -1326,6 +1326,7 @@ pub fn build_full( telemetry_worker_handle: Option, overseer_enable_anyways: bool, overseer_gen: impl OverseerGen, + overseer_channel_capacity_override: Option, hwbench: Option, ) -> Result, Error> { #[cfg(feature = "rococo-native")] diff --git a/parachain/test-parachains/adder/collator/src/main.rs b/parachain/test-parachains/adder/collator/src/main.rs index 2b3e468d9b42..703ee8320e1b 100644 --- a/parachain/test-parachains/adder/collator/src/main.rs +++ b/parachain/test-parachains/adder/collator/src/main.rs @@ -70,6 +70,7 @@ fn main() -> Result<()> { None, false, polkadot_service::RealOverseerGen, + cli.run.overseer_channel_capacity_override, None, ) .map_err(|e| e.to_string())?; diff --git a/parachain/test-parachains/undying/collator/src/main.rs b/parachain/test-parachains/undying/collator/src/main.rs index 5bacf927e4fb..ef392557b0f7 100644 --- a/parachain/test-parachains/undying/collator/src/main.rs +++ b/parachain/test-parachains/undying/collator/src/main.rs @@ -70,6 +70,7 @@ fn main() -> Result<()> { None, false, polkadot_service::RealOverseerGen, + cli.run.overseer_channel_capacity_override, None, ) .map_err(|e| e.to_string())?; From e8bc9f99c07a9165980f745bb90d824d19c7f44c Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Thu, 5 May 2022 10:57:51 +0200 Subject: [PATCH 3/8] chore: rename --- node/overseer/overseer-gen/proc-macro/src/impl_builder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs b/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs index a233dc48939b..ceb5e4230b44 100644 --- a/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs +++ b/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs @@ -361,7 +361,7 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { } /// Create a new connector with non-default signal channel capacity. - fn with_signal_capacity(signal_capacity: usize) -> Self { + pub fn with_signal_capacity(signal_capacity: usize) -> Self { let (events_tx, events_rx) = #support_crate ::metered::channel::< #event >(signal_capacity); @@ -375,7 +375,7 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { impl ::std::default::Default for #connector { fn default() -> Self { - Self::with_capacity(SIGNAL_CHANNEL_CAPACITY) + Self::with_signal_capacity(SIGNAL_CHANNEL_CAPACITY) } } }); From 6cfab51fa082fb7208bf439fbb35ddcb0d72436a Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Thu, 5 May 2022 11:27:35 +0200 Subject: [PATCH 4/8] rustc goes yay --- cli/src/command.rs | 2 +- .../proc-macro/src/impl_builder.rs | 38 ++++++++++++++----- node/service/src/lib.rs | 8 +++- node/service/src/overseer.rs | 10 ++++- 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/cli/src/command.rs b/cli/src/command.rs index 562fec638c66..19c046e7c2e2 100644 --- a/cli/src/command.rs +++ b/cli/src/command.rs @@ -331,8 +331,8 @@ where jaeger_agent, None, false, - cli.run.overseer_channel_capacity_override, overseer_gen, + cli.run.overseer_channel_capacity_override, hwbench, ) .map(|full| full.task_manager) diff --git a/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs b/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs index ceb5e4230b44..6d634cc9f344 100644 --- a/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs +++ b/node/overseer/overseer-gen/proc-macro/src/impl_builder.rs @@ -150,6 +150,9 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { #baggage_name: self. #baggage_name, )* spawner: self.spawner, + + channel_capacity: self.channel_capacity, + signal_capacity: self.signal_capacity, } } /// Specify the the initialization function for a subsystem @@ -171,6 +174,10 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { #baggage_name: self. #baggage_name, )* spawner: self.spawner, + + + channel_capacity: self.channel_capacity, + signal_capacity: self.signal_capacity, } } } @@ -207,6 +214,9 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { #baggage_name: self. #baggage_name, )* spawner: self.spawner, + + channel_capacity: self.channel_capacity, + signal_capacity: self.signal_capacity, } } } @@ -254,6 +264,9 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { #to_keep_baggage_name: self. #to_keep_baggage_name, )* spawner: self.spawner, + + channel_capacity: self.channel_capacity, + signal_capacity: self.signal_capacity, } } } @@ -272,6 +285,9 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { #to_keep_baggage_name: self. #to_keep_baggage_name, )* spawner: self.spawner, + + channel_capacity: self.channel_capacity, + signal_capacity: self.signal_capacity, } } } @@ -360,11 +376,11 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { self.handle.clone() } - /// Create a new connector with non-default signal channel capacity. - pub fn with_signal_capacity(signal_capacity: usize) -> Self { + /// Create a new connector with non-default event channel capacity. + pub fn with_event_capacity(event_channel_capacity: usize) -> Self { let (events_tx, events_rx) = #support_crate ::metered::channel::< #event - >(signal_capacity); + >(event_channel_capacity); Self { handle: events_tx, @@ -375,7 +391,7 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { impl ::std::default::Default for #connector { fn default() -> Self { - Self::with_signal_capacity(SIGNAL_CHANNEL_CAPACITY) + Self::with_event_capacity(SIGNAL_CHANNEL_CAPACITY) } } }); @@ -454,19 +470,19 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { // message and signal channel capacity ts.extend(quote! { impl - #builder + #builder, #( #subsystem_passthrough_state_generics, )* #( #baggage_passthrough_state_generics, )*> where - #spawner_where_clause + #spawner_where_clause, { /// Set the interconnecting signal channel capacity. - pub fn signal_channel_capacity(self, capacity: usize) -> Self + pub fn signal_channel_capacity(mut self, capacity: usize) -> Self { self.signal_capacity = Some(capacity); self } /// Set the interconnecting message channel capacities. - pub fn message_channel_capacity(self, capacity: usize) -> Self + pub fn message_channel_capacity(mut self, capacity: usize) -> Self { self.channel_capacity = Some(capacity); self @@ -489,7 +505,7 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { /// Complete the construction and create the overseer type. pub fn build(self) -> ::std::result::Result<(#overseer_name, #handle), #error_ty> { - let connector = #connector ::with_signal_capacity( + let connector = #connector ::with_event_capacity( self.signal_capacity.unwrap_or(SIGNAL_CHANNEL_CAPACITY) ); self.build_with_connector(connector) @@ -557,7 +573,9 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream { let message_rx: SubsystemIncomingMessages< #consumes > = #support_crate ::select( #channel_name_rx, #channel_name_unbounded_rx ); - let (signal_tx, signal_rx) = #support_crate ::metered::channel(SIGNAL_CHANNEL_CAPACITY); + let (signal_tx, signal_rx) = #support_crate ::metered::channel( + self.signal_capacity.unwrap_or(SIGNAL_CHANNEL_CAPACITY) + ); // Generate subsystem name based on overseer field name. let subsystem_string = String::from(stringify!(#subsystem_name)); diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index 9780b2def2d7..aaeb33c60381 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -719,6 +719,7 @@ pub fn new_full( program_path: Option, overseer_enable_anyways: bool, overseer_gen: OverseerGenerator, + overseer_message_channel_capacity_override: Option, hwbench: Option, ) -> Result>>, Error> where @@ -1038,6 +1039,7 @@ where chain_selection_config, dispute_coordinator_config, pvf_checker_enabled, + overseer_message_channel_capacity_override, }, ) .map_err(|e| { @@ -1326,7 +1328,7 @@ pub fn build_full( telemetry_worker_handle: Option, overseer_enable_anyways: bool, overseer_gen: impl OverseerGen, - overseer_channel_capacity_override: Option, + overseer_message_channel_override: Option, hwbench: Option, ) -> Result, Error> { #[cfg(feature = "rococo-native")] @@ -1344,6 +1346,7 @@ pub fn build_full( None, overseer_enable_anyways, overseer_gen, + overseer_message_channel_override, hwbench, ) .map(|full| full.with_client(Client::Rococo)) @@ -1361,6 +1364,7 @@ pub fn build_full( None, overseer_enable_anyways, overseer_gen, + overseer_message_channel_override, hwbench, ) .map(|full| full.with_client(Client::Kusama)) @@ -1378,6 +1382,7 @@ pub fn build_full( None, overseer_enable_anyways, overseer_gen, + overseer_message_channel_override, hwbench, ) .map(|full| full.with_client(Client::Westend)) @@ -1395,6 +1400,7 @@ pub fn build_full( None, overseer_enable_anyways, overseer_gen, + None, hwbench, ) .map(|full| full.with_client(Client::Polkadot)) diff --git a/node/service/src/overseer.rs b/node/service/src/overseer.rs index fd07ddfe825d..aa90abb91fb2 100644 --- a/node/service/src/overseer.rs +++ b/node/service/src/overseer.rs @@ -111,6 +111,8 @@ where pub dispute_coordinator_config: DisputeCoordinatorConfig, /// Enable PVF pre-checking pub pvf_checker_enabled: bool, + /// Overseer channel capacity override. + pub overseer_message_channel_capacity_override: Option, } /// Obtain a prepared `OverseerBuilder`, that is initialized @@ -138,6 +140,7 @@ pub fn prepared_overseer_builder<'a, Spawner, RuntimeClient>( chain_selection_config, dispute_coordinator_config, pvf_checker_enabled, + overseer_message_channel_capacity_override, }: OverseerGenArgs<'a, Spawner, RuntimeClient>, ) -> Result< InitializedOverseerBuilder< @@ -292,7 +295,12 @@ where .known_leaves(LruCache::new(KNOWN_LEAVES_CACHE_SIZE)) .metrics(metrics) .spawner(spawner); - Ok(builder) + + if let Some(capacity) = overseer_message_channel_capacity_override { + Ok(builder.message_channel_capacity(capacity)) + } else { + Ok(builder) + } } /// Trait for the `fn` generating the overseer. From c5d628ae3880cd8b2a4e283449b953a643c9a60a Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Thu, 5 May 2022 12:00:42 +0200 Subject: [PATCH 5/8] fix test --- node/test/service/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/node/test/service/src/lib.rs b/node/test/service/src/lib.rs index 4ac3d765c82b..c1a381e10089 100644 --- a/node/test/service/src/lib.rs +++ b/node/test/service/src/lib.rs @@ -98,6 +98,7 @@ pub fn new_full( false, polkadot_service::RealOverseerGen, None, + None, ) } From 2a7e9358dc052a66c6df9873be9791fba72a52df Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Thu, 5 May 2022 13:37:04 +0200 Subject: [PATCH 6/8] missing args in tests --- parachain/test-parachains/adder/collator/src/main.rs | 2 +- parachain/test-parachains/undying/collator/src/main.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/parachain/test-parachains/adder/collator/src/main.rs b/parachain/test-parachains/adder/collator/src/main.rs index 703ee8320e1b..25f2920dafc7 100644 --- a/parachain/test-parachains/adder/collator/src/main.rs +++ b/parachain/test-parachains/adder/collator/src/main.rs @@ -70,7 +70,7 @@ fn main() -> Result<()> { None, false, polkadot_service::RealOverseerGen, - cli.run.overseer_channel_capacity_override, + None, None, ) .map_err(|e| e.to_string())?; diff --git a/parachain/test-parachains/undying/collator/src/main.rs b/parachain/test-parachains/undying/collator/src/main.rs index ef392557b0f7..68be8cafe59c 100644 --- a/parachain/test-parachains/undying/collator/src/main.rs +++ b/parachain/test-parachains/undying/collator/src/main.rs @@ -70,7 +70,7 @@ fn main() -> Result<()> { None, false, polkadot_service::RealOverseerGen, - cli.run.overseer_channel_capacity_override, + None, None, ) .map_err(|e| e.to_string())?; From 0b8c4b975245da5f42cf0c16718c607f63288405 Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Thu, 5 May 2022 13:50:28 +0200 Subject: [PATCH 7/8] disable beefy on adder and undying collators --- parachain/test-parachains/adder/collator/src/main.rs | 2 +- parachain/test-parachains/undying/collator/src/main.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/parachain/test-parachains/adder/collator/src/main.rs b/parachain/test-parachains/adder/collator/src/main.rs index 25f2920dafc7..00e1532fcf68 100644 --- a/parachain/test-parachains/adder/collator/src/main.rs +++ b/parachain/test-parachains/adder/collator/src/main.rs @@ -65,7 +65,7 @@ fn main() -> Result<()> { config, polkadot_service::IsCollator::Yes(collator.collator_key()), None, - true, + false, None, None, false, diff --git a/parachain/test-parachains/undying/collator/src/main.rs b/parachain/test-parachains/undying/collator/src/main.rs index 68be8cafe59c..65e97f34f695 100644 --- a/parachain/test-parachains/undying/collator/src/main.rs +++ b/parachain/test-parachains/undying/collator/src/main.rs @@ -65,7 +65,7 @@ fn main() -> Result<()> { config, polkadot_service::IsCollator::Yes(collator.collator_key()), None, - true, + false, None, None, false, From c430a679afbc730a28b5ba47dbdb6b803304c6ac Mon Sep 17 00:00:00 2001 From: Bernhard Schuster Date: Thu, 5 May 2022 15:13:41 +0200 Subject: [PATCH 8/8] add warning, avoid a warn causing a build failure --- node/service/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index aaeb33c60381..810b0b61aaf3 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -1400,7 +1400,10 @@ pub fn build_full( None, overseer_enable_anyways, overseer_gen, - None, + overseer_message_channel_override.map(|capacity| { + gum::warn!("Channel capacity should _never_ be tampered with on polkadot!"); + capacity + }), hwbench, ) .map(|full| full.with_client(Client::Polkadot))