From e8176b466a0fb6df60deb9d988d90456ee185dda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Wed, 9 Sep 2026 09:13:33 +0200 Subject: [PATCH 1/3] Only start EC2 instances for known branches --- src/bors/handlers/workflow.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/bors/handlers/workflow.rs b/src/bors/handlers/workflow.rs index 7f83c680..749a7c85 100644 --- a/src/bors/handlers/workflow.rs +++ b/src/bors/handlers/workflow.rs @@ -158,14 +158,14 @@ pub(super) async fn handle_workflow_job_started( repo: Arc, payload: WorkflowJobStarted, ) -> anyhow::Result<()> { - if let Err(error) = try_start_ec2_instance(ctx, &db, &repo, &payload).await { - tracing::error!("Cannot start EC2 instance: {error:?}"); - } - let Some(build_kind) = get_build_kind_from_branch(&payload.branch) else { return Ok(()); }; + if let Err(error) = try_start_ec2_instance(ctx, &db, &repo, &payload, build_kind).await { + tracing::error!("Cannot start EC2 instance: {error:?}"); + } + if let BuildKind::Auto = build_kind { ctx.get_job_cache().auto_job_started( repo.repository(), @@ -185,6 +185,7 @@ async fn try_start_ec2_instance( db: &PgDbClient, repo: &RepositoryState, payload: &WorkflowJobStarted, + build_kind: BuildKind, ) -> anyhow::Result<()> { let Some(ec2_ctx) = ctx.get_ec2_ctx() else { return Ok(()); @@ -230,10 +231,6 @@ async fn try_start_ec2_instance( } }; - // If we don't know what kind of branch it is, we just assume that it is a try build - let build_kind = get_build_kind_from_branch(&payload.branch).unwrap_or(BuildKind::Try); - - // We try to spawn EC2 instances for all kinds of jobs, even those outside of try/auto branches let data = Ec2InstanceStartData { job_id: payload.job_id, job_name: payload.name.clone(), From 139d96821168089b9238e3abe24e8069f1162fae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Wed, 9 Sep 2026 09:23:33 +0200 Subject: [PATCH 2/3] Cache idempotency tokens of spawned EC2 instances in memory --- src/ec2/mod.rs | 91 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 82 insertions(+), 9 deletions(-) diff --git a/src/ec2/mod.rs b/src/ec2/mod.rs index 07ef42e3..16c8711a 100644 --- a/src/ec2/mod.rs +++ b/src/ec2/mod.rs @@ -11,7 +11,7 @@ use regex::Regex; use serde::Deserialize; use std::collections::HashMap; use std::process::Command; -use std::sync::{Arc, LazyLock}; +use std::sync::{Arc, LazyLock, Mutex}; use std::time::Duration; /// Script that will be executed on the launched EC2 instance. @@ -69,14 +69,64 @@ impl<'a> ParsedLabel<'a> { } } +/// How long should we cache spawned instances in memory. +const INSTANCE_CACHE_LIMIT: chrono::Duration = chrono::Duration::hours(1); +/// Maximum number of spawned instances to remember. +const INSTANCE_CACHE_SIZE: usize = 300; + +struct SpawnedInstance { + /// Idempotency token of the spawned instance + token: String, + spawned_at: DateTime, +} + +/// Remembers which EC2 instances were spawned recently. +#[derive(Default)] +struct SpawnedInstanceCache { + instances: Vec, +} + +impl SpawnedInstanceCache { + fn add_token(&mut self, token: String) { + self.instances.push(SpawnedInstance { + token, + spawned_at: Utc::now(), + }); + self.prune(); + } + + fn has_token(&self, token: &str) -> bool { + // Iterate from the newest ones, as there's a highest chance of getting a hit + self.instances + .iter() + .rev() + .any(|instance| instance.token == token) + } + + fn prune(&mut self) { + let now = Utc::now(); + self.instances + .retain(|instance| (now - instance.spawned_at) <= INSTANCE_CACHE_LIMIT); + if self.instances.len() > INSTANCE_CACHE_SIZE { + // Keep `INSTANCE_CACHE_SIZE` newest entries + let oldest_to_keep = self.instances.len() - INSTANCE_CACHE_SIZE; + self.instances.drain(..oldest_to_keep).for_each(|_| {}); + } + } +} + /// Context necessary to perform actions related to EC2. pub struct Ec2Context { role_arn: String, + tokens: Mutex, } impl Ec2Context { pub fn new(role_arn: String) -> Self { - Self { role_arn } + Self { + role_arn, + tokens: Mutex::new(Default::default()), + } } } @@ -117,6 +167,34 @@ pub async fn start_ec2_github_runner( )); } + // Idempotency token, to avoid starting the same instance multiple times + // For some reason, GitHub sometimes sends us the workflow job started webhook multiple + // times... + + // The idempotency token cannot be longer than 64 characters + // Commit SHA is 40 characters + // GitHub job ID is e.g. `102375935997`, so around ~12 characters + // Thus below we should have ~53 characters, with some to spare + let mut idempotency_token = format!("{}-{}", data.job_id, data.commit_sha); + idempotency_token.truncate(64); + + // Ideally, we would just be using EC2's idempotency mechanism directly. + // However, since we use a different `--user-data` for each EC2 instance, but some of them + // might get the same idempotency token (e.g. if GitHub sends us a duplicated webhook), + // the start will fail, because EC2 doesn't allow having the same idempotency token, but + // different request parameters. + // This produces noise in bors logs, and detecting this failure is brittle. + // Instead, we have our own cache, so that we don't even attempt to start such an instance + // multiple times in a short time period. + // If the cache is empty (e.g. due to a bors restart), it doesn't really matter much, because + // we still use the idempotency token, and at worst we'll get an entry in the logs. + if ec2_ctx.tokens.lock().unwrap().has_token(&idempotency_token) { + tracing::warn!( + "Skipping spawning of EC2 instance for token {idempotency_token}, as it was already started" + ); + return Ok(()); + } + // Emulate a "UUID" to avoid adding dependency on the uuid crate just for this one line. let runner_name = format!("{:x}", rand::random::()); @@ -204,13 +282,6 @@ pub async fn start_ec2_github_runner( .collect::>() .join(","); - // Idempotency token, to avoid starting the same instance multiple times - // For some reason, GitHub sometimes sends us the workflow job started webhook multiple - // times... - let mut idempotency_token = format!("{}-{}", data.job_id, data.commit_sha); - // The idempotency token cannot be longer than 64 characters - idempotency_token.truncate(64); - // Using the AWS cli is not ideal, but the alternative (depending on aws-config, aws-sdk-ssm and // asd-sdk-ec2) has a massive impact on build times and binary size, plus it currently runs into // feature hell (ring vs aws-lc-sys). The choice might be reevaluated in the future. @@ -245,6 +316,8 @@ pub async fn start_ec2_github_runner( .as_str() .unwrap_or("unknown instance id") ); + // Remember that this token was recently spawned + ec2_ctx.tokens.lock().unwrap().add_token(idempotency_token); Ok(()) } From eebd9a784df5d1396013d9f89cdb8a80f0a1a947 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Wed, 9 Sep 2026 09:32:56 +0200 Subject: [PATCH 3/3] Separate idempotency token for normal and backfilled jobs --- src/bors/handlers/workflow.rs | 3 ++- src/ec2/mod.rs | 24 +++++++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/bors/handlers/workflow.rs b/src/bors/handlers/workflow.rs index 749a7c85..26910a4d 100644 --- a/src/bors/handlers/workflow.rs +++ b/src/bors/handlers/workflow.rs @@ -8,7 +8,7 @@ use crate::bors::event::{ use crate::bors::handlers::{get_build_kind_from_branch, is_bors_observed_branch}; use crate::bors::{BuildKind, build}; use crate::database::{BuildModel, BuildStatus, PullRequestModel, WorkflowStatus}; -use crate::ec2::{Ec2InstanceStartData, ParsedLabel, start_ec2_github_runner}; +use crate::ec2::{Ec2InstanceStartData, InstanceSpawnKind, ParsedLabel, start_ec2_github_runner}; use crate::github::CommitSha; use crate::github::api::client::GithubRepositoryClient; use crate::{BorsContext, PgDbClient}; @@ -238,6 +238,7 @@ async fn try_start_ec2_instance( commit_sha: payload.commit_sha.clone(), pr_number, build_kind, + spawn_kind: InstanceSpawnKind::Normal, }; start_ec2_github_runner(ec2_ctx, ec2_config, repo, label, data).await } diff --git a/src/ec2/mod.rs b/src/ec2/mod.rs index 16c8711a..36def06a 100644 --- a/src/ec2/mod.rs +++ b/src/ec2/mod.rs @@ -70,7 +70,7 @@ impl<'a> ParsedLabel<'a> { } /// How long should we cache spawned instances in memory. -const INSTANCE_CACHE_LIMIT: chrono::Duration = chrono::Duration::hours(1); +const INSTANCE_CACHE_LIMIT: chrono::Duration = chrono::Duration::minutes(30); /// Maximum number of spawned instances to remember. const INSTANCE_CACHE_SIZE: usize = 300; @@ -130,6 +130,13 @@ impl Ec2Context { } } +pub enum InstanceSpawnKind { + /// We are spawning an instance in reaction to a webhook about a job being started. + Normal, + /// We are spawning an instance for a job that didn't receive any runner in some time. + Backfill, +} + pub struct Ec2InstanceStartData { pub job_id: JobId, pub job_name: String, @@ -137,6 +144,7 @@ pub struct Ec2InstanceStartData { pub commit_sha: CommitSha, pub pr_number: Option, pub build_kind: BuildKind, + pub spawn_kind: InstanceSpawnKind, } /// Starts an EC2 instance on AWS, which should run a self-hosted GitHub Actions runner @@ -175,7 +183,20 @@ pub async fn start_ec2_github_runner( // Commit SHA is 40 characters // GitHub job ID is e.g. `102375935997`, so around ~12 characters // Thus below we should have ~53 characters, with some to spare + // If we are backfilling, allow "breaking" through the idempotency token by adding a separate + // marker. This allows us to spawn an additional instance even if there was one spawned + // previously, with the hope that it will now work. + // Note that due to the idempotency token memory cache below, we won't be able to spawn even a + // backfilled instance more than once per `INSTANCE_CACHE_LIMIT`. let mut idempotency_token = format!("{}-{}", data.job_id, data.commit_sha); + match data.spawn_kind { + InstanceSpawnKind::Normal => {} + InstanceSpawnKind::Backfill => { + // Use a short marker to avoid filling up the 64 characters. Commit SHAs should never + // contain a dash, so this shouldn't conflict with it. + idempotency_token.push_str("-b"); + } + } idempotency_token.truncate(64); // Ideally, we would just be using EC2's idempotency mechanism directly. @@ -467,6 +488,7 @@ pub async fn backfill_ec2_instances( commit_sha: CommitSha(build.commit_sha.clone()), pr_number, build_kind: build.kind, + spawn_kind: InstanceSpawnKind::Backfill, }; let res = start_ec2_github_runner(ec2_ctx, ec2_config, &repo, label, data).await; if let Err(error) = res {