Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve artifact cache unless stale #1918

Merged
merged 41 commits into from
Nov 19, 2023
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
a563e25
make pruning explicit
eagr Oct 17, 2023
f31d354
preserve cache unless stale
eagr Oct 17, 2023
195bbce
barely working
eagr Oct 17, 2023
99aa012
use ArtifactId::from_file_name()
eagr Oct 17, 2023
17973ef
ignore non-unicode file names
eagr Oct 17, 2023
e81b456
generalize concat_const!()
eagr Oct 18, 2023
851a77a
per advices
eagr Oct 18, 2023
a3b0fcf
break on IO error
eagr Oct 18, 2023
b323c28
make pruning sound
eagr Oct 18, 2023
a2808f8
log more events
eagr Oct 18, 2023
0657d41
refactor
eagr Oct 20, 2023
0384ae7
doc
eagr Oct 20, 2023
4b1bb0a
Refactor indentation
mrcnski Oct 21, 2023
a8bcce4
refactor
eagr Oct 21, 2023
50b7ccc
checksum poc
eagr Oct 22, 2023
b6c1a07
Revert "checksum poc"
eagr Oct 30, 2023
2ad5262
redo checksum p1
eagr Oct 30, 2023
3723806
p2
eagr Oct 30, 2023
e44c451
remove corrupted cache
eagr Oct 31, 2023
6c19164
diversify results
eagr Nov 1, 2023
dad5285
fix tests
eagr Nov 1, 2023
1ede201
fix pruning
eagr Nov 1, 2023
2d51b52
fix message serialization
eagr Nov 1, 2023
36a33e8
clean up
eagr Nov 1, 2023
69f6a44
retire path_prefix()
eagr Nov 2, 2023
d3254f5
improve test
eagr Nov 3, 2023
f4d22fa
Merge branch 'master' into preserve-art
mrcnski Nov 12, 2023
39448ff
Fix test
mrcnski Nov 12, 2023
3a2c1cd
cargo fmt
mrcnski Nov 12, 2023
a02cb06
as per advices
eagr Nov 13, 2023
53e4557
tag artifact with runtime version
eagr Nov 13, 2023
d4f3083
fix tests
eagr Nov 13, 2023
9d2142e
Merge branch 'master' into preserve-art
mrcnski Nov 14, 2023
931aae1
upstream build fn to substrate
eagr Nov 15, 2023
5de4e8e
glitch
eagr Nov 15, 2023
a0b71c5
wrong attribution
eagr Nov 15, 2023
c85adfe
as per suggestions
eagr Nov 17, 2023
b897f1c
glitch
eagr Nov 17, 2023
89ada31
prevent `cargo tree` from accessing network
eagr Nov 17, 2023
868426a
glitch
eagr Nov 17, 2023
652bec7
Merge branch 'master' into preserve-art
eagr Nov 19, 2023
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
25 changes: 13 additions & 12 deletions Cargo.lock

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

6 changes: 3 additions & 3 deletions polkadot/node/core/candidate-validation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

use polkadot_node_core_pvf::{
InternalValidationError, InvalidCandidate as WasmInvalidCandidate, PrepareError,
PrepareJobKind, PrepareStats, PvfPrepData, ValidationError, ValidationHost,
PrepareJobKind, PvfPrepData, ValidationError, ValidationHost,
};
use polkadot_node_primitives::{
BlockData, InvalidCandidate, PoV, ValidationResult, POV_BOMB_LIMIT, VALIDATION_CODE_BOMB_LIMIT,
Expand Down Expand Up @@ -788,7 +788,7 @@ trait ValidationBackend {
validation_result
}

async fn precheck_pvf(&mut self, pvf: PvfPrepData) -> Result<PrepareStats, PrepareError>;
async fn precheck_pvf(&mut self, pvf: PvfPrepData) -> Result<(), PrepareError>;
}

#[async_trait]
Expand Down Expand Up @@ -818,7 +818,7 @@ impl ValidationBackend for ValidationHost {
})?
}

