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

☭ Liberate workers! ☭ #7570

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
26 changes: 21 additions & 5 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 10 additions & 12 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,6 @@
name = "polkadot"
path = "src/main.rs"

[[bin]]
name = "polkadot-execute-worker"
path = "src/bin/execute-worker.rs"

[[bin]]
name = "polkadot-prepare-worker"
path = "src/bin/prepare-worker.rs"

[package]
name = "polkadot"
description = "Implementation of a `https://polkadot.network` node in Rust based on the Substrate framework."
Expand Down Expand Up @@ -38,24 +30,24 @@ polkadot-node-core-pvf = { path = "node/core/pvf" }
polkadot-node-core-pvf-prepare-worker = { path = "node/core/pvf/prepare-worker" }
polkadot-overseer = { path = "node/overseer" }

# Needed for worker binaries.
polkadot-node-core-pvf-common = { path = "node/core/pvf/common" }
polkadot-node-core-pvf-execute-worker = { path = "node/core/pvf/execute-worker" }

[dev-dependencies]
assert_cmd = "2.0.4"
nix = { version = "0.26.1", features = ["signal"] }
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" }
polkadot-execute-worker = { path = "bin/execute-worker" }
polkadot-prepare-worker = { path = "bin/prepare-worker" }

[build-dependencies]
substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" }

