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

Commit

Permalink
Address PR review feedback #1
Browse files Browse the repository at this point in the history
Signed-off-by: Andrei Sandu <[email protected]>
  • Loading branch information
sandreim committed Nov 9, 2021
1 parent cf80a1f commit 393fb66
Show file tree
Hide file tree
Showing 11 changed files with 46 additions and 39 deletions.
2 changes: 1 addition & 1 deletion node/collation-generation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ async fn handle_new_activations<Context: SubsystemContext>(
let mut task_sender = sender.clone();
let metrics = metrics.clone();
ctx.spawn(
"collation generation collation builder",
"collation-builder",
Box::pin(async move {
let persisted_validation_data_hash = validation_data.hash();

Expand Down
5 changes: 3 additions & 2 deletions node/core/av-store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,9 @@ impl Error {
fn trace(&self) {
match self {
// don't spam the log with spurious errors
Self::RuntimeApi(_) | Self::Oneshot(_) =>
tracing::debug!(target: LOG_TARGET, err = ?self),
Self::RuntimeApi(_) | Self::Oneshot(_) => {
tracing::debug!(target: LOG_TARGET, err = ?self)
},
// it's worth reporting otherwise
_ => tracing::warn!(target: LOG_TARGET, err = ?self),
}
Expand Down
16 changes: 9 additions & 7 deletions node/core/backing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ impl CandidateBackingJob {
}
};
sender
.send_command(FromJobCommand::Spawn("Backing Validation", bg.boxed()))
.send_command(FromJobCommand::Spawn("backing-validation", bg.boxed()))
.await?;
}

Expand Down Expand Up @@ -900,11 +900,13 @@ impl CandidateBackingJob {
.await;

match confirmation_rx.await {
Err(oneshot::Canceled) =>
tracing::debug!(target: LOG_TARGET, "Dispute coordinator confirmation lost",),
Err(oneshot::Canceled) => {
tracing::debug!(target: LOG_TARGET, "Dispute coordinator confirmation lost",)
},
Ok(ImportStatementsResult::ValidImport) => {},
Ok(ImportStatementsResult::InvalidImport) =>
tracing::warn!(target: LOG_TARGET, "Failed to import statements of validity",),
Ok(ImportStatementsResult::InvalidImport) => {
tracing::warn!(target: LOG_TARGET, "Failed to import statements of validity",)
},
}
}

Expand Down Expand Up @@ -1168,8 +1170,8 @@ impl util::JobTrait for CandidateBackingJob {
type RunArgs = SyncCryptoStorePtr;
type Metrics = Metrics;

const NAME: &'static str = "CandidateBackingJob";
const SUBSYSTEM: &'static str = "candidate_backing";
const NAME: &'static str = "candidate-backing-job";
const SUBSYSTEM: &'static str = "candidate-backing";

fn run<S: SubsystemSender>(
parent: Hash,
Expand Down
4 changes: 2 additions & 2 deletions node/core/bitfield-signing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,8 @@ impl JobTrait for BitfieldSigningJob {
type RunArgs = SyncCryptoStorePtr;
type Metrics = Metrics;

const NAME: &'static str = "BitfieldSigningJob";
const SUBSYSTEM: &'static str = "bitfield_signing";
const NAME: &'static str = "bitfield-signing-job";
const SUBSYSTEM: &'static str = "bitfield-signing";

/// Run a job for the parent block indicated
fn run<S: SubsystemSender>(
Expand Down
2 changes: 1 addition & 1 deletion node/core/provisioner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ impl JobTrait for ProvisioningJob {
type RunArgs = ();
type Metrics = Metrics;

const NAME: &'static str = "ProvisioningJob";
const NAME: &'static str = "provisioning-job";
const SUBSYSTEM: &'static str = "provisioner";

/// Run a job for the parent block indicated
Expand Down
8 changes: 4 additions & 4 deletions node/core/pvf/src/executor_intf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,17 +280,17 @@ impl TaskExecutor {
impl sp_core::traits::SpawnNamed for TaskExecutor {
fn spawn_blocking(
&self,
_: &'static str,
_: &'static str,
_task_name: &'static str,
_subsystem_name: &'static str,
future: futures::future::BoxFuture<'static, ()>,
) {
self.0.spawn_ok(future);
}

fn spawn(
&self,
_: &'static str,
_: &'static str,
_task_name: &'static str,
_subsystem_name: &'static str,
future: futures::future::BoxFuture<'static, ()>,
) {
self.0.spawn_ok(future);
Expand Down
4 changes: 2 additions & 2 deletions node/network/availability-distribution/src/tests/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ impl TestState {
// lock ;-)
let update_tx = tx.clone();
harness.pool.spawn(
"Sending active leaves updates",
"sending-active-leaves-updates",
"",
async move {
for update in updates {
Expand Down Expand Up @@ -309,7 +309,7 @@ fn to_incoming_req(
let (tx, rx): (oneshot::Sender<netconfig::OutgoingResponse>, oneshot::Receiver<_>) =
oneshot::channel();
executor.spawn(
"Message forwarding",
"message-forwarding",
"",
async {
let response = rx.await;
Expand Down
2 changes: 1 addition & 1 deletion node/network/availability-recovery/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,7 @@ where
awaiting: vec![response_sender],
});

if let Err(e) = ctx.spawn("recovery interaction", Box::pin(remote)) {
if let Err(e) = ctx.spawn("recovery-interaction", Box::pin(remote)) {
tracing::warn!(
target: LOG_TARGET,
err = ?e,
Expand Down
8 changes: 4 additions & 4 deletions node/overseer/overseer-gen/examples/dummy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,17 +104,17 @@ struct DummySpawner;
impl SpawnNamed for DummySpawner {
fn spawn_blocking(
&self,
name: &'static str,
_: &'static str,
_task_name: &'static str,
_subsystem_name: &'static str,
_future: futures::future::BoxFuture<'static, ()>,
) {
unimplemented!("spawn blocking {}", name)
}

fn spawn(
&self,
name: &'static str,
_: &'static str,
_task_name: &'static str,
_subsystem_name: &'static str,
_future: futures::future::BoxFuture<'static, ()>,
) {
unimplemented!("spawn {}", name)
Expand Down
20 changes: 12 additions & 8 deletions node/overseer/overseer-gen/proc-macro/src/impl_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,14 +349,18 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream {
#channel_name_rx, #channel_name_unbounded_rx
);
let (signal_tx, signal_rx) = #support_crate ::metered::channel(SIGNAL_CHANNEL_CAPACITY);
let subsystem_string = stringify!(#subsystem_name);

// Generate subsystem name based on overseer field name.
let mut subsystem_string = String::from(stringify!(#subsystem_name));
// Convert owned `snake case` string to a `kebab case` static str.
let subsystem_static_str = Box::leak(subsystem_string.replace("_", "-").into_boxed_str());

let ctx = #subsyste_ctx_name::< #consumes >::new(
signal_rx,
message_rx,
channels_out.clone(),
to_overseer_tx.clone(),
subsystem_string
subsystem_static_str
);

let #subsystem_name: OverseenSubsystem< #consumes > =
Expand All @@ -367,7 +371,7 @@ pub(crate) fn impl_builder(info: &OverseerInfo) -> proc_macro2::TokenStream {
unbounded_meter,
ctx,
#subsystem_name,
subsystem_string,
subsystem_static_str,
&mut running_subsystems,
)?;
)*
Expand Down Expand Up @@ -493,22 +497,22 @@ pub(crate) fn impl_task_kind(info: &OverseerInfo) -> proc_macro2::TokenStream {
/// Task kind to launch.
pub trait TaskKind {
/// Spawn a task, it depends on the implementer if this is blocking or not.
fn launch_task<S: SpawnNamed>(spawner: &mut S, name: &'static str, subsystem: &'static str, future: BoxFuture<'static, ()>);
fn launch_task<S: SpawnNamed>(spawner: &mut S, task_name: &'static str, subsystem_name: &'static str, future: BoxFuture<'static, ()>);
}

#[allow(missing_docs)]
struct Regular;
impl TaskKind for Regular {
fn launch_task<S: SpawnNamed>(spawner: &mut S, name: &'static str, subsystem: &'static str, future: BoxFuture<'static, ()>) {
spawner.spawn(name, subsystem, future)
fn launch_task<S: SpawnNamed>(spawner: &mut S, task_name: &'static str, subsystem_name: &'static str, future: BoxFuture<'static, ()>) {
spawner.spawn(task_name, subsystem_name, future)
}
}

#[allow(missing_docs)]
struct Blocking;
impl TaskKind for Blocking {
fn launch_task<S: SpawnNamed>(spawner: &mut S, name: &'static str, subsystem: &'static str, future: BoxFuture<'static, ()>) {
spawner.spawn(name, subsystem, future)
fn launch_task<S: SpawnNamed>(spawner: &mut S, task_name: &'static str, subsystem_name: &'static str, future: BoxFuture<'static, ()>) {
spawner.spawn(task_name, subsystem_name, future)
}
}

Expand Down
14 changes: 7 additions & 7 deletions node/overseer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ where

futures::future::ready(())
});
overseer.spawner().spawn("metrics_metronome", "overseer", Box::pin(metronome));
overseer.spawner().spawn("metrics-metronome", "overseer", Box::pin(metronome));

Ok(())
}
Expand Down Expand Up @@ -775,19 +775,19 @@ where

fn spawn_job(
&mut self,
name: &'static str,
subsystem: &'static str,
task_name: &'static str,
subsystem_name: &'static str,
j: BoxFuture<'static, ()>,
) {
self.spawner.spawn(name, subsystem, j);
self.spawner.spawn(task_name, subsystem_name, j);
}

fn spawn_blocking_job(
&mut self,
name: &'static str,
subsystem: &'static str,
task_name: &'static str,
subsystem_name: &'static str,
j: BoxFuture<'static, ()>,
) {
self.spawner.spawn_blocking(name, subsystem, j);
self.spawner.spawn_blocking(task_name, subsystem_name, j);
}
}

0 comments on commit 393fb66

Please sign in to comment.