Skip to content

Commit aac878a

Browse files
Prevent duplicate posting of benchmarking completion
Previously the coe relied on the (false) belief that a single commit would only be started benchmarking once. This is incorrect; we sometimes aren't aware of the new commits presence in the timing queue when the next_commit request comes in.
1 parent 1183fa6 commit aac878a

File tree

2 files changed

+20
-12
lines changed

2 files changed

+20
-12
lines changed

site/src/load.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,9 @@ pub struct Persistent {
100100
// This only persists for one try build (so should not be long at any point).
101101
#[serde(default)]
102102
pub pending_try_builds: HashSet<u32>,
103+
// Set of commit hashes for which we've completed benchmarking.
104+
#[serde(default)]
105+
pub posted_ends: Vec<String>,
103106
}
104107

105108
lazy_static::lazy_static! {
@@ -122,6 +125,7 @@ impl Persistent {
122125
try_commits: Vec::new(),
123126
current: None,
124127
pending_try_builds: HashSet::new(),
128+
posted_ends: Vec::new(),
125129
});
126130
p.write().unwrap();
127131
p

site/src/server.rs

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,7 @@ pub async fn handle_collected(
524524
let mut comment = None;
525525
{
526526
let mut persistent = data.persistent.lock();
527+
let mut persistent = &mut *persistent;
527528
match body {
528529
collected::Request::BenchmarkCommit { commit, benchmarks } => {
529530
let issue = if let Some(r#try) =
@@ -558,7 +559,6 @@ pub async fn handle_collected(
558559
} else {
559560
String::new()
560561
};
561-
let mut should_clear = false;
562562
if let Some(current) = persistent.current.as_mut() {
563563
// If the request was received twice (e.g., we stopped after we wrote DB but before
564564
// responding) then we don't want to loop the collector.
@@ -569,20 +569,24 @@ pub async fn handle_collected(
569569
if current.benchmarks.is_empty() {
570570
// post a comment to some issue
571571
if let Some(issue) = &current.issue {
572-
comment = Some((
573-
issue.clone(),
574-
format!(
575-
"Finished benchmarking try commit {}{}",
576-
current.commit.sha, comparison_url
577-
),
578-
));
579-
should_clear = true;
572+
let commit = current.commit.sha.clone();
573+
if !persistent.posted_ends.contains(&commit) {
574+
comment = Some((
575+
issue.clone(),
576+
format!(
577+
"Finished benchmarking try commit {}{}",
578+
commit, comparison_url
579+
),
580+
));
581+
persistent.posted_ends.push(commit);
582+
// keep 100 commits in cache
583+
if persistent.posted_ends.len() > 100 {
584+
persistent.posted_ends.remove(0);
585+
}
586+
}
580587
}
581588
}
582589
}
583-
if should_clear {
584-
persistent.current = None;
585-
}
586590
}
587591
}
588592

0 commit comments

Comments
 (0)