Skip to content

Commit

Permalink
Digest function now auto-detected from request
Browse files Browse the repository at this point in the history 
The digest function the client requested is now properly implemented
in the OriginContext. This allows us to inspect what digest-function
the client requested from anywhere in the code.
  • Loading branch information
@allada
allada committed May 7, 2024
1 parent 829904e commit 6c33e74
Show file tree
Hide file tree
Showing 31 changed files with 692 additions and 392 deletions.
6 changes: 3 additions & 3 deletions .bazelrc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ build --test_output=errors

build:debug -c dbg

build:self_test --remote_instance_name=main
#build:self_test --remote_instance_name=main
build:self_test --remote_cache=grpc://127.0.0.1:50051

build:self_execute --remote_executor=grpc://127.0.0.1:50052
build:self_execute --remote_instance_name=main
build:self_execute --remote_executor=grpc://127.0.0.1:50051
#build:self_execute --remote_instance_name=main
build:self_execute --remote_default_exec_properties=cpu_count=1
build:self_execute --platform_suffix=self-execute

Expand Down
41 changes: 27 additions & 14 deletions nativelink-config/examples/basic_cas.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,40 @@
}
}
},
"CAS_STORE": {
"verify": {
"backend": {
"filesystem": {
"content_path": "/tmp/nativelink/data-worker-test/content_path-cas",
"temp_path": "/tmp/nativelink/data-worker-test/tmp_path-cas",
"eviction_policy": {
// 10gb.
"max_bytes": 10000000000,
}
}
},
"verify_size": true,
"verify_hash": true
}
},
"WORKER_FAST_SLOW_STORE": {
"fast_slow": {
// "fast" must be a "filesystem" store because the worker uses it to make
// hardlinks on disk to a directory where the jobs are running.
"fast": {
"filesystem": {
"content_path": "/tmp/nativelink/data-worker-test/content_path-cas",
"temp_path": "/tmp/nativelink/data-worker-test/tmp_path-cas",
"content_path": "/tmp/nativelink/data-worker-test/content_path-worker",
"temp_path": "/tmp/nativelink/data-worker-test/tmp_path-worker",
"eviction_policy": {
// 10gb.
"max_bytes": 10000000000,
}
}
},
"slow": {
/// Discard data.
/// This example usage has the CAS and the Worker live in the same place,
/// so they share the same underlying CAS. Since workers require a fast_slow
/// store, we use the fast store as our primary data store, and the slow store
/// is just a noop, since there's no shared storage in this config.
"noop": {}
"ref_store": {
"name": "CAS_STORE"
}
}
}
}
Expand Down Expand Up @@ -108,31 +121,31 @@
},
"services": {
"cas": {
"main": {
"cas_store": "WORKER_FAST_SLOW_STORE"
"": {
"cas_store": "CAS_STORE"
}
},
"ac": {
"main": {
"": {
"ac_store": "AC_MAIN_STORE"
}
},
"execution": {
"main": {
"": {
"cas_store": "WORKER_FAST_SLOW_STORE",
"scheduler": "MAIN_SCHEDULER",
}
},
"capabilities": {
"main": {
"": {
"remote_execution": {
"scheduler": "MAIN_SCHEDULER",
}
}
},
"bytestream": {
"cas_stores": {
"main": "WORKER_FAST_SLOW_STORE",
"": "CAS_STORE",
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions nativelink-config/src/stores.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,13 +368,13 @@ pub struct VerifyStore {
#[serde(default)]
pub verify_size: bool,

/// The digest hash function to hash the contents and to verify if the digest hash is
/// matching before writing the entry to underlying store.
///
/// If None, the hash verification will be disabled.
/// If the data should be hashed and verify that the key matches the
/// computed hash. The hash function is automatically determined based
/// request and if not set will use the global default.
///
/// This should be set to None for AC, but hashing function like `sha256` for CAS stores.
pub hash_verification_function: Option<ConfigDigestHashFunction>,
#[serde(default)]
pub verify_hash: bool,
}

#[derive(Serialize, Deserialize, Debug, Clone)]
Expand Down
12 changes: 11 additions & 1 deletion nativelink-proto/com/github/trace_machina/nativelink/remote_execution/worker_api.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,16 @@ message ExecuteResult {
/// are running or cached.
uint64 salt = 3;

// The digest function that was used to compute the action digest
// and all related blobs.
//
// If the digest function used is one of MD5, MURMUR3, SHA1, SHA256,
// SHA384, SHA512, or VSO, the client MAY leave this field unset. In
// that case the server SHOULD infer the digest function using the
// length of the action digest hash and the digest functions announced
// in the server's capabilities.
build.bazel.remote.execution.v2.DigestFunction.Value digest_function = 7;

/// The actual response data.
oneof result {
/// Result of the execution. See `build.bazel.remote.execution.v2.ExecuteResponse`
Expand All @@ -121,7 +131,7 @@ message ExecuteResult {
google.rpc.Status internal_error = 5;
}

reserved 7; // NextId.
reserved 8; // NextId.
}

/// Result sent back from the server when a node connects.
Expand Down
2 changes: 1 addition & 1 deletion nativelink-proto/gen_lib_rs_tool.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
// *** DO NOT MODIFY ***
// This file is auto-generated. To update it, run:
// `bazel run proto:update_protos`
// `bazel run nativelink-proto:update_protos`
"""


Expand Down
13 changes: 13 additions & 0 deletions nativelink-proto/genproto/com.github.trace_machina.nativelink.remote_execution.pb.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,19 @@ pub struct ExecuteResult {
/// / are running or cached.
#[prost(uint64, tag = "3")]
pub salt: u64,
/// The digest function that was used to compute the action digest
/// and all related blobs.
///
/// If the digest function used is one of MD5, MURMUR3, SHA1, SHA256,
/// SHA384, SHA512, or VSO, the client MAY leave this field unset. In
/// that case the server SHOULD infer the digest function using the
/// length of the action digest hash and the digest functions announced
/// in the server's capabilities.
#[prost(
enumeration = "super::super::super::super::super::build::bazel::remote::execution::v2::digest_function::Value",
tag = "7"
)]
pub digest_function: i32,
/// / The actual response data.
#[prost(oneof = "execute_result::Result", tags = "4, 5")]
pub result: ::core::option::Option<execute_result::Result>,
Expand Down
2 changes: 1 addition & 1 deletion nativelink-proto/genproto/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

// *** DO NOT MODIFY ***
// This file is auto-generated. To update it, run:
// `bazel run proto:update_protos`
// `bazel run nativelink-proto:update_protos`

pub mod build {
pub mod bazel {
Expand Down
2 changes: 1 addition & 1 deletion nativelink-proto/update_protos.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def check(proto_packages):
failed = True

if failed:
print("To update, run: 'bazel run proto:update_protos'")
print("To update, run: 'bazel run nativelink-proto:update_protos'")
raise SystemExit(1)


Expand Down
9 changes: 5 additions & 4 deletions nativelink-scheduler/tests/action_messages_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ mod action_messages_tests {
let action_state = ActionState {
unique_qualifier: ActionInfoHashKey {
instance_name: "foo_instance".to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: DigestInfo::new([1u8; 32], 5),
salt: 0,
},
Expand Down Expand Up @@ -119,11 +120,11 @@ mod action_messages_tests {
insert_timestamp: SystemTime::UNIX_EPOCH,
unique_qualifier: ActionInfoHashKey {
instance_name: INSTANCE_NAME.to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: DigestInfo::new([0u8; 32], 0),
salt: 0,
},
skip_cache_lookup: true,
digest_function: DigestHasherFunc::Sha256,
});
let lowest_priority_action = Arc::new(ActionInfo {
command_digest: DigestInfo::new([0u8; 32], 0),
Expand All @@ -137,11 +138,11 @@ mod action_messages_tests {
insert_timestamp: SystemTime::UNIX_EPOCH,
unique_qualifier: ActionInfoHashKey {
instance_name: INSTANCE_NAME.to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: DigestInfo::new([1u8; 32], 0),
salt: 0,
},
skip_cache_lookup: true,
digest_function: DigestHasherFunc::Sha256,
});
let mut action_set = BTreeSet::<Arc<ActionInfo>>::new();
action_set.insert(lowest_priority_action.clone());
Expand Down Expand Up @@ -175,11 +176,11 @@ mod action_messages_tests {
insert_timestamp: SystemTime::UNIX_EPOCH,
unique_qualifier: ActionInfoHashKey {
instance_name: INSTANCE_NAME.to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: DigestInfo::new([0u8; 32], 0),
salt: 0,
},
skip_cache_lookup: true,
digest_function: DigestHasherFunc::Sha256,
});
let current_action = Arc::new(ActionInfo {
command_digest: DigestInfo::new([0u8; 32], 0),
Expand All @@ -193,11 +194,11 @@ mod action_messages_tests {
insert_timestamp: make_system_time(0),
unique_qualifier: ActionInfoHashKey {
instance_name: INSTANCE_NAME.to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: DigestInfo::new([1u8; 32], 0),
salt: 0,
},
skip_cache_lookup: true,
digest_function: DigestHasherFunc::Sha256,
});
let mut action_set = BTreeSet::<Arc<ActionInfo>>::new();
action_set.insert(current_action.clone());
Expand Down
2 changes: 2 additions & 0 deletions nativelink-scheduler/tests/cache_lookup_scheduler_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use nativelink_scheduler::platform_property_manager::PlatformPropertyManager;
use nativelink_store::memory_store::MemoryStore;
use nativelink_util::action_messages::{ActionInfoHashKey, ActionResult, ActionStage, ActionState};
use nativelink_util::common::DigestInfo;
use nativelink_util::digest_hasher::DigestHasherFunc;
use nativelink_util::store_trait::Store;
use prost::Message;
use tokio::sync::watch;
Expand Down Expand Up @@ -115,6 +116,7 @@ mod cache_lookup_scheduler_tests {
let context = make_cache_scheduler()?;
let action_name = ActionInfoHashKey {
instance_name: "instance".to_string(),
digest_function: DigestHasherFunc::Sha256.proto_digest_func().try_into()?,
digest: DigestInfo::new([8; 32], 1),
salt: 1000,
};
Expand Down
2 changes: 2 additions & 0 deletions nativelink-scheduler/tests/property_modifier_scheduler_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ fn make_modifier_scheduler(modifications: Vec<PropertyModification>) -> TestCont

#[cfg(test)]
mod property_modifier_scheduler_tests {
use nativelink_util::digest_hasher::DigestHasherFunc;
use pretty_assertions::assert_eq;

use super::*; // Must be declared in every module.
Expand Down Expand Up @@ -247,6 +248,7 @@ mod property_modifier_scheduler_tests {
let context = make_modifier_scheduler(vec![]);
let action_name = ActionInfoHashKey {
instance_name: "instance".to_string(),
digest_function: DigestHasherFunc::Sha256.proto_digest_func().try_into()?,
digest: DigestInfo::new([8; 32], 1),
salt: 1000,
};
Expand Down
13 changes: 13 additions & 0 deletions nativelink-scheduler/tests/simple_scheduler_test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ async fn setup_action(

#[cfg(test)]
mod scheduler_tests {
use nativelink_util::digest_hasher::DigestHasherFunc;
use pretty_assertions::assert_eq;

use super::*; // Must be declared in every module.
Expand Down Expand Up @@ -251,6 +252,7 @@ mod scheduler_tests {

let unique_qualifier = ActionInfoHashKey {
instance_name: "".to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: DigestInfo::zero_digest(),
salt: 0,
};
Expand Down Expand Up @@ -532,6 +534,7 @@ mod scheduler_tests {
let mut expected_action_state = ActionState {
unique_qualifier: ActionInfoHashKey {
instance_name: "".to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: DigestInfo::zero_digest(),
salt: 0,
}, // Will be filled later.
Expand Down Expand Up @@ -691,6 +694,7 @@ mod scheduler_tests {
// Name is a random string, so we ignore it and just make it the same.
unique_qualifier: ActionInfoHashKey {
instance_name: "".to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: DigestInfo::zero_digest(),
salt: 0,
},
Expand Down Expand Up @@ -793,6 +797,7 @@ mod scheduler_tests {

let action_info_hash_key = ActionInfoHashKey {
instance_name: INSTANCE_NAME.to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: action_digest,
salt: 0,
};
Expand Down Expand Up @@ -898,6 +903,7 @@ mod scheduler_tests {

let action_info_hash_key = ActionInfoHashKey {
instance_name: INSTANCE_NAME.to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: action_digest,
salt: 0,
};
Expand Down Expand Up @@ -999,6 +1005,7 @@ mod scheduler_tests {

let action_info_hash_key = ActionInfoHashKey {
instance_name: INSTANCE_NAME.to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: action_digest,
salt: 0,
};
Expand Down Expand Up @@ -1074,6 +1081,7 @@ mod scheduler_tests {
let mut expected_action_state = ActionState {
unique_qualifier: ActionInfoHashKey {
instance_name: "".to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: DigestInfo::zero_digest(),
salt: 0,
}, // Will be filled later.
Expand Down Expand Up @@ -1148,6 +1156,7 @@ mod scheduler_tests {
&WORKER_ID,
&ActionInfoHashKey {
instance_name: INSTANCE_NAME.to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: action_digest,
salt: 0,
},
Expand Down Expand Up @@ -1265,6 +1274,7 @@ mod scheduler_tests {
&WORKER_ID,
&ActionInfoHashKey {
instance_name: INSTANCE_NAME.to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: action_digest1,
salt: 0,
},
Expand Down Expand Up @@ -1310,6 +1320,7 @@ mod scheduler_tests {
&WORKER_ID,
&ActionInfoHashKey {
instance_name: INSTANCE_NAME.to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: action_digest2,
salt: 0,
},
Expand Down Expand Up @@ -1422,6 +1433,7 @@ mod scheduler_tests {

let action_info_hash_key = ActionInfoHashKey {
instance_name: INSTANCE_NAME.to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: action_digest,
salt: 0,
};
Expand Down Expand Up @@ -1581,6 +1593,7 @@ mod scheduler_tests {
&WORKER_ID1,
&ActionInfoHashKey {
instance_name: INSTANCE_NAME.to_string(),
digest_function: DigestHasherFunc::Sha256,
digest: action_digest,
salt: 0,
},
Expand Down
Loading

0 comments on commit 6c33e74

Please sign in to comment.