Skip to content

Commit

Permalink
Disable hostperfcheck by default (paritytech#6640)
Browse files Browse the repository at this point in the history
This feature should only be activated by the polkadot binary. Otherwise
parachains may accidentally activate this feature.
  • Loading branch information
bkchr authored Feb 7, 2023
1 parent ac9a732 commit 81d2650
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 26 deletions.
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ repository = "https://github.com/paritytech/polkadot.git"
version = "0.9.37"

[dependencies]
polkadot-cli = { path = "cli", features = [ "kusama-native", "westend-native", "rococo-native" ] }
polkadot-cli = { path = "cli", features = [ "kusama-native", "westend-native", "rococo-native", "hostperfcheck" ] }
color-eyre = { version = "0.6.1", default-features = false }
tikv-jemallocator = "0.5.0"

Expand All @@ -30,7 +30,7 @@ tempfile = "3.2.0"
tokio = "1.24.2"
substrate-rpc-client = { git = "https://github.com/paritytech/substrate", branch = "master" }
polkadot-core-primitives = { path = "core-primitives" }

[workspace]
members = [
"cli",
Expand Down
2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ sc-storage-monitor = { git = "https://github.com/paritytech/substrate", branch =
substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" }

[features]
default = ["db", "cli", "hostperfcheck", "full-node", "polkadot-native"]
default = ["db", "cli", "full-node", "polkadot-native"]
db = ["service/db"]
cli = [
"clap",
Expand Down
28 changes: 16 additions & 12 deletions cli/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use sp_keyring::Sr25519Keyring;
use std::net::ToSocketAddrs;

pub use crate::{error::Error, service::BlockId};
#[cfg(feature = "hostperfcheck")]
pub use polkadot_performance_test::PerfCheckError;

impl From<String> for Error {
Expand Down Expand Up @@ -238,29 +239,32 @@ macro_rules! unwrap_client {
#[cfg(feature = "rococo-native")]
polkadot_client::Client::Rococo($client) => $code,
#[allow(unreachable_patterns)]
_ => Err(Error::CommandNotImplemented),
_ => {
let _ = $client;

Err(Error::CommandNotImplemented)
},
}
};
}

/// Runs performance checks.
/// Should only be used in release build since the check would take too much time otherwise.
fn host_perf_check() -> Result<()> {
#[cfg(not(build_type = "release"))]
#[cfg(not(feature = "hostperfcheck"))]
{
return Err(Error::FeatureNotEnabled { feature: "hostperfcheck" }.into())
}

#[cfg(all(not(build_type = "release"), feature = "hostperfcheck"))]
{
return Err(PerfCheckError::WrongBuildType.into())
}
#[cfg(build_type = "release")]

#[cfg(all(feature = "hostperfcheck", build_type = "release"))]
{
#[cfg(not(feature = "hostperfcheck"))]
{
return Err(PerfCheckError::FeatureNotEnabled { feature: "hostperfcheck" }.into())
}
#[cfg(feature = "hostperfcheck")]
{
crate::host_perf_check::host_perf_check()?;
return Ok(())
}
crate::host_perf_check::host_perf_check()?;
return Ok(())
}
}

Expand Down
4 changes: 4 additions & 0 deletions cli/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub enum Error {
SubstrateTracing(#[from] sc_tracing::logging::Error),

#[error(transparent)]
#[cfg(feature = "hostperfcheck")]
PerfCheck(#[from] polkadot_performance_test::PerfCheckError),

#[cfg(not(feature = "pyroscope"))]
Expand All @@ -53,4 +54,7 @@ pub enum Error {

#[error("Other: {0}")]
Other(String),

#[error("This subcommand is only available when compiled with `{feature}`")]
FeatureNotEnabled { feature: &'static str },
}
34 changes: 26 additions & 8 deletions node/service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1355,32 +1355,36 @@ pub fn new_chain_ops(
> {
config.keystore = service::config::KeystoreConfig::InMemory;

let telemetry_worker_handle = None;

#[cfg(feature = "rococo-native")]
if config.chain_spec.is_rococo() ||
config.chain_spec.is_wococo() ||
config.chain_spec.is_versi()
{
return chain_ops!(config, jaeger_agent, telemetry_worker_handle; rococo_runtime, RococoExecutorDispatch, Rococo)
return chain_ops!(config, jaeger_agent, None; rococo_runtime, RococoExecutorDispatch, Rococo)
}

#[cfg(feature = "kusama-native")]
if config.chain_spec.is_kusama() {
return chain_ops!(config, jaeger_agent, telemetry_worker_handle; kusama_runtime, KusamaExecutorDispatch, Kusama)
return chain_ops!(config, jaeger_agent, None; kusama_runtime, KusamaExecutorDispatch, Kusama)
}

#[cfg(feature = "westend-native")]
if config.chain_spec.is_westend() {
return chain_ops!(config, jaeger_agent, telemetry_worker_handle; westend_runtime, WestendExecutorDispatch, Westend)
return chain_ops!(config, jaeger_agent, None; westend_runtime, WestendExecutorDispatch, Westend)
}

#[cfg(feature = "polkadot-native")]
{
return chain_ops!(config, jaeger_agent, telemetry_worker_handle; polkadot_runtime, PolkadotExecutorDispatch, Polkadot)
return chain_ops!(config, jaeger_agent, None; polkadot_runtime, PolkadotExecutorDispatch, Polkadot)
}

#[cfg(not(feature = "polkadot-native"))]
Err(Error::NoRuntime)
{
let _ = config;
let _ = jaeger_agent;

Err(Error::NoRuntime)
}
}

/// Build a full node.
Expand Down Expand Up @@ -1488,7 +1492,21 @@ pub fn build_full(
}

#[cfg(not(feature = "polkadot-native"))]
Err(Error::NoRuntime)
{
let _ = config;
let _ = is_collator;
let _ = grandpa_pause;
let _ = enable_beefy;
let _ = jaeger_agent;
let _ = telemetry_worker_handle;
let _ = overseer_enable_anyways;
let _ = overseer_gen;
let _ = overseer_message_channel_override;
let _ = malus_finality_delay;
let _ = hwbench;

Err(Error::NoRuntime)
}
}

/// Reverts the node state down to at most the last finalized block.
Expand Down
3 changes: 0 additions & 3 deletions node/test/performance-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,6 @@ pub enum PerfCheckError {
#[error("This subcommand is only available in release mode")]
WrongBuildType,

#[error("This subcommand is only available when compiled with `{feature}`")]
FeatureNotEnabled { feature: &'static str },

#[error("No wasm code found for running the performance test")]
WasmBinaryMissing,

Expand Down

0 comments on commit 81d2650

Please sign in to comment.