async fn precheck_pvf(&mut self, pvf: PvfPrepData) -> Result<PrepareStats, PrepareError> {
async fn precheck_pvf(&mut self, pvf: PvfPrepData) -> Result<(), PrepareError> {
let (tx, rx) = oneshot::channel();
if let Err(err) = self.precheck_pvf(pvf, tx).await {
// Return an IO error if there was an error communicating with the host.
Expand Down
12 changes: 6 additions & 6 deletions polkadot/node/core/candidate-validation/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ impl ValidationBackend for MockValidateCandidateBackend {
result
}

async fn precheck_pvf(&mut self, _pvf: PvfPrepData) -> Result<PrepareStats, PrepareError> {
async fn precheck_pvf(&mut self, _pvf: PvfPrepData) -> Result<(), PrepareError> {
unreachable!()
}
}
Expand Down Expand Up @@ -1012,11 +1012,11 @@ fn pov_decompression_failure_is_invalid() {
}

struct MockPreCheckBackend {
result: Result<PrepareStats, PrepareError>,
result: Result<(), PrepareError>,
}

impl MockPreCheckBackend {
fn with_hardcoded_result(result: Result<PrepareStats, PrepareError>) -> Self {
fn with_hardcoded_result(result: Result<(), PrepareError>) -> Self {
Self { result }
}
}
Expand All @@ -1032,7 +1032,7 @@ impl ValidationBackend for MockPreCheckBackend {
unreachable!()
}

async fn precheck_pvf(&mut self, _pvf: PvfPrepData) -> Result<PrepareStats, PrepareError> {
async fn precheck_pvf(&mut self, _pvf: PvfPrepData) -> Result<(), PrepareError> {
self.result.clone()
}
}
Expand All @@ -1049,7 +1049,7 @@ fn precheck_works() {

let (check_fut, check_result) = precheck_pvf(
ctx.sender(),
MockPreCheckBackend::with_hardcoded_result(Ok(PrepareStats::default())),
MockPreCheckBackend::with_hardcoded_result(Ok(())),
relay_parent,
validation_code_hash,
)
Expand Down Expand Up @@ -1111,7 +1111,7 @@ fn precheck_invalid_pvf_blob_compression() {

let (check_fut, check_result) = precheck_pvf(
ctx.sender(),
MockPreCheckBackend::with_hardcoded_result(Ok(PrepareStats::default())),
MockPreCheckBackend::with_hardcoded_result(Ok(())),
relay_parent,
validation_code_hash,
)
Expand Down
1 change: 1 addition & 0 deletions polkadot/node/core/pvf/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ license.workspace = true

[dependencies]
always-assert = "0.1"
blake3 = "1.5"
cfg-if = "1.0"
futures = "0.3.21"
futures-timer = "3.0.2"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl TestHost {
&self,
code: &[u8],
executor_params: ExecutorParams,
) -> Result<PrepareStats, PrepareError> {
) -> Result<(), PrepareError> {
let (result_tx, result_rx) = futures::channel::oneshot::channel();

let code = sp_maybe_compressed_blob::decompress(code, 16 * 1024 * 1024)
Expand Down
1 change: 1 addition & 0 deletions polkadot/node/core/pvf/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
name = "polkadot-node-core-pvf-common"
description = "Polkadot crate that contains functionality related to PVFs that is shared by the PVF host and the PVF workers."
version = "1.0.0"
build = "build.rs"
mrcnski marked this conversation as resolved.
Show resolved Hide resolved
authors.workspace = true
edition.workspace = true
license.workspace = true
Expand Down
28 changes: 28 additions & 0 deletions polkadot/node/core/pvf/common/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
fn main() {
get_wasmtime_version();
}

pub fn get_wasmtime_version() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a comment that in case there are multiple wasmtime versions being used, is an error. Example:

 $ cargo tree --package=aead --depth=0
error: There are multiple `aead` packages in your project, and the specification `aead` is ambiguous.
Please re-run this command with one of the following specifications:
  aead@0.3.2
  aead@0.4.3
  aead@0.5.2

I think an error in that case great, we definitely don't want to be mixing wasmtime versions anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is somehow captured. Right now this shows a warning and the compilation will fail if they try to refer to the environment variable. Would that be enough?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean a comment would be useful for readers of the code. I was wondering what would happen if there were multiple versions of wasmtime, I didn't know it would be an error until I tried it.

// we only care about the root of the tree
match std::process::Command::new("cargo")
.args(&["tree", "--package=wasmtime", "--depth=0"])
.output()
{
Ok(out) if out.status.success() => {
// wasmtime vX.X.X
let version = String::from_utf8_lossy(&out.stdout);
if let Some(version) = version.strip_prefix("wasmtime v") {
println!("cargo:rustc-env=SUBSTRATE_WASMTIME_VERSION={}", version);
} else {
println!("cargo:warning=build.rs: unexpected result {}", version);
}
},
Ok(out) => println!(
"cargo:warning=build.rs: `cargo tree` {}",
String::from_utf8_lossy(&out.stderr),
),
Err(err) => {
println!("cargo:warning=build.rs: Could not run `cargo tree`: {}", err);
},
}
}
20 changes: 14 additions & 6 deletions polkadot/node/core/pvf/common/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,21 @@
// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

use crate::prepare::PrepareStats;
use crate::prepare::{PrepareSuccess, PrepareWorkerSuccess};
use parity_scale_codec::{Decode, Encode};
use std::fmt;

/// Result of PVF preparation performed by the validation host. Contains stats about the preparation
/// if successful
pub type PrepareResult = Result<PrepareStats, PrepareError>;
/// Result of PVF preparation from a worker, with checksum of the compiled PVF and stats of the
/// preparation if successful.
pub type PrepareWorkerResult = Result<PrepareWorkerSuccess, PrepareError>;

/// Result of PVF preparation propagated all the way back to the host, with path to the concluded
/// artifact and stats of the preparation if successful.
pub type PrepareResult = Result<PrepareSuccess, PrepareError>;

/// Result of prechecking PVF performed by the validation host. Contains stats about the preparation
/// if successful.
pub type PrecheckResult = Result<(), PrepareError>;

/// An error that occurred during the prepare part of the PVF pipeline.
// Codec indexes are intended to stabilize pre-encoded payloads (see `OOM_PAYLOAD` below)
Expand Down Expand Up @@ -70,7 +78,7 @@ pub enum PrepareError {
ClearWorkerDir(String),
}

/// Pre-encoded length-prefixed `PrepareResult::Err(PrepareError::OutOfMemory)`
/// Pre-encoded length-prefixed `PrepareWorkerResult::Err(PrepareError::OutOfMemory)`
pub const OOM_PAYLOAD: &[u8] = b"\x02\x00\x00\x00\x00\x00\x00\x00\x01\x08";

impl PrepareError {
Expand Down Expand Up @@ -167,7 +175,7 @@ impl fmt::Display for InternalValidationError {

#[test]
fn pre_encoded_payloads() {
let oom_enc = PrepareResult::Err(PrepareError::OutOfMemory).encode();
let oom_enc = PrepareWorkerResult::Err(PrepareError::OutOfMemory).encode();
let mut oom_payload = oom_enc.len().to_le_bytes().to_vec();
oom_payload.extend(oom_enc);
assert_eq!(oom_payload, OOM_PAYLOAD);
Expand Down
2 changes: 2 additions & 0 deletions polkadot/node/core/pvf/common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ pub use sp_tracing;

const LOG_TARGET: &str = "parachain::pvf-common";

pub const RUNTIME_VERSION: &str = env!("SUBSTRATE_WASMTIME_VERSION");

use std::{
io::{self, Read, Write},
mem,
Expand Down
19 changes: 19 additions & 0 deletions polkadot/node/core/pvf/common/src/prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,25 @@
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

use parity_scale_codec::{Decode, Encode};
use std::path::PathBuf;

/// Result from prepare worker if successful.
#[derive(Debug, Clone, Default, Encode, Decode)]
pub struct PrepareWorkerSuccess {
/// Checksum of the compiled PVF.
pub checksum: String,
/// Stats of the current preparation run.
pub stats: PrepareStats,
}

/// Result of PVF preparation if successful.
#[derive(Debug, Clone, Default)]
pub struct PrepareSuccess {
/// Canonical path to the compiled artifact.
pub path: PathBuf,
/// Stats of the current preparation run.
pub stats: PrepareStats,
}

/// Preparation statistics, including the CPU time and memory taken.
#[derive(Debug, Clone, Default, Encode, Decode)]
Expand Down
2 changes: 1 addition & 1 deletion polkadot/node/core/pvf/common/src/pvf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl fmt::Debug for PvfPrepData {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(
f,
"Pvf {{ code, code_hash: {:?}, executor_params: {:?}, prep_timeout: {:?} }}",
"Pvf {{ code_hash: {:?}, executor_params: {:?}, prep_timeout: {:?} }}",
mrcnski marked this conversation as resolved.
Show resolved Hide resolved
self.code_hash, self.executor_params, self.prep_timeout
)
}
Expand Down
1 change: 1 addition & 0 deletions polkadot/node/core/pvf/prepare-worker/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ edition.workspace = true
license.workspace = true

[dependencies]
blake3 = "1.5"
cfg-if = "1.0"
gum = { package = "tracing-gum", path = "../../../gum" }
libc = "0.2.139"
Expand Down
17 changes: 11 additions & 6 deletions polkadot/node/core/pvf/prepare-worker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ use crate::memory_stats::max_rss_stat::{extract_max_rss_stat, get_max_rss_thread
use crate::memory_stats::memory_tracker::{get_memory_tracker_loop_stats, memory_tracker_loop};
use parity_scale_codec::{Decode, Encode};
use polkadot_node_core_pvf_common::{
error::{PrepareError, PrepareResult, OOM_PAYLOAD},
error::{PrepareError, PrepareWorkerResult, OOM_PAYLOAD},
executor_intf::create_runtime_from_artifact_bytes,
framed_recv_blocking, framed_send_blocking,
prepare::{MemoryStats, PrepareJobKind, PrepareStats},
prepare::{MemoryStats, PrepareJobKind, PrepareStats, PrepareWorkerSuccess},
pvf::PvfPrepData,
worker::{
cpu_time_monitor_loop, stringify_panic_payload,
Expand Down Expand Up @@ -91,7 +91,7 @@ fn recv_request(stream: &mut UnixStream) -> io::Result<PvfPrepData> {
Ok(pvf)
}

fn send_response(stream: &mut UnixStream, result: PrepareResult) -> io::Result<()> {
fn send_response(stream: &mut UnixStream, result: PrepareWorkerResult) -> io::Result<()> {
framed_send_blocking(stream, &result.encode())
}

Expand Down Expand Up @@ -165,8 +165,8 @@ fn end_memory_tracking() -> isize {
///
/// 6. If compilation succeeded, write the compiled artifact into a temporary file.
///
/// 7. Send the result of preparation back to the host. If any error occurred in the above steps, we
/// send that in the `PrepareResult`.
/// 7. Send the result of preparation back to the host, including the checksum of the artifact. If
/// any error occurred in the above steps, we send that in the `PrepareWorkerResult`.
mrcnski marked this conversation as resolved.
Show resolved Hide resolved
pub fn worker_entrypoint(
socket_path: PathBuf,
worker_dir_path: PathBuf,
Expand Down Expand Up @@ -341,7 +341,12 @@ pub fn worker_entrypoint(
);
fs::write(&temp_artifact_dest, &artifact)?;

Ok(PrepareStats { cpu_time_elapsed, memory_stats })
let checksum =
blake3::hash(&artifact.as_ref()).to_hex().to_string();
Ok(PrepareWorkerSuccess {
checksum,
stats: PrepareStats { cpu_time_elapsed, memory_stats },
})
},
}
},
Expand Down
Loading
Loading