Skip to content

Commit 5595963

Browse files
committed
Support ignorable platform properties
1 parent 24c637a commit 5595963

File tree

8 files changed

+52
-5
lines changed

8 files changed

+52
-5
lines changed

nativelink-config/examples/basic_cas.json5

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
"container-image": "priority",
6161
"lre-rs": "priority",
6262
ISA: "exact",
63+
InputRootAbsolutePath: "ignore", // used by chromium builds, but we can drop it
6364
},
6465
},
6566
},

nativelink-config/src/schedulers.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ pub enum PropertyType {
5353
/// to cause the scheduler to prefer certain workers over others, but not
5454
/// restrict them based on these values.
5555
Priority,
56+
57+
//// Allows jobs to be requested with said key, but without requiring workers
58+
//// to have that key
59+
Ignore,
5660
}
5761

5862
/// When a worker is being searched for to run a job, this will be used

nativelink-scheduler/src/platform_property_manager.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ impl PlatformPropertyManager {
8888
)),
8989
PropertyType::Exact => Ok(PlatformPropertyValue::Exact(value.to_string())),
9090
PropertyType::Priority => Ok(PlatformPropertyValue::Priority(value.to_string())),
91+
PropertyType::Ignore => Ok(PlatformPropertyValue::Ignore(value.to_string())),
9192
};
9293
}
9394
Err(make_input_err!("Unknown platform property '{}'", key))

nativelink-scheduler/src/worker_capability_index.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,11 @@ impl WorkerCapabilityIndex {
9090
.or_default()
9191
.insert(worker_id.clone());
9292
}
93-
PlatformPropertyValue::Minimum(_) => {
93+
PlatformPropertyValue::Minimum(_) | PlatformPropertyValue::Ignore(_) => {
9494
// Minimum properties are tracked via property_presence only.
9595
// Their actual values are checked at runtime since they're dynamic.
96+
97+
// Ignore properties we just drop
9698
}
9799
}
98100
}
@@ -200,6 +202,7 @@ impl WorkerCapabilityIndex {
200202
}
201203
candidates = Some(internal_candidates);
202204
}
205+
PlatformPropertyValue::Ignore(_) => {}
203206
}
204207
}
205208

nativelink-scheduler/tests/worker_capability_index_test.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,3 +214,25 @@ fn test_priority_property() {
214214
let result = index.find_matching_workers(&props, true);
215215
assert_eq!(result.len(), 2);
216216
}
217+
218+
#[test]
219+
fn test_ignore_property() {
220+
let mut index = WorkerCapabilityIndex::new();
221+
222+
let worker1 = make_worker_id("worker1");
223+
let worker2 = make_worker_id("worker2");
224+
225+
index.add_worker(
226+
&worker1,
227+
&make_properties(&[("foo", PlatformPropertyValue::Priority("high".to_string()))]),
228+
);
229+
index.add_worker(
230+
&worker2,
231+
&make_properties(&[("bar", PlatformPropertyValue::Priority("low".to_string()))]),
232+
);
233+
234+
// Ignore doesn't care if the worker has the property, so both workers with and without it should match
235+
let props = make_properties(&[("foo", PlatformPropertyValue::Ignore("any".to_string()))]);
236+
let result = index.find_matching_workers(&props, true);
237+
assert_eq!(result.len(), 2);
238+
}

nativelink-util/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ rust_test_suite(
103103
"tests/metrics_test.rs",
104104
"tests/operation_id_tests.rs",
105105
"tests/origin_event_test.rs",
106+
"tests/platform_properties_tests.rs",
106107
"tests/proto_stream_utils_test.rs",
107108
"tests/resource_info_test.rs",
108109
"tests/retry_test.rs",

nativelink-util/src/platform_properties.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,11 +106,15 @@ impl From<&PlatformProperties> for ProtoPlatform {
106106
/// TODO(palfrey) In the future this will be used by the scheduler and
107107
/// worker to cause the scheduler to prefer certain workers over others,
108108
/// but not restrict them based on these values.
109+
/// Ignore - Jobs can request this key, but workers do not have to have it. This allows
110+
/// for example the `InputRootAbsolutePath` case for chromium builds, where we can safely
111+
/// ignore it without having to change the worker configs.
109112
#[derive(Eq, PartialEq, Hash, Clone, Ord, PartialOrd, Debug, Serialize, Deserialize)]
110113
pub enum PlatformPropertyValue {
111114
Exact(String),
112115
Minimum(u64),
113116
Priority(String),
117+
Ignore(String),
114118
Unknown(String),
115119
}
116120

@@ -131,17 +135,18 @@ impl PlatformPropertyValue {
131135
// Priority is used to pass info to the worker and not restrict which
132136
// workers can be selected, but might be used to prefer certain workers
133137
// over others.
134-
Self::Priority(_) => true,
138+
Self::Priority(_) | Self::Ignore(_) => true,
135139
// Success exact case is handled above.
136140
Self::Exact(_) | Self::Unknown(_) => false,
137141
}
138142
}
139143

140144
pub fn as_str(&self) -> Cow<'_, str> {
141145
match self {
142-
Self::Exact(value) | Self::Priority(value) | Self::Unknown(value) => {
143-
Cow::Borrowed(value)
144-
}
146+
Self::Exact(value)
147+
| Self::Priority(value)
148+
| Self::Unknown(value)
149+
| Self::Ignore(value) => Cow::Borrowed(value),
145150
Self::Minimum(value) => Cow::Owned(value.to_string()),
146151
}
147152
}
@@ -159,6 +164,7 @@ impl MetricsComponent for PlatformPropertyValue {
159164
Self::Exact(v) => publish!(name, v, kind, help, "exact"),
160165
Self::Minimum(v) => publish!(name, v, kind, help, "minimum"),
161166
Self::Priority(v) => publish!(name, v, kind, help, "priority"),
167+
Self::Ignore(v) => publish!(name, v, kind, help, "ignore"),
162168
Self::Unknown(v) => publish!(name, v, kind, help, "unknown"),
163169
}
164170

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
use nativelink_util::platform_properties::PlatformPropertyValue;
2+
3+
#[test]
4+
fn ignore_properties_match_all() {
5+
let ignore_property = PlatformPropertyValue::Ignore("foo".to_string());
6+
let other_property = PlatformPropertyValue::Exact("bar".to_string());
7+
assert!(ignore_property.is_satisfied_by(&ignore_property));
8+
assert!(ignore_property.is_satisfied_by(&other_property));
9+
}

0 commit comments

Comments
 (0)