Skip to content

Commit

Permalink
add delay between build attempts
Browse files Browse the repository at this point in the history
  • Loading branch information
syphar committed Oct 14, 2023
1 parent b879020 commit c8154ee
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 7 deletions.
76 changes: 69 additions & 7 deletions src/build_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, &registry],
)?;
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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())",
&[],
)?;

Expand All @@ -544,14 +557,15 @@ 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"],
)?
.unwrap();
assert_eq!(row.get::<_, i32>(0), 9);
assert_eq!(row.get::<_, i32>(1), 0);
assert!(row.get::<_, Option<DateTime<Utc>>>(2).is_none());
Ok(())
})
}
Expand All @@ -574,13 +588,60 @@ 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;

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();
Expand Down Expand Up @@ -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();

Expand Down
5 changes: 5 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -130,6 +131,10 @@ impl Config {

Ok(Self {
build_attempts: env("DOCSRS_BUILD_ATTEMPTS", 5)?,
delay_between_build_attempts: Duration::from_secs(env::<u64>(
"DOCSRS_DELAY_BETWEEN_BUILD_ATTEMPTS",
60,
)?),

crates_io_api_call_retries: env("DOCSRS_CRATESIO_API_CALL_RETRIES", 3)?,

Expand Down
9 changes: 9 additions & 0 deletions src/db/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,15 @@ pub fn migrate(version: Option<Version>, 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 {
Expand Down

0 comments on commit c8154ee

Please sign in to comment.