Skip to content

Commit 1bcb45c

Browse files
committed
Improve queue ordering when bootstrapping the system
If the new system boots up with an empty DB (in local experiments), it should at least try to order the master requests semi-reasonably. This change improves that.
1 parent d729dd7 commit 1bcb45c

File tree

2 files changed

+41
-12
lines changed

2 files changed

+41
-12
lines changed

database/src/tests/mod.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ use tokio_postgres::Config;
1010
use crate::pool::postgres::make_client;
1111
use crate::tests::builder::CollectorBuilder;
1212
use crate::{
13-
ArtifactId, ArtifactIdNumber, BenchmarkRequest, CollectorConfig, Commit, CommitType,
14-
Connection, Date, Pool,
13+
ArtifactId, ArtifactIdNumber, BenchmarkRequest, BenchmarkRequestInsertResult, CollectorConfig,
14+
Commit, CommitType, Connection, Date, Pool,
1515
};
1616

1717
enum TestDb {
@@ -139,7 +139,8 @@ impl TestContext {
139139
pr: u32,
140140
) -> BenchmarkRequest {
141141
let req = BenchmarkRequest::create_master(sha, parent, pr, Utc::now());
142-
self.db().insert_benchmark_request(&req).await.unwrap();
142+
let result = self.db().insert_benchmark_request(&req).await.unwrap();
143+
assert!(matches!(result, BenchmarkRequestInsertResult::Inserted));
143144
req
144145
}
145146

site/src/job_queue/mod.rs

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -130,22 +130,35 @@ fn sort_benchmark_requests(pending: PendingBenchmarkRequests) -> Vec<BenchmarkRe
130130
// The next level is those elements in the unordered queue which
131131
// are ready to be benchmarked (i.e., those with parent in done or no
132132
// parent).
133-
let level_len = partition_in_place(pending[finished..].iter_mut(), |bmr| {
133+
let mut level_len = partition_in_place(pending[finished..].iter_mut(), |bmr| {
134134
bmr.parent_sha().is_none_or(|parent| done.contains(parent))
135135
});
136136

137137
// No commit is ready for benchmarking. This can happen e.g. when a try parent commit
138-
// was forcefully removed from the master branch of rust-lang/rust. In this case, just
139-
// let the commits be benchmarked in the current order that we have, these benchmark runs
140-
// just won't have a parent result available.
138+
// was forcefully removed from the master branch of rust-lang/rust.
139+
// In this case, some benchmark runs just won't have a parent result available.
141140
if level_len == 0 {
142-
if cfg!(test) {
143-
panic!("No master/try commit is ready for benchmarking");
141+
// As a last-resort attempt, prioritize commits whose parent is unknown.
142+
// If we have commits C1(parent=C2) and C2(parent=C3) and we do not know anything about
143+
// C3, it still makes to benchmark C2 before C1, because C1 depends on C2.
144+
let pending2 = pending.clone();
145+
let level2_len = partition_in_place(pending[finished..].iter_mut(), |bmr| {
146+
bmr.parent_sha()
147+
.is_none_or(|parent| !pending2.iter().any(|p| p.tag() == Some(parent)))
148+
});
149+
if level2_len > 0 {
150+
level_len = level2_len;
144151
} else {
145-
log::warn!("No master/try commit is ready for benchmarking");
146-
return pending;
152+
// If that doesn't work just let the commits be benchmarked in the current order
153+
// that we have.
154+
if cfg!(test) {
155+
panic!("No master/try commit is ready for benchmarking");
156+
} else {
157+
log::warn!("No master/try commit is ready for benchmarking");
158+
return pending;
159+
}
147160
}
148-
}
161+
};
149162

150163
// Everything in level has the same topological order, then we sort based on heuristics
151164
let level = &mut pending[finished..][..level_len];
@@ -810,6 +823,21 @@ mod tests {
810823
.await;
811824
}
812825

826+
#[tokio::test]
827+
async fn queue_two_master_commits() {
828+
run_postgres_test(|ctx| async {
829+
ctx.add_collector(CollectorBuilder::default()).await;
830+
831+
ctx.insert_master_request("sha1", "sha2", 1).await;
832+
ctx.insert_master_request("sha2", "sha3", 2).await;
833+
834+
let queue = build_queue(ctx.db()).await?;
835+
queue_order_matches(&queue, &["sha2", "sha1"]);
836+
Ok(ctx)
837+
})
838+
.await;
839+
}
840+
813841
#[tokio::test]
814842
async fn insert_all_jobs() {
815843
run_postgres_test(|mut ctx| async {

0 commit comments

Comments
 (0)