Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Warn validators with slow hardware #12620

Merged
merged 21 commits into from
Jan 20, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion bin/node/cli/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
//! Service implementation. Specialized wrapper over substrate service.

use codec::Encode;
use frame_benchmarking_cli::SUBSTRATE_REFERENCE_HARDWARE;
use frame_system_rpc_runtime_api::AccountNonceApi;
use futures::prelude::*;
use kitchensink_runtime::RuntimeApi;
Expand Down Expand Up @@ -320,7 +321,11 @@ pub fn new_full_base(
let hwbench = if !disable_hardware_benchmarks {
config.database.path().map(|database_path| {
let _ = std::fs::create_dir_all(&database_path);
sc_sysinfo::gather_hwbench(Some(database_path))
sc_sysinfo::gather_hwbench(
Some(database_path),
SUBSTRATE_REFERENCE_HARDWARE.clone(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its a bit unfortunate that we have to pass this in twice now; once for the sysinfo and once for the machine benchmarking.
But I dont see how it could be done better.

config.role.is_authority(),
)
})
} else {
None
Expand Down
3 changes: 2 additions & 1 deletion client/sysinfo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ mod sysinfo_linux;
pub use sysinfo::{
benchmark_cpu, benchmark_disk_random_writes, benchmark_disk_sequential_writes,
benchmark_memory, benchmark_sr25519_verify, gather_hwbench, gather_sysinfo,
serialize_throughput, serialize_throughput_option, Throughput,
serialize_throughput, serialize_throughput_option, Metric, Requirement, Requirements,
Throughput,
};

/// The operating system part of the current target triplet.
Expand Down
131 changes: 128 additions & 3 deletions client/sysinfo/src/sysinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ use crate::{ExecutionLimit, HwBench};
use sc_telemetry::SysInfo;
use sp_core::{sr25519, Pair};
use sp_io::crypto::sr25519_verify;
use sp_std::{fmt, prelude::*};
use sp_std::{fmt, fmt::Formatter, prelude::*};

use rand::{seq::SliceRandom, Rng, RngCore};
use serde::Serializer;
use serde::{de::Visitor, Deserialize, Deserializer, Serialize, Serializer};
use std::{
fs::File,
io::{Seek, SeekFrom, Write},
Expand All @@ -33,6 +33,43 @@ use std::{
time::{Duration, Instant},
};

/// A single hardware metric.
#[derive(Deserialize, Serialize, Debug, Clone, Copy, PartialEq)]
pub enum Metric {
/// SR25519 signature verification.
Sr25519Verify,
/// Blake2-256 hashing algorithm.
Blake2256,
/// Copying data in RAM.
MemCopy,
/// Disk sequential write.
DiskSeqWrite,
/// Disk random write.
DiskRndWrite,
}

impl Metric {
/// The category of the metric.
pub fn category(&self) -> &'static str {
match self {
Self::Sr25519Verify | Self::Blake2256 => "CPU",
Self::MemCopy => "Memory",
Self::DiskSeqWrite | Self::DiskRndWrite => "Disk",
}
}

/// The name of the metric. It is always prefixed by the [`self.category()`].
pub fn name(&self) -> &'static str {
match self {
Self::Sr25519Verify => "SR25519-Verify",
Self::Blake2256 => "BLAKE2-256",
Self::MemCopy => "Copy",
Self::DiskSeqWrite => "Seq Write",
Self::DiskRndWrite => "Rnd Write",
}
}
}

/// The unit in which the [`Throughput`] (bytes per second) is denoted.
pub enum Unit {
GiBs,
Expand Down Expand Up @@ -137,6 +174,54 @@ where
serializer.serialize_none()
}

/// Serializes throughput into MiBs and represents it as `f64`.
fn serialize_throughput_as_f64<S>(throughput: &Throughput, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
serializer.serialize_f64(throughput.as_mibs())
}

struct ThroughputVisitor;
impl<'de> Visitor<'de> for ThroughputVisitor {
type Value = Throughput;

fn expecting(&self, formatter: &mut Formatter) -> fmt::Result {
formatter.write_str("A value that is a f64.")
}

fn visit_f64<E>(self, value: f64) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
Ok(Throughput::from_mibs(value))
}
}

fn deserialize_throughput<'de, D>(deserializer: D) -> Result<Throughput, D::Error>
where
D: Deserializer<'de>,
{
Ok(deserializer.deserialize_f64(ThroughputVisitor))?
}

/// Multiple requirements for the hardware.
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
pub struct Requirements(pub Vec<Requirement>);

/// A single requirement for the hardware.
#[derive(Deserialize, Serialize, Debug, Clone, Copy, PartialEq)]
pub struct Requirement {
/// The metric to measure.
pub metric: Metric,
/// The minimal throughput that needs to be archived for this requirement.
#[serde(
serialize_with = "serialize_throughput_as_f64",
deserialize_with = "deserialize_throughput"
)]
pub minimum: Throughput,
}

