Skip to content

Commit eadb4e2

Browse files
JamesbarfordJamesbarford
committed
PR Feedback; split enqueuing job into database into fields, have default_profiles in the database as opposed to the collector
1 parent 2a514d3 commit eadb4e2

File tree

7 files changed

+135
-57
lines changed

7 files changed

+135
-57
lines changed

‎collector/src/bin/collector.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1042,7 +1042,12 @@ fn main_result() -> anyhow::Result<i32> {
10421042

10431043
let compile_config = CompileBenchmarkConfig {
10441044
benchmarks,
1045-
profiles: Profile::default_profiles(),
1045+
profiles: vec![
1046+
Profile::Check,
1047+
Profile::Debug,
1048+
Profile::Doc,
1049+
Profile::Opt,
1050+
],
10461051
scenarios: Scenario::all(),
10471052
backends,
10481053
iterations: runs.map(|v| v as usize),

‎collector/src/compile/benchmark/profile.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,6 @@ impl Profile {
3434
]
3535
}
3636

37-
/// Set of default profiles that should be benchmarked for a master/try artifact.
38-
pub fn default_profiles() -> Vec<Self> {
39-
vec![Profile::Check, Profile::Debug, Profile::Doc, Profile::Opt]
40-
}
41-
4237
pub fn is_doc(&self) -> bool {
4338
match self {
4439
Profile::Doc | Profile::DocJson => true,

‎database/src/lib.rs

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,9 +1052,14 @@ impl BenchmarkRequestIndex {
10521052
#[derive(Debug, Clone, PartialEq)]
10531053
pub enum BenchmarkJobStatus {
10541054
Queued,
1055-
InProgress,
1056-
Success,
1057-
Failure,
1055+
InProgress {
1056+
started_at: DateTime<Utc>,
1057+
},
1058+
Completed {
1059+
started_at: DateTime<Utc>,
1060+
completed_at: DateTime<Utc>,
1061+
success: bool,
1062+
},
10581063
}
10591064

10601065
const BENCHMARK_JOB_STATUS_QUEUED_STR: &str = "queued";
@@ -1066,9 +1071,14 @@ impl BenchmarkJobStatus {
10661071
pub fn as_str(&self) -> &str {
10671072
match self {
10681073
BenchmarkJobStatus::Queued => BENCHMARK_JOB_STATUS_QUEUED_STR,
1069-
BenchmarkJobStatus::InProgress => BENCHMARK_JOB_STATUS_IN_PROGRESS_STR,
1070-
BenchmarkJobStatus::Success => BENCHMARK_JOB_STATUS_SUCCESS_STR,
1071-
BenchmarkJobStatus::Failure => BENCHMARK_JOB_STATUS_FAILURE_STR,
1074+
BenchmarkJobStatus::InProgress { .. } => BENCHMARK_JOB_STATUS_IN_PROGRESS_STR,
1075+
BenchmarkJobStatus::Completed { success, .. } => {
1076+
if *success {
1077+
BENCHMARK_JOB_STATUS_SUCCESS_STR
1078+
} else {
1079+
BENCHMARK_JOB_STATUS_FAILURE_STR
1080+
}
1081+
}
10721082
}
10731083
}
10741084
}
@@ -1090,32 +1100,26 @@ pub struct BenchmarkJob {
10901100
request_tag: String,
10911101
benchmark_set: BenchmarkSet,
10921102
created_at: DateTime<Utc>,
1093-
started_at: Option<DateTime<Utc>>,
1094-
completed_at: Option<DateTime<Utc>>,
10951103
status: BenchmarkJobStatus,
10961104
retry: u32,
10971105
}
10981106

10991107
impl BenchmarkJob {
1100-
pub fn new(
1108+
pub fn create_queued(
11011109
target: Target,
11021110
backend: CodegenBackend,
11031111
profile: Profile,
11041112
request_tag: &str,
11051113
benchmark_set: u32,
1106-
created_at: DateTime<Utc>,
1107-
status: BenchmarkJobStatus,
11081114
) -> Self {
11091115
BenchmarkJob {
11101116
target,
11111117
backend,
11121118
profile,
1119+
created_at: Utc::now(),
11131120
request_tag: request_tag.to_string(),
11141121
benchmark_set: BenchmarkSet(benchmark_set),
1115-
created_at,
1116-
started_at: None,
1117-
completed_at: None,
1118-
status,
1122+
status: BenchmarkJobStatus::Queued,
11191123
retry: 0,
11201124
}
11211125
}

‎database/src/pool.rs

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::{
2-
ArtifactCollection, ArtifactId, ArtifactIdNumber, BenchmarkJob, BenchmarkRequest,
3-
BenchmarkRequestIndex, BenchmarkRequestStatus, CodegenBackend, CompileBenchmark, Target,
2+
ArtifactCollection, ArtifactId, ArtifactIdNumber, BenchmarkRequest, BenchmarkRequestIndex,
3+
BenchmarkRequestStatus, CodegenBackend, CompileBenchmark, Target,
44
};
55
use crate::{CollectionId, Index, Profile, QueuedCommit, Scenario, Step};
66
use chrono::{DateTime, Utc};
@@ -212,7 +212,14 @@ pub trait Connection: Send + Sync {
212212
) -> anyhow::Result<()>;
213213

214214
/// Add a benchmark job to the job queue.
215-
async fn enqueue_benchmark_job(&self, benchmark_job: &BenchmarkJob) -> anyhow::Result<()>;
215+
async fn enqueue_benchmark_job(
216+
&self,
217+
request_tag: &str,
218+
target: &Target,
219+
backend: &CodegenBackend,
220+
profile: &Profile,
221+
benchmark_set: u32,
222+
) -> anyhow::Result<()>;
216223
}
217224

218225
#[async_trait::async_trait]
@@ -338,7 +345,7 @@ mod tests {
338345
use super::*;
339346
use crate::{
340347
tests::{run_db_test, run_postgres_test},
341-
BenchmarkRequestStatus, BenchmarkRequestType, Commit, CommitType, Date,
348+
BenchmarkJob, BenchmarkRequestStatus, BenchmarkRequestType, Commit, CommitType, Date,
342349
};
343350

344351
/// Create a Commit
@@ -587,4 +594,43 @@ mod tests {
587594
})
588595
.await;
589596
}
597+
598+
#[tokio::test]
599+
async fn enqueue_benchmark_job() {
600+
run_postgres_test(|ctx| async {
601+
let db = ctx.db_client();
602+
let db = db.connection().await;
603+
let time = chrono::DateTime::from_str("2021-09-01T00:00:00.000Z").unwrap();
604+
let benchmark_request =
605+
BenchmarkRequest::create_master("sha-1", "parent-sha-1", 42, time);
606+
607+
let benchmark_job = BenchmarkJob::create_queued(
608+
Target::X86_64UnknownLinuxGnu,
609+
CodegenBackend::Llvm,
610+
Profile::Opt,
611+
benchmark_request.tag().unwrap(),
612+
2,
613+
);
614+
615+
// Insert the request so we don't violate the foreign key
616+
db.insert_benchmark_request(&benchmark_request)
617+
.await
618+
.unwrap();
619+
620+
// Now we can insert the job
621+
let result = db
622+
.enqueue_benchmark_job(
623+
benchmark_job.request_tag(),
624+
&benchmark_job.target(),
625+
&benchmark_job.backend(),
626+
&benchmark_job.profile(),
627+
0u32,
628+
)
629+
.await;
630+
assert!(result.is_ok());
631+
632+
Ok(ctx)
633+
})
634+
.await;
635+
}
590636
}

‎database/src/pool/postgres.rs

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::pool::{Connection, ConnectionManager, ManagedConnection, Transaction};
22
use crate::{
3-
ArtifactCollection, ArtifactId, ArtifactIdNumber, Benchmark, BenchmarkJob, BenchmarkJobStatus,
3+
ArtifactCollection, ArtifactId, ArtifactIdNumber, Benchmark, BenchmarkJobStatus,
44
BenchmarkRequest, BenchmarkRequestIndex, BenchmarkRequestStatus, BenchmarkRequestType,
55
CodegenBackend, CollectionId, Commit, CommitType, CompileBenchmark, Date, Index, Profile,
66
QueuedCommit, Scenario, Target, BENCHMARK_REQUEST_MASTER_STR, BENCHMARK_REQUEST_RELEASE_STR,
@@ -332,7 +332,7 @@ static MIGRATIONS: &[&str] = &[
332332
backend TEXT NOT NULL,
333333
profile TEXT NOT NULL,
334334
benchmark_set INTEGER NOT NULL,
335-
collector_id TEXT,
335+
collector_id INTEGER,
336336
created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
337337
started_at TIMESTAMPTZ,
338338
completed_at TIMESTAMPTZ,
@@ -344,7 +344,14 @@ static MIGRATIONS: &[&str] = &[
344344
REFERENCES benchmark_request(tag)
345345
ON DELETE CASCADE,
346346
347-
CONSTRAINT job_queue_unique UNIQUE(request_tag, target, backend, profile)
347+
CONSTRAINT job_queue_unique
348+
UNIQUE (
349+
request_tag,
350+
target,
351+
backend,
352+
profile,
353+
benchmark_set
354+
)
348355
);
349356
CREATE INDEX IF NOT EXISTS job_queue_request_tag_idx ON job_queue (request_tag);
350357
"#,
@@ -1633,7 +1640,14 @@ where
16331640
Ok(requests)
16341641
}
16351642

1636-
async fn enqueue_benchmark_job(&self, benchmark_job: &BenchmarkJob) -> anyhow::Result<()> {
1643+
async fn enqueue_benchmark_job(
1644+
&self,
1645+
request_tag: &str,
1646+
target: &Target,
1647+
backend: &CodegenBackend,
1648+
profile: &Profile,
1649+
benchmark_set: u32,
1650+
) -> anyhow::Result<()> {
16371651
self.conn()
16381652
.execute(
16391653
r#"
@@ -1646,14 +1660,15 @@ where
16461660
status
16471661
)
16481662
VALUES ($1, $2, $3, $4, $5, $6)
1663+
ON CONFLICT DO NOTHING
16491664
"#,
16501665
&[
1651-
&benchmark_job.request_tag(),
1652-
&benchmark_job.target(),
1653-
&benchmark_job.backend(),
1654-
&benchmark_job.profile(),
1655-
&(benchmark_job.benchmark_set() as i32),
1656-
&benchmark_job.status(),
1666+
&request_tag,
1667+
&target,
1668+
&backend,
1669+
&profile,
1670+
&(benchmark_set as i32),
1671+
&BenchmarkJobStatus::Queued,
16571672
],
16581673
)
16591674
.await

‎database/src/pool/sqlite.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use crate::pool::{Connection, ConnectionManager, ManagedConnection, Transaction};
22
use crate::{
3-
ArtifactCollection, ArtifactId, Benchmark, BenchmarkJob, BenchmarkRequest,
4-
BenchmarkRequestIndex, BenchmarkRequestStatus, CodegenBackend, CollectionId, Commit,
5-
CommitType, CompileBenchmark, Date, Profile, Target,
3+
ArtifactCollection, ArtifactId, Benchmark, BenchmarkRequest, BenchmarkRequestIndex,
4+
BenchmarkRequestStatus, CodegenBackend, CollectionId, Commit, CommitType, CompileBenchmark,
5+
Date, Profile, Target,
66
};
77
use crate::{ArtifactIdNumber, Index, QueuedCommit};
88
use chrono::{DateTime, TimeZone, Utc};
@@ -1297,7 +1297,14 @@ impl Connection for SqliteConnection {
12971297
no_queue_implementation_abort!()
12981298
}
12991299

1300-
async fn enqueue_benchmark_job(&self, _benchmark_job: &BenchmarkJob) -> anyhow::Result<()> {
1300+
async fn enqueue_benchmark_job(
1301+
&self,
1302+
_request_tag: &str,
1303+
_target: &Target,
1304+
_backend: &CodegenBackend,
1305+
_profile: &Profile,
1306+
_benchmark_set: u32,
1307+
) -> anyhow::Result<()> {
13011308
no_queue_implementation_abort!()
13021309
}
13031310
}

‎site/src/job_queue/mod.rs

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ use crate::load::{partition_in_place, SiteCtxt};
77
use chrono::Utc;
88
use collector::benchmark_set::benchmark_set_count;
99
use database::{
10-
BenchmarkJob, BenchmarkJobStatus, BenchmarkRequest, BenchmarkRequestIndex,
11-
BenchmarkRequestStatus, Target,
10+
BenchmarkJob, BenchmarkRequest, BenchmarkRequestIndex, BenchmarkRequestStatus, Target,
1211
};
1312
use hashbrown::HashSet;
1413
use parking_lot::RwLock;
@@ -198,14 +197,12 @@ pub fn create_benchmark_jobs(
198197
) {
199198
for backend in backends.iter() {
200199
for profile in profiles.iter() {
201-
let job = BenchmarkJob::new(
200+
let job = BenchmarkJob::create_queued(
202201
target,
203202
*backend,
204203
*profile,
205204
benchmark_request.tag().unwrap(),
206205
benchmark_set as u32,
207-
Utc::now(),
208-
BenchmarkJobStatus::Queued,
209206
);
210207
jobs.push(job);
211208
}
@@ -218,35 +215,46 @@ pub fn create_benchmark_jobs(
218215

219216
/// Enqueue the job into the job_queue
220217
async fn enqueue_next_job(
221-
conn: &dyn database::pool::Connection,
218+
conn: &mut dyn database::pool::Connection,
222219
index: &mut BenchmarkRequestIndex,
223220
) -> anyhow::Result<()> {
224221
let queue = build_queue(conn, index).await?;
222+
let mut tx = conn.transaction().await;
225223
for request in queue {
226224
if request.status() != BenchmarkRequestStatus::InProgress {
227-
for job in create_benchmark_jobs(&request)? {
228-
conn.enqueue_benchmark_job(&job).await?;
225+
for benchmark_job in create_benchmark_jobs(&request)? {
226+
tx.conn()
227+
.enqueue_benchmark_job(
228+
benchmark_job.request_tag(),
229+
&benchmark_job.target(),
230+
&benchmark_job.backend(),
231+
&benchmark_job.profile(),
232+
benchmark_job.benchmark_set(),
233+
)
234+
.await?;
229235
}
230-
conn.update_benchmark_request_status(
231-
request.tag().unwrap(),
232-
BenchmarkRequestStatus::InProgress,
233-
)
234-
.await?;
236+
tx.conn()
237+
.update_benchmark_request_status(
238+
request.tag().unwrap(),
239+
BenchmarkRequestStatus::InProgress,
240+
)
241+
.await?;
235242
break;
236243
}
237244
}
245+
tx.commit().await?;
238246
Ok(())
239247
}
240248

241249
/// For queueing jobs, add the jobs you want to queue to this function
242250
async fn cron_enqueue_jobs(site_ctxt: &Arc<SiteCtxt>) -> anyhow::Result<()> {
243-
let conn = site_ctxt.conn().await;
251+
let mut conn = site_ctxt.conn().await;
244252
let mut index = conn.load_benchmark_request_index().await?;
245253
// Put the master commits into the `benchmark_requests` queue
246254
create_benchmark_request_master_commits(site_ctxt, &*conn, &index).await?;
247255
// Put the releases into the `benchmark_requests` queue
248256
create_benchmark_request_releases(&*conn, &index).await?;
249-
enqueue_next_job(&*conn, &mut index).await?;
257+
enqueue_next_job(&mut *conn, &mut index).await?;
250258
Ok(())
251259
}
252260

@@ -455,14 +463,12 @@ mod tests {
455463
let jobs = create_benchmark_jobs(&request).unwrap();
456464

457465
let create_job = |profile: Profile| -> BenchmarkJob {
458-
BenchmarkJob::new(
466+
BenchmarkJob::create_queued(
459467
Target::X86_64UnknownLinuxGnu,
460468
CodegenBackend::Llvm,
461469
profile,
462470
request.tag().unwrap(),
463471
0u32,
464-
Utc::now(),
465-
BenchmarkJobStatus::Queued,
466472
)
467473
};
468474

0 commit comments

Comments
 (0)