Skip to content

Commit

Permalink
Add ProcessResultMetadata as a sibling to ExecutedActionMetadata
Browse files Browse the repository at this point in the history
# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano committed Feb 24, 2021
1 parent e7515e8 commit 6185203
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 17 deletions.
22 changes: 12 additions & 10 deletions src/rust/engine/process_execution/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,18 +193,20 @@ impl CommandRunner {
let stdout_digest = result.stdout_digest;
let stderr_digest = result.stderr_digest;

let action_result = remexec::ActionResult {
exit_code: result.exit_code,
output_directories: vec![remexec::OutputDirectory {
path: String::new(),
tree_digest: Some((&result.output_directory).into()),
}],
stdout_digest: Some((&stdout_digest).into()),
stderr_digest: Some((&stderr_digest).into()),
execution_metadata: Some(result.metadata.clone().into()),
..remexec::ActionResult::default()
};
let execute_response = remexec::ExecuteResponse {
cached_result: true,
result: Some(remexec::ActionResult {
exit_code: result.exit_code,
output_directories: vec![remexec::OutputDirectory {
path: String::new(),
tree_digest: Some((&result.output_directory).into()),
}],
stdout_digest: Some((&stdout_digest).into()),
stderr_digest: Some((&stderr_digest).into()),
..remexec::ActionResult::default()
}),
result: Some(action_result),
..remexec::ExecuteResponse::default()
};

Expand Down
48 changes: 46 additions & 2 deletions src/rust/engine/process_execution/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@
extern crate derivative;

use async_trait::async_trait;
use bazel_protos::gen::build::bazel::remote::execution::v2 as remexec;
pub use log::Level;
use remexec::ExecutedActionMetadata;
use serde::{Deserialize, Serialize};
use std::collections::{BTreeMap, BTreeSet};
use std::convert::TryFrom;
Expand Down Expand Up @@ -65,6 +67,7 @@ pub mod named_caches;
extern crate uname;

pub use crate::named_caches::{CacheDest, CacheName, NamedCaches};
use concrete_time::TimeSpan;
use fs::RelativePath;

#[derive(PartialOrd, Ord, Clone, Copy, Debug, Eq, PartialEq, Hash, Serialize, Deserialize)]
Expand Down Expand Up @@ -341,13 +344,54 @@ pub struct ProcessMetadata {
///
/// The result of running a process.
///
#[derive(Clone, Debug, Eq, PartialEq)]
#[derive(Derivative, Clone, Debug, Eq)]
#[derivative(PartialEq, Hash)]
pub struct FallibleProcessResultWithPlatform {
pub stdout_digest: Digest,
pub stderr_digest: Digest,
pub exit_code: i32,
pub platform: Platform,
pub output_directory: hashing::Digest,
pub platform: Platform,
#[derivative(PartialEq = "ignore", Hash = "ignore")]
pub metadata: ProcessResultMetadata,
}

/// Metadata for a ProcessResult corresponding to the REAPI `ExecutedActionMetadata` proto.
#[derive(Clone, Debug, Default, Eq, PartialEq)]
pub struct ProcessResultMetadata {
pub execution_time: Option<TimeSpan>,
}

impl From<ExecutedActionMetadata> for ProcessResultMetadata {
fn from(metadata: ExecutedActionMetadata) -> Self {
let execution_time = match (
metadata.execution_start_timestamp,
metadata.execution_completed_timestamp,
) {
(Some(started), Some(completed)) => {
TimeSpan::from_start_and_end(&started, &completed, "").ok()
}
_ => None,
};
Self { execution_time }
}
}

impl Into<ExecutedActionMetadata> for ProcessResultMetadata {
fn into(self) -> ExecutedActionMetadata {
let (exec_start, exec_end) = match self.execution_time {
Some(timespan) => {
let (start, end) = timespan.as_timestamps();
(Some(start), Some(end))
}
None => (None, None),
};
ExecutedActionMetadata {
execution_start_timestamp: exec_start,
execution_completed_timestamp: exec_end,
..ExecutedActionMetadata::default()
}
}
}

#[derive(Clone)]
Expand Down
3 changes: 3 additions & 0 deletions src/rust/engine/process_execution/src/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use workunit_store::Metric;

use crate::{
Context, FallibleProcessResultWithPlatform, MultiPlatformProcess, NamedCaches, Platform, Process,
ProcessResultMetadata,
};

pub const USER_EXECUTABLE_MODE: u32 = 0o100755;
Expand Down Expand Up @@ -579,6 +580,7 @@ pub trait CapturedWorkdir {
exit_code: child_results.exit_code,
output_directory: output_snapshot.digest,
platform,
metadata: ProcessResultMetadata::default(),
})
}
Err(msg) if msg == "deadline has elapsed" => {
Expand All @@ -594,6 +596,7 @@ pub trait CapturedWorkdir {
exit_code: -libc::SIGTERM,
output_directory: hashing::EMPTY_DIGEST,
platform,
metadata: ProcessResultMetadata::default(),
})
}
Err(msg) => Err(msg),
Expand Down
10 changes: 7 additions & 3 deletions src/rust/engine/process_execution/src/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use workunit_store::{

use crate::{
Context, FallibleProcessResultWithPlatform, MultiPlatformProcess, Platform, Process,
ProcessCacheScope, ProcessMetadata,
ProcessCacheScope, ProcessMetadata, ProcessResultMetadata,
};
use grpc_util::headers_to_interceptor_fn;

Expand Down Expand Up @@ -1099,6 +1099,7 @@ pub async fn populate_fallible_execution_result_for_timeout(
exit_code: -libc::SIGTERM,
output_directory: hashing::EMPTY_DIGEST,
platform,
metadata: ProcessResultMetadata::default(),
})
}