#[inline(always)]
pub(crate) fn benchmark<E>(
name: &str,
Expand Down Expand Up @@ -504,7 +589,11 @@ pub fn benchmark_sr25519_verify(limit: ExecutionLimit) -> Throughput {
/// Benchmarks the hardware and returns the results of those benchmarks.
///
/// Optionally accepts a path to a `scratch_directory` to use to benchmark the disk.
pub fn gather_hwbench(scratch_directory: Option<&Path>) -> HwBench {
pub fn gather_hwbench(
Szegoo marked this conversation as resolved.
Show resolved Hide resolved
scratch_directory: Option<&Path>,
requirements: Requirements,
is_authority: bool,
) -> HwBench {
#[allow(unused_mut)]
let mut hwbench = HwBench {
cpu_hashrate_score: benchmark_cpu(DEFAULT_CPU_EXECUTION_LIMIT),
Expand Down Expand Up @@ -534,9 +623,45 @@ pub fn gather_hwbench(scratch_directory: Option<&Path>) -> HwBench {
};
}

if is_authority {
ensure_requirements(hwbench.clone(), requirements);
}

hwbench
}

fn ensure_requirements(hwbench: HwBench, requirements: Requirements) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also slightly bad that we have the logic here again, but I think its okay.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function could just have been made public and return a bool. Everybody could then build its own logic.

let mut failed = 0;
for requirement in requirements.0.iter() {
match requirement.metric {
Metric::Blake2256 =>
if requirement.minimum > hwbench.cpu_hashrate_score {
failed += 1;
},
Metric::MemCopy =>
if requirement.minimum > hwbench.memory_memcpy_score {
failed += 1;
},
Metric::DiskSeqWrite =>
if let Some(score) = hwbench.disk_sequential_write_score {
if requirement.minimum > score {
failed += 1;
}
},
Metric::DiskRndWrite =>
if let Some(score) = hwbench.disk_random_write_score {
if requirement.minimum > score {
failed += 1;
}
},
_ => (),
Szegoo marked this conversation as resolved.
Show resolved Hide resolved
}
}
if failed != 0 {
log::warn!("The hardware fails to meet the requirements");
Szegoo marked this conversation as resolved.
Show resolved Hide resolved
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
2 changes: 1 addition & 1 deletion utils/frame/benchmarking-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ mod storage;

pub use block::BlockCmd;
pub use extrinsic::{ExtrinsicBuilder, ExtrinsicCmd, ExtrinsicFactory};
pub use machine::{MachineCmd, Requirements, SUBSTRATE_REFERENCE_HARDWARE};
pub use machine::{MachineCmd, SUBSTRATE_REFERENCE_HARDWARE};
pub use overhead::OverheadCmd;
pub use pallet::PalletCmd;
pub use sc_service::BasePath;
Expand Down
91 changes: 1 addition & 90 deletions utils/frame/benchmarking-cli/src/machine/hardware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,40 +18,7 @@
//! Contains types to define hardware requirements.

use lazy_static::lazy_static;
use sc_sysinfo::Throughput;
use serde::{de::Visitor, Deserialize, Deserializer, Serialize, Serializer};
use sp_std::{fmt, fmt::Formatter};

/// Serializes throughput into MiBs and represents it as `f64`.
fn serialize_throughput_as_f64<S>(throughput: &Throughput, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
serializer.serialize_f64(throughput.as_mibs())
}

struct ThroughputVisitor;
impl<'de> Visitor<'de> for ThroughputVisitor {
type Value = Throughput;

fn expecting(&self, formatter: &mut Formatter) -> fmt::Result {
formatter.write_str("A value that is a f64.")
}

fn visit_f64<E>(self, value: f64) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
Ok(Throughput::from_mibs(value))
}
}

fn deserialize_throughput<'de, D>(deserializer: D) -> Result<Throughput, D::Error>
where
D: Deserializer<'de>,
{
Ok(deserializer.deserialize_f64(ThroughputVisitor))?
}
use sc_sysinfo::Requirements;

lazy_static! {
/// The hardware requirements as measured on reference hardware.
Expand All @@ -67,62 +34,6 @@ lazy_static! {
};
}

/// Multiple requirements for the hardware.
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
pub struct Requirements(pub Vec<Requirement>);

/// A single requirement for the hardware.
#[derive(Deserialize, Serialize, Debug, Clone, Copy, PartialEq)]
pub struct Requirement {
/// The metric to measure.
pub metric: Metric,
/// The minimal throughput that needs to be archived for this requirement.
#[serde(
serialize_with = "serialize_throughput_as_f64",
deserialize_with = "deserialize_throughput"
)]
pub minimum: Throughput,
}

/// A single hardware metric.
///
/// The implementation of these is in `sc-sysinfo`.
#[derive(Deserialize, Serialize, Debug, Clone, Copy, PartialEq)]
pub enum Metric {
/// SR25519 signature verification.
Sr25519Verify,
/// Blake2-256 hashing algorithm.
Blake2256,
/// Copying data in RAM.
MemCopy,
/// Disk sequential write.
DiskSeqWrite,
/// Disk random write.
DiskRndWrite,
}

impl Metric {
/// The category of the metric.
pub fn category(&self) -> &'static str {
match self {
Self::Sr25519Verify | Self::Blake2256 => "CPU",
Self::MemCopy => "Memory",
Self::DiskSeqWrite | Self::DiskRndWrite => "Disk",
}
}

/// The name of the metric. It is always prefixed by the [`self::category()`].
pub fn name(&self) -> &'static str {
match self {
Self::Sr25519Verify => "SR25519-Verify",
Self::Blake2256 => "BLAKE2-256",
Self::MemCopy => "Copy",
Self::DiskSeqWrite => "Seq Write",
Self::DiskRndWrite => "Rnd Write",
}
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
5 changes: 3 additions & 2 deletions utils/frame/benchmarking-cli/src/machine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ use sc_cli::{CliConfiguration, Result, SharedParams};
use sc_service::Configuration;
use sc_sysinfo::{
benchmark_cpu, benchmark_disk_random_writes, benchmark_disk_sequential_writes,
benchmark_memory, benchmark_sr25519_verify, ExecutionLimit, Throughput,
benchmark_memory, benchmark_sr25519_verify, ExecutionLimit, Metric, Requirement, Requirements,
Throughput,
};

use crate::shared::check_build_profile;
pub use hardware::{Metric, Requirement, Requirements, SUBSTRATE_REFERENCE_HARDWARE};
pub use hardware::SUBSTRATE_REFERENCE_HARDWARE;

/// Command to benchmark the hardware.
///
Expand Down