Skip to content

Commit 3e6b6cb

Browse files
JamesbarfordJamesbarford
committed
Allow for optional jobs not interfering which benchmark completion
1 parent ac90432 commit 3e6b6cb

File tree

6 files changed

+170
-10
lines changed

6 files changed

+170
-10
lines changed

‎database/src/lib.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,6 +1164,7 @@ pub struct BenchmarkJob {
11641164
status: BenchmarkJobStatus,
11651165
deque_counter: u32,
11661166
kind: BenchmarkJobKind,
1167+
is_optional: bool,
11671168
}
11681169

11691170
impl BenchmarkJob {
@@ -1215,6 +1216,10 @@ impl BenchmarkJob {
12151216
pub fn kind(&self) -> BenchmarkJobKind {
12161217
self.kind
12171218
}
1219+
1220+
pub fn is_optional(&self) -> bool {
1221+
self.is_optional
1222+
}
12181223
}
12191224

12201225
/// Describes the final state of a job

‎database/src/pool.rs‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ pub trait Connection: Send + Sync {
241241

242242
/// Add a benchmark job to the job queue and returns its ID, if it was not
243243
/// already in the DB previously.
244+
#[allow(clippy::too_many_arguments)]
244245
async fn enqueue_benchmark_job(
245246
&self,
246247
request_tag: &str,
@@ -249,6 +250,7 @@ pub trait Connection: Send + Sync {
249250
profile: Profile,
250251
benchmark_set: u32,
251252
kind: BenchmarkJobKind,
253+
is_optional: bool,
252254
) -> JobEnqueueResult;
253255

254256
/// Returns a set of compile-time benchmark test cases that were already computed for the
@@ -747,6 +749,7 @@ mod tests {
747749
Profile::Opt,
748750
0u32,
749751
BenchmarkJobKind::Runtime,
752+
false,
750753
)
751754
.await;
752755
match result {
@@ -906,6 +909,7 @@ mod tests {
906909
Profile::Opt,
907910
1u32,
908911
BenchmarkJobKind::Runtime,
912+
false,
909913
)
910914
.await
911915
{
@@ -1006,6 +1010,7 @@ mod tests {
10061010
Profile::Opt,
10071011
benchmark_set.0,
10081012
BenchmarkJobKind::Runtime,
1013+
false,
10091014
)
10101015
.await
10111016
.unwrap();
@@ -1260,6 +1265,7 @@ mod tests {
12601265
Profile::Check,
12611266
0,
12621267
BenchmarkJobKind::Compiletime,
1268+
false,
12631269
)
12641270
.await
12651271
.unwrap();
@@ -1286,4 +1292,98 @@ mod tests {
12861292
})
12871293
.await;
12881294
}
1295+
1296+
#[tokio::test]
1297+
async fn optional_jobs_should_not_block_benchmark_request_from_being_completed() {
1298+
run_postgres_test(|ctx| async {
1299+
let db = ctx.db();
1300+
let time = chrono::DateTime::from_str("2021-09-01T00:00:00.000Z").unwrap();
1301+
let benchmark_set = BenchmarkSet(0u32);
1302+
let tag = "sha-1";
1303+
let collector_name = "collector-1";
1304+
let target = Target::X86_64UnknownLinuxGnu;
1305+
1306+
let insert_result = db
1307+
.add_collector_config(collector_name, target, 1, true)
1308+
.await;
1309+
assert!(insert_result.is_ok());
1310+
1311+
/* Create the request */
1312+
let benchmark_request = BenchmarkRequest::create_release(tag, time);
1313+
db.insert_benchmark_request(&benchmark_request)
1314+
.await
1315+
.unwrap();
1316+
1317+
/* Create job for the request */
1318+
db.enqueue_benchmark_job(
1319+
benchmark_request.tag().unwrap(),
1320+
target,
1321+
CodegenBackend::Llvm,
1322+
Profile::Opt,
1323+
benchmark_set.0,
1324+
BenchmarkJobKind::Runtime,
1325+
false,
1326+
)
1327+
.await
1328+
.unwrap();
1329+
1330+
/* Create optional job for the request, this is somewhat unrealsitic
1331+
* as we're only going to make specific targets jobs optional */
1332+
db.enqueue_benchmark_job(
1333+
benchmark_request.tag().unwrap(),
1334+
target,
1335+
CodegenBackend::Llvm,
1336+
Profile::Doc,
1337+
benchmark_set.0,
1338+
BenchmarkJobKind::Runtime,
1339+
true,
1340+
)
1341+
.await
1342+
.unwrap();
1343+
1344+
let (job, _) = db
1345+
.dequeue_benchmark_job(collector_name, target, benchmark_set)
1346+
.await
1347+
.unwrap()
1348+
.unwrap();
1349+
1350+
assert_eq!(job.request_tag(), benchmark_request.tag().unwrap());
1351+
1352+
/* Make the job take some amount of time */
1353+
std::thread::sleep(Duration::from_millis(1000));
1354+
1355+
/* Mark the job as complete */
1356+
db.mark_benchmark_job_as_completed(job.id(), BenchmarkJobConclusion::Success)
1357+
.await
1358+
.unwrap();
1359+
1360+
db.maybe_mark_benchmark_request_as_completed(tag)
1361+
.await
1362+
.unwrap();
1363+
1364+
/* From the status page view we can see that the duration has been
1365+
* updated. Albeit that it will be a very short duration. */
1366+
let completed = db.get_last_n_completed_benchmark_requests(1).await.unwrap();
1367+
let req = &completed
1368+
.iter()
1369+
.find(|it| it.request.tag() == Some(tag))
1370+
.unwrap()
1371+
.request;
1372+
1373+
assert!(matches!(
1374+
req.status(),
1375+
BenchmarkRequestStatus::Completed { .. }
1376+
));
1377+
let BenchmarkRequestStatus::Completed { duration, .. } = req.status() else {
1378+
unreachable!();
1379+
};
1380+
assert!(duration >= Duration::from_millis(1000));
1381+
1382+
let completed_index = db.load_benchmark_request_index().await.unwrap();
1383+
assert!(completed_index.contains_tag("sha-1"));
1384+
1385+
Ok(ctx)
1386+
})
1387+
.await;
1388+
}
12891389
}

‎database/src/pool/postgres.rs‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,7 @@ static MIGRATIONS: &[&str] = &[
440440
ALTER TABLE runtime_pstat_series DROP CONSTRAINT runtime_pstat_series_benchmark_metric_key;
441441
ALTER TABLE runtime_pstat_series ADD CONSTRAINT runtime_test_case UNIQUE(benchmark, target, metric);
442442
"#,
443+
r#"ALTER TABLE job_queue ADD COLUMN is_optional BOOLEAN NOT NULL DEFAULT FALSE"#,
443444
];
444445

445446
#[async_trait::async_trait]
@@ -1791,6 +1792,7 @@ where
17911792
profile: Profile,
17921793
benchmark_set: u32,
17931794
kind: BenchmarkJobKind,
1795+
is_optional: bool,
17941796
) -> JobEnqueueResult {
17951797
// This will return zero rows if the job already exists
17961798
let result = self
@@ -1804,9 +1806,10 @@ where
18041806
profile,
18051807
benchmark_set,
18061808
status,
1807-
kind
1809+
kind,
1810+
is_optional
18081811
)
1809-
VALUES ($1, $2, $3, $4, $5, $6, $7)
1812+
VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
18101813
ON CONFLICT DO NOTHING
18111814
RETURNING job_queue.id
18121815
"#,
@@ -1818,6 +1821,7 @@ where
18181821
&(benchmark_set as i32),
18191822
&BENCHMARK_JOB_STATUS_QUEUED_STR,
18201823
&kind,
1824+
&is_optional,
18211825
],
18221826
)
18231827
.await;
@@ -1985,11 +1989,12 @@ where
19851989
AND target = $2
19861990
AND benchmark_set = $3
19871991
ORDER BY
1988-
-- Prefer in-progress jobs that have not been finished previously, so that
1989-
-- we can finish them.
1992+
-- Prefer in-progress jobs that have not been finished
1993+
-- previously, so that we can finish them. Also prefer
1994+
-- mandatory jobs over optional ones.
19901995
CASE
1991-
WHEN status = $5 THEN 0
1992-
WHEN status = $1 THEN 1
1996+
WHEN (status = $5 AND is_optional = FALSE) THEN 0
1997+
WHEN (status = $1 AND is_optional = FALSE) THEN 1
19931998
ELSE 2
19941999
END,
19952000
request_tag,
@@ -2019,6 +2024,7 @@ where
20192024
updated.started_at,
20202025
updated.retry,
20212026
updated.kind,
2027+
updated.is_optional,
20222028
br.commit_type,
20232029
br.commit_date
20242030
FROM updated
@@ -2054,9 +2060,10 @@ where
20542060
deque_counter: row.get::<_, i32>(6) as u32,
20552061
kind: BenchmarkJobKind::from_str(row.get::<_, &str>(7))
20562062
.map_err(|e| anyhow::anyhow!(e))?,
2063+
is_optional: row.get::<_, bool>(8),
20572064
};
2058-
let commit_type = row.get::<_, &str>(8);
2059-
let commit_date = row.get::<_, Option<DateTime<Utc>>>(9);
2065+
let commit_type = row.get::<_, &str>(9);
2066+
let commit_date = row.get::<_, Option<DateTime<Utc>>>(10);
20602067

