From c8154ee2a4b323d31dfff298f973131b1d3ee7a2 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Sat, 14 Oct 2023 17:27:30 +0200 Subject: [PATCH] add delay between build attempts --- src/build_queue.rs | 76 +++++++++++++++++++++++++++++++++++++++++----- src/config.rs | 5 +++ src/db/migrate.rs | 9 ++++++ 3 files changed, 83 insertions(+), 7 deletions(-) diff --git a/src/build_queue.rs b/src/build_queue.rs index d57c42260..92e5369a5 100644 --- a/src/build_queue.rs +++ b/src/build_queue.rs @@ -83,7 +83,8 @@ impl BuildQueue { ON CONFLICT (name, version) DO UPDATE SET priority = EXCLUDED.priority, registry = EXCLUDED.registry, - attempt = 0 + attempt = 0, + last_attempt = NULL ;", &[&name, &version, &priority, ®istry], )?; @@ -179,11 +180,16 @@ impl BuildQueue { .query_opt( "SELECT id, name, version, priority, registry FROM queue - WHERE attempt < $1 + WHERE + attempt < $1 AND + (last_attempt IS NULL OR last_attempt < NOW() - make_interval(secs => $2)) ORDER BY priority ASC, attempt ASC, id ASC LIMIT 1 FOR UPDATE SKIP LOCKED", - &[&self.max_attempts], + &[ + &self.max_attempts, + &self.config.delay_between_build_attempts.as_secs_f64(), + ], )? .map(|row| QueuedCrate { id: row.get("id"), @@ -219,7 +225,12 @@ impl BuildQueue { // Increase attempt count let attempt: i32 = transaction .query_one( - "UPDATE queue SET attempt = attempt + 1 WHERE id = $1 RETURNING attempt;", + "UPDATE queue + SET + attempt = attempt + 1, + last_attempt = NOW() + WHERE id = $1 + RETURNING attempt;", &[&to_process.id], )? .get(0); @@ -502,6 +513,8 @@ impl BuildQueue { #[cfg(test)] mod tests { use super::*; + use chrono::{DateTime, Utc}; + use std::time::Duration; #[test] fn test_add_duplicate_doesnt_fail_last_priority_wins() { @@ -531,8 +544,8 @@ mod tests { let mut conn = env.db().conn(); conn.execute( " - INSERT INTO queue (name, version, priority, attempt ) - VALUES ('failed_crate', '0.1.1', 0, 99)", + INSERT INTO queue (name, version, priority, attempt, last_attempt ) + VALUES ('failed_crate', '0.1.1', 0, 99, NOW())", &[], )?; @@ -544,7 +557,7 @@ mod tests { let row = conn .query_opt( - "SELECT priority, attempt + "SELECT priority, attempt, last_attempt FROM queue WHERE name = $1 AND version = $2", &[&"failed_crate", &"0.1.1"], @@ -552,6 +565,7 @@ mod tests { .unwrap(); assert_eq!(row.get::<_, i32>(0), 9); assert_eq!(row.get::<_, i32>(1), 0); + assert!(row.get::<_, Option>>(2).is_none()); Ok(()) }) } @@ -574,6 +588,52 @@ mod tests { }) } + #[test] + fn test_wait_between_build_attempts() { + crate::test::wrapper(|env| { + env.override_config(|config| { + config.build_attempts = 99; + config.delay_between_build_attempts = Duration::from_secs(1); + }); + + let queue = env.build_queue(); + + queue.add_crate("krate", "1.0.0", 0, None)?; + + // first let it fail + queue.process_next_crate(|krate| { + assert_eq!(krate.name, "krate"); + anyhow::bail!("simulate a failure"); + })?; + + queue.process_next_crate(|_| { + // this can't happen since we didn't wait between attempts + unreachable!(); + })?; + + { + // fake the build-attempt timestamp so it's older + let mut conn = env.db().conn(); + conn.execute( + "UPDATE queue SET last_attempt = $1", + &[&(Utc::now() - chrono::Duration::seconds(60))], + )?; + } + + let mut handled = false; + // now we can process it again + queue.process_next_crate(|krate| { + assert_eq!(krate.name, "krate"); + handled = true; + Ok(()) + })?; + + assert!(handled); + + Ok(()) + }) + } + #[test] fn test_add_and_process_crates() { const MAX_ATTEMPTS: u16 = 3; @@ -581,6 +641,7 @@ mod tests { crate::test::wrapper(|env| { env.override_config(|config| { config.build_attempts = MAX_ATTEMPTS; + config.delay_between_build_attempts = Duration::ZERO; }); let queue = env.build_queue(); @@ -775,6 +836,7 @@ mod tests { crate::test::wrapper(|env| { env.override_config(|config| { config.build_attempts = MAX_ATTEMPTS; + config.delay_between_build_attempts = Duration::ZERO; }); let queue = env.build_queue(); diff --git a/src/config.rs b/src/config.rs index a773a8302..f1cea68bf 100644 --- a/src/config.rs +++ b/src/config.rs @@ -94,6 +94,7 @@ pub struct Config { // Build params pub(crate) build_attempts: u16, + pub(crate) delay_between_build_attempts: Duration, pub(crate) rustwide_workspace: PathBuf, pub(crate) temp_dir: PathBuf, pub(crate) inside_docker: bool, @@ -130,6 +131,10 @@ impl Config { Ok(Self { build_attempts: env("DOCSRS_BUILD_ATTEMPTS", 5)?, + delay_between_build_attempts: Duration::from_secs(env::( + "DOCSRS_DELAY_BETWEEN_BUILD_ATTEMPTS", + 60, + )?), crates_io_api_call_retries: env("DOCSRS_CRATESIO_API_CALL_RETRIES", 3)?, diff --git a/src/db/migrate.rs b/src/db/migrate.rs index 693af5e2a..d98d387a1 100644 --- a/src/db/migrate.rs +++ b/src/db/migrate.rs @@ -896,6 +896,15 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res "ALTER TYPE feature DROP ATTRIBUTE optional_dependency;", "ALTER TYPE feature ADD ATTRIBUTE optional_dependency BOOL;" ), + sql_migration!( + context, + 39, + "Added last_attempt column to build queue", + // upgrade query + "ALTER TABLE queue ADD COLUMN last_attempt TIMESTAMP WITH TIME ZONE;", + // downgrade query + "ALTER TABLE queue DROP COLUMN last_attempt;" + ), ]; for migration in migrations {