Expand All @@ -1115,7 +1116,6 @@ pub fn populate_fallible_execution_result(
platform: Platform,
treat_tree_digest_as_final_directory_hack: bool,
) -> BoxFuture<Result<FallibleProcessResultWithPlatform, String>> {
let exit_code = action_result.exit_code;
future::try_join3(
extract_stdout(&store, action_result),
extract_stderr(&store, action_result),
Expand All @@ -1130,9 +1130,13 @@ pub fn populate_fallible_execution_result(
Ok(FallibleProcessResultWithPlatform {
stdout_digest,
stderr_digest,
exit_code,
exit_code: action_result.exit_code,
output_directory,
platform,
metadata: action_result
.execution_metadata
.clone()
.map_or(ProcessResultMetadata::default(), |metadata| metadata.into()),
})
},
)
Expand Down
1 change: 1 addition & 0 deletions src/rust/engine/process_execution/src/remote_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ impl CommandRunner {
exit_code: result.exit_code,
stdout_digest: Some(result.stdout_digest.into()),
stderr_digest: Some(result.stderr_digest.into()),
execution_metadata: Some(result.metadata.clone().into()),
..ActionResult::default()
};

Expand Down
6 changes: 4 additions & 2 deletions src/rust/engine/process_execution/src/remote_cache_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use workunit_store::WorkunitStore;
use crate::remote::{ensure_action_stored_locally, make_execute_request};
use crate::{
CommandRunner as CommandRunnerTrait, Context, FallibleProcessResultWithPlatform,
MultiPlatformProcess, Platform, Process, ProcessMetadata,
MultiPlatformProcess, Platform, Process, ProcessMetadata, ProcessResultMetadata,
};

/// A mock of the local runner used for better hermeticity of the tests.
Expand All @@ -45,6 +45,7 @@ impl MockLocalCommandRunner {
exit_code,
output_directory: EMPTY_DIGEST,
platform: Platform::current().unwrap(),
metadata: ProcessResultMetadata::default(),
}),
call_counter,
delay: Duration::from_millis(delay_ms),
Expand Down Expand Up @@ -560,9 +561,10 @@ async fn make_action_result_basic() {
let process_result = FallibleProcessResultWithPlatform {
stdout_digest: TestData::roland().digest(),
stderr_digest: TestData::robin().digest(),
output_directory: directory_digest,
exit_code: 102,
platform: Platform::Linux,
output_directory: directory_digest,
metadata: ProcessResultMetadata::default(),
};

let (action_result, digests) = runner
Expand Down

0 comments on commit 6185203

Please sign in to comment.