20612068
let commit_date = Date(commit_date.ok_or_else(|| {
20622069
anyhow::anyhow!("Dequeuing job for a benchmark request without commit date")
@@ -2114,6 +2121,7 @@ where
21142121
WHERE
21152122
job_queue.request_tag = benchmark_request.tag
21162123
AND job_queue.status NOT IN ($3, $4)
2124+
AND job_queue.is_optional = FALSE
21172125
)
21182126
AND (
21192127
benchmark_request.parent_sha IS NULL
@@ -2264,6 +2272,7 @@ where
22642272
deque_counter: row.get::<_, i32>(10) as u32,
22652273
kind: BenchmarkJobKind::from_str(row.get::<_, &str>(12))
22662274
.map_err(|e| anyhow::anyhow!(e))?,
2275+
is_optional: row.get::<_, bool>(13),
22672276
};
22682277
request_to_jobs
22692278
.entry(job.request_tag.clone())

‎database/src/pool/sqlite.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1359,6 +1359,7 @@ impl Connection for SqliteConnection {
13591359
_profile: Profile,
13601360
_benchmark_set: u32,
13611361
_kind: BenchmarkJobKind,
1362+
_is_optional: bool,
13621363
) -> JobEnqueueResult {
13631364
no_queue_implementation_abort!()
13641365
}

‎database/src/tests/builder.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ impl RequestBuilder {
4949
job.profile,
5050
job.benchmark_set,
5151
job.kind,
52+
false,
5253
)
5354
.await
5455
{
@@ -116,13 +117,19 @@ pub struct JobBuilder {
116117
benchmark_set: u32,
117118
conclusion: BenchmarkJobConclusion,
118119
kind: BenchmarkJobKind,
120+
is_optional: bool,
119121
}
120122

121123
impl JobBuilder {
122124
pub fn profile(mut self, profile: Profile) -> Self {
123125
self.profile = profile;
124126
self
125127
}
128+
129+
pub fn is_optional(mut self, is_optional: bool) -> Self {
130+
self.is_optional = is_optional;
131+
self
132+
}
126133
}
127134

128135
impl Default for JobBuilder {
@@ -134,6 +141,7 @@ impl Default for JobBuilder {
134141
benchmark_set: 0,
135142
conclusion: BenchmarkJobConclusion::Success,
136143
kind: BenchmarkJobKind::Compiletime,
144+
is_optional: false,
137145
}
138146
}
139147
}

‎site/src/job_queue/mod.rs‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,10 +282,19 @@ pub async fn enqueue_benchmark_request(
282282
profile,
283283
benchmark_set,
284284
kind,
285-
mode: EnqueueMode| {
285+
mode: EnqueueMode,
286+
is_optional: bool| {
286287
let result = tx
287288
.conn()
288-
.enqueue_benchmark_job(request_tag, target, backend, profile, benchmark_set, kind)
289+
.enqueue_benchmark_job(
290+
request_tag,
291+
target,
292+
backend,
293+
profile,
294+
benchmark_set,
295+
kind,
296+
is_optional,
297+
)
289298
.await;
290299

291300
let make_error = |msg| {
@@ -319,6 +328,12 @@ pub async fn enqueue_benchmark_request(
319328

320329
// Target x benchmark_set x backend x profile -> BenchmarkJob
321330
for target in Target::all() {
331+
// We only have X86_64 at the moment and when we add other targets do
332+
// not want to block the Benchmark request from completing to wait on
333+
// other targets. Essentially, for the time being, other targets will
334+
// run in the background
335+
let is_optional = target != Target::X86_64UnknownLinuxGnu;
336+
322337
for benchmark_set in 0..get_benchmark_sets_for_target(target.into()).len() {
323338
for &backend in backends.iter() {
324339
for &profile in profiles.iter() {
@@ -331,6 +346,7 @@ pub async fn enqueue_benchmark_request(
331346
benchmark_set as u32,
332347
BenchmarkJobKind::Compiletime,
333348
EnqueueMode::Commit,
349+
is_optional,
334350
)
335351
.await?;
336352

@@ -354,6 +370,7 @@ pub async fn enqueue_benchmark_request(
354370
benchmark_set as u32,
355371
BenchmarkJobKind::Compiletime,
356372
EnqueueMode::Parent,
373+
is_optional,
357374
)
358375
.await?;
359376
}
@@ -372,6 +389,7 @@ pub async fn enqueue_benchmark_request(
372389
BENCHMARK_SET_RUNTIME_BENCHMARKS,
373390
BenchmarkJobKind::Runtime,
374391
EnqueueMode::Commit,
392+
is_optional,
375393
)
376394
.await?;
377395
}
@@ -389,12 +407,30 @@ pub async fn enqueue_benchmark_request(
389407
BENCHMARK_SET_RUSTC,
390408
BenchmarkJobKind::Rustc,
391409
EnqueueMode::Commit,
410+
false,
392411
)
393412
.await?;
394413
}
395414
BenchmarkRequestType::Release { .. } => {}
396415
}
397416

417+
// Enqueue Rustc job for only for x86_64 & llvm. This benchmark is how long
418+
// it takes to build the rust compiler. It takes a while to run and is
419+
// assumed that if the compilation of other rust project improve then this
420+
// too would improve.
421+
enqueue_job(
422+
&mut tx,
423+
request_tag,
424+
Target::X86_64UnknownLinuxGnu,
425+
CodegenBackend::Llvm,
426+
Profile::Opt,
427+
0u32,
428+
BenchmarkJobKind::Rustc,
429+
EnqueueMode::Commit,
430+
false,
431+
)
432+
.await?;
433+
398434
tx.conn()
399435
.update_benchmark_request_status(request_tag, BenchmarkRequestStatus::InProgress)
400436
.await
@@ -642,6 +678,7 @@ mod tests {
642678
Profile::Opt,
643679
benchmark_set,
644680
BenchmarkJobKind::Compiletime,
681+
false,
645682
)
646683
.await
647684
{

0 commit comments

Comments
 (0)