[workspace]
members = [
"cli",
"bin/execute-worker",
"bin/prepare-worker",
"core-primitives",
"erasure-coding",
"erasure-coding/fuzzer",
Expand Down Expand Up @@ -142,6 +134,12 @@ members = [
"utils/generate-bags",
]

default-members = [
".",
"bin/execute-worker",
"bin/prepare-worker",
]

[badges]
maintenance = { status = "actively-developed" }

Expand Down
16 changes: 16 additions & 0 deletions bin/execute-worker/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[package]
name = "polkadot-execute-worker"
version.workspace = true
authors.workspace = true
edition.workspace = true

[[bin]]
name = "polkadot-execute-worker"
path = "src/main.rs"

[dependencies]
polkadot-node-core-pvf-common = { path = "../../node/core/pvf/common" }
polkadot-node-core-pvf-execute-worker = { path = "../../node/core/pvf/execute-worker" }

[build-dependencies]
substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" }
22 changes: 22 additions & 0 deletions bin/execute-worker/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright (C) Parity Technologies (UK) Ltd.
// This file is part of Polkadot.

// Polkadot is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Polkadot is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

fn main() {
substrate_build_script_utils::generate_cargo_keys();
// For the node/worker version check, make sure we always rebuild the node and binary workers
// when the version changes.
substrate_build_script_utils::rerun_if_git_head_changed();
}
18 changes: 18 additions & 0 deletions bin/execute-worker/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (C) Parity Technologies (UK) Ltd.
// This file is part of Polkadot.

// Polkadot is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Polkadot is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

/// Execute worker binary name
pub const BINARY_NAME: &str = "polkadot-execute-worker";
33 changes: 33 additions & 0 deletions bin/execute-worker/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright (C) Parity Technologies (UK) Ltd.
// This file is part of Polkadot.

// Polkadot is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Polkadot is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

//! Execute worker.

#[cfg(test)]
use polkadot_execute_worker::BINARY_NAME;

polkadot_node_core_pvf_common::decl_worker_main!(
"execute-worker",
polkadot_node_core_pvf_execute_worker::worker_entrypoint,
env!("SUBSTRATE_CLI_IMPL_VERSION")
);

#[test]
fn name_test() {
// If binary name ever needs to change, it must be changed in the package manifest
// as well as in the constant value
assert_eq!(env!("CARGO_BIN_NAME"), BINARY_NAME);
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

//! Execute worker.
//! Dummy integration test needed for the binary to get build when other intergation
//! tests are run

polkadot_node_core_pvf_common::decl_worker_main!(
"execute-worker",
polkadot_node_core_pvf_execute_worker::worker_entrypoint,
env!("SUBSTRATE_CLI_IMPL_VERSION")
);
#[test]
fn dummy_test() {
assert!(true);
}
16 changes: 16 additions & 0 deletions bin/prepare-worker/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[package]
name = "polkadot-prepare-worker"
version.workspace = true
authors.workspace = true
edition.workspace = true

[[bin]]
name = "polkadot-prepare-worker"
path = "src/main.rs"

[dependencies]
polkadot-node-core-pvf-common = { path = "../../node/core/pvf/common" }
polkadot-node-core-pvf-prepare-worker = { path = "../../node/core/pvf/prepare-worker" }

[build-dependencies]
substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" }
22 changes: 22 additions & 0 deletions bin/prepare-worker/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright (C) Parity Technologies (UK) Ltd.
// This file is part of Polkadot.

// Polkadot is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Polkadot is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

fn main() {
substrate_build_script_utils::generate_cargo_keys();
// For the node/worker version check, make sure we always rebuild the node and binary workers
// when the version changes.
substrate_build_script_utils::rerun_if_git_head_changed();
}
18 changes: 18 additions & 0 deletions bin/prepare-worker/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (C) Parity Technologies (UK) Ltd.
// This file is part of Polkadot.

// Polkadot is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Polkadot is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

/// Prepare worker binary name
pub const BINARY_NAME: &str = "polkadot-prepare-worker";
Comment on lines +17 to +18
Copy link
Contributor

Choose a reason for hiding this comment

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

It's the same as PREPARE_BINARY_NAME in pvf-host. Can we move it to the pvf/prepare-worker crate, and use that here, for less duplication?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm there is an issue with the PVF host depending on the worker binaries in this way. The worker binaries will be recompiled after every commit which will be annoying when hacking on pvf crate. I think we can move the binary names into pvf-common.

33 changes: 33 additions & 0 deletions bin/prepare-worker/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright (C) Parity Technologies (UK) Ltd.
// This file is part of Polkadot.

// Polkadot is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Polkadot is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

//! Prepare worker.

#[cfg(test)]
use polkadot_prepare_worker::BINARY_NAME;

polkadot_node_core_pvf_common::decl_worker_main!(
"prepare-worker",
polkadot_node_core_pvf_prepare_worker::worker_entrypoint,
env!("SUBSTRATE_CLI_IMPL_VERSION")
);

#[test]
fn name_test() {
// If binary name ever needs to change, it must be changed in the package manifest
// as well as in the constant value
assert_eq!(env!("CARGO_BIN_NAME"), BINARY_NAME);
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

//! Prepare worker.
//! Dummy integration test needed for the binary to get build when other intergation
//! tests are run

polkadot_node_core_pvf_common::decl_worker_main!(
"prepare-worker",
polkadot_node_core_pvf_prepare_worker::worker_entrypoint,
env!("SUBSTRATE_CLI_IMPL_VERSION")
);
#[test]
fn dummy_test() {
assert!(true);
}
3 changes: 3 additions & 0 deletions node/core/pvf/prepare-worker/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ sp-io = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-maybe-compressed-blob = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master" }

[build-dependencies]
substrate-build-script-utils = { git = "https://github.com/paritytech/substrate", branch = "master" }

[target.'cfg(target_os = "linux")'.dependencies]
tikv-jemalloc-ctl = "0.5.0"

Expand Down
18 changes: 0 additions & 18 deletions node/malus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,6 @@ publish = false
name = "malus"
path = "src/malus.rs"

# Use artifact dependencies once stable.
# See https://github.com/rust-lang/cargo/issues/9096.
[[bin]]
name = "polkadot-execute-worker"
path = "../../src/bin/execute-worker.rs"
# Prevent rustdoc error. Already documented from top-level Cargo.toml.
doc = false
[[bin]]
name = "polkadot-prepare-worker"
path = "../../src/bin/prepare-worker.rs"
# Prevent rustdoc error. Already documented from top-level Cargo.toml.
doc = false

[dependencies]
polkadot-cli = { path = "../../cli", features = [ "malus", "rococo-native", "kusama-native", "westend-native", "polkadot-native" ] }
polkadot-node-subsystem = { path = "../subsystem" }
Expand All @@ -47,11 +34,6 @@ gum = { package = "tracing-gum", path = "../gum/" }
erasure = { package = "polkadot-erasure-coding", path = "../../erasure-coding" }
rand = "0.8.5"

# Required for worker binaries to build.
polkadot-node-core-pvf-common = { path = "../core/pvf/common" }
polkadot-node-core-pvf-execute-worker = { path = "../core/pvf/execute-worker" }
polkadot-node-core-pvf-prepare-worker = { path = "../core/pvf/prepare-worker" }

[dev-dependencies]
polkadot-node-subsystem-test-helpers = { path = "../subsystem-test-helpers" }
sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
Expand Down
2 changes: 1 addition & 1 deletion scripts/ci/gitlab/pipeline/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ build-malus:
- .compiler-info
- .collect-artifacts
script:
- time cargo build --locked --profile testnet --verbose -p polkadot-test-malus
- time cargo build --locked --profile testnet --verbose -p polkadot-test-malus -p polkadot-execute-worker -p polkadot-prepare-worker
Copy link
Contributor Author

@s0me0ne-unkn0wn s0me0ne-unkn0wn Aug 2, 2023

Choose a reason for hiding this comment

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

I tried different approaches and ended up explicitly building workers for malus.

This one even works but has some flaws and looks too hacky to end up in the production code.

Any better ideas are appreciated.

Copy link
Contributor

@mrcnski mrcnski Aug 4, 2023

Choose a reason for hiding this comment

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

Is it possible to have a separate workspace for malus, then build it the exact same way we do polkadot? i.e. if we list the workers as default-members, we don't have to specify them with -p for each build command.

I think this would work if the polkadot workspace didn't list malus, and the malus Cargo.toml depended on polkadot and listed the workers as workspace members and default-members. Worth a try at least.

This one even works but has some flaws and looks too hacky to end up in the production code.

Yeah that looks too hacky to me, but -p .. is fine if nothing else works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it possible to have a separate workspace for malus

Unfortunately, no, nested workspaces are proposed but not supported yet. I went through a lot of interesting proposals while I was researching possible solutions, like intersecting workspaces and binary dependencies, but neither of them is implemented now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I went through a lot of interesting proposals while I was researching possible solutions ... but neither of them is implemented now.

Yeah I know the feeling. I think -p .. is fine, maybe just update the readme with build instructions like we did for node metrics.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've found out that cargo test --workspace --profile testnet triggers building some auxiliary binaries like adder-collator and undying, which are required for tests residing in tests/. But I still need to understand what mechanism triggers those builds. Probably it can be used for malus as well.

# pack artifacts
- mkdir -p ./artifacts
- mv ./target/testnet/malus ./artifacts/.
Expand Down
Loading